From 5db4f3daccd90723fedb244945b90600f332154e Mon Sep 17 00:00:00 2001 From: "dmaclach@chromium.org" Date: Wed, 11 Nov 2009 21:49:29 +0000 Subject: BookmarkBubble.xib - Change control from combobox to popup button. The massive changes due to the xib are due to editing the xib on SnowLeopard. Changes internals of bookmark bubble controller so that it will work correctly with items with the same name. BUG=27330 TEST=Create several bookmark folders and items using the bookmark bubble and verify that it is still working correctly. Review URL: http://codereview.chromium.org/384025 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31719 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/app/nibs/BookmarkBubble.xib | 1010 ++++++++++++++++---- chrome/browser/cocoa/bookmark_bubble_controller.h | 19 +- chrome/browser/cocoa/bookmark_bubble_controller.mm | 191 ++-- .../cocoa/bookmark_bubble_controller_unittest.mm | 112 ++- 4 files changed, 1038 insertions(+), 294 deletions(-) diff --git a/chrome/app/nibs/BookmarkBubble.xib b/chrome/app/nibs/BookmarkBubble.xib index 4188850..fab9030 100644 --- a/chrome/app/nibs/BookmarkBubble.xib +++ b/chrome/app/nibs/BookmarkBubble.xib @@ -1,14 +1,18 @@ - + 1050 - 9L31a - 677 - 949.54 - 353.00 + 10C540 + 732 + 1038.25 + 458.00 + + com.apple.InterfaceBuilder.CocoaPlugin + 732 + YES - + YES @@ -16,7 +20,7 @@ YES - + YES @@ -48,7 +52,7 @@ Window BookmarkBubbleWindow - {3.40282e+38, 3.40282e+38} + {1.79769e+308, 1.79769e+308} 256 @@ -69,9 +73,9 @@ 68288064 272630784 ^IDS_BOOMARK_BUBBLE_PAGE_BOOKMARK - + LucidaGrande - 1.300000e+01 + 13 1044 @@ -79,9 +83,9 @@ 6 System controlColor - + 3 - MC42NjY2NjY2OQA + MC42NjY2NjY2NjY3AA @@ -112,7 +116,7 @@ ^IDS_CLOSE LucidaGrande - 1.100000e+01 + 11 3100 @@ -170,7 +174,7 @@ 129 LucidaGrande - 1.100000e+01 + 11 16 @@ -253,7 +257,7 @@ 6 System textBackgroundColor - + 3 MQA @@ -266,95 +270,53 @@ - + - 266 - {{8, 5}, {270, 22}} + 268 + {{5, 5}, {273, 22}} YES - - 74579521 - 272761856 - + + -2076049856 + 133120 - - YES - - - 5 - YES - - - - 274 - {15, 0} - - - YES - - YES - - - 1.200000e+01 - 1.000000e+01 - 1.000000e+03 - - 75628032 - 0 - - - LucidaGrande - 1.200000e+01 - 16 - - - 3 - MC4zMzMzMzI5OQA - - - - - 338820672 - 1024 - - - YES - - 6 - System - controlBackgroundColor - - - - - 3 - YES - - + + 109199615 + 1 + + + 400 + 75 + + YES + + + 1048576 + 2147483647 + + NSImage + NSMenuCheckmark - 3.000000e+00 - 2.000000e+00 - - - 6 - System - gridColor - - 3 - MC41AA - + + NSImage + NSMenuMixedState + + _popUpItemAction: + + + YES + + OtherViews + + YES - 1.600000e+01 - tableViewAction: - -767524864 - - - - 1 - 15 - 0 - YES + + -1 + 1 + YES + YES + 2 @@ -392,21 +354,13 @@ {{0, 0}, {2560, 1578}} - {3.40282e+38, 3.40282e+38} + {1.79769e+308, 1.79769e+308} YES - - delegate - - - - 23 - - remove: @@ -440,14 +394,6 @@ - folderComboBox_ - - - - 33 - - - viewToResize_ @@ -510,45 +456,59 @@ 56 + + + folderChanged: + + + + 63 + + + + folderPopUpButton_ + + + + 64 + YES 0 - - YES - + -2 - - RmlsZSdzIE93bmVyA + + File's Owner -1 - + First Responder -3 - + Application 31 - + 42 - + 50 @@ -557,7 +517,7 @@ YES - + 51 @@ -625,7 +585,7 @@ YES - + @@ -639,20 +599,6 @@ - 14 - - - YES - - - - - - 15 - - - - 13 @@ -732,11 +678,37 @@ + + 57 + + + YES + + + + + + 58 + + + YES + + + + + + 59 + + + YES + + + YES - + YES -1.IBPluginDependency -2.IBPluginDependency @@ -750,8 +722,6 @@ 11.IBPluginDependency 12.IBPluginDependency 13.IBPluginDependency - 14.IBPluginDependency - 15.IBPluginDependency 16.IBPluginDependency 17.IBPluginDependency 18.IBPluginDependency @@ -767,6 +737,10 @@ 50.IBWindowTemplateEditedContentRect 50.NSWindowTemplate.visibleAtLaunch 51.IBPluginDependency + 57.IBPluginDependency + 58.IBPluginDependency + 59.IBEditorWindowLastContentRect + 59.IBPluginDependency 8.IBPluginDependency 9.IBPluginDependency @@ -781,7 +755,7 @@ YES - 8.000000e+00 + 8 3 @@ -801,8 +775,6 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin - com.apple.InterfaceBuilder.CocoaPlugin - com.apple.InterfaceBuilder.CocoaPlugin {{241, 633}, {374, 145}} com.apple.InterfaceBuilder.CocoaPlugin {{241, 633}, {374, 145}} @@ -810,13 +782,15 @@ com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin com.apple.InterfaceBuilder.CocoaPlugin + {{314, 702}, {273, 4}} + com.apple.InterfaceBuilder.CocoaPlugin + com.apple.InterfaceBuilder.CocoaPlugin + com.apple.InterfaceBuilder.CocoaPlugin YES - - YES - + YES @@ -824,15 +798,13 @@ YES - - YES - + YES - 56 + 64 @@ -842,10 +814,11 @@ NSWindowController YES - + YES cancel: edit: + folderChanged: ok: remove: @@ -855,22 +828,23 @@ id id id + id YES - + YES bigTitle_ delegate_ - folderComboBox_ + folderPopUpButton_ nameTextField_ YES NSTextField id - NSComboBox + NSPopUpButton NSTextField @@ -908,7 +882,7 @@ NSObject YES - + YES otherObjectToLocalize_ owner_ @@ -931,7 +905,7 @@ NSObject YES - + YES localizerOwner_ localizer_ @@ -954,7 +928,7 @@ NSView YES - + YES viewToResize_ viewToSlideAndResize_ @@ -991,8 +965,722 @@ - - 0 + + YES + + NSActionCell + NSCell + + IBFrameworkSource + AppKit.framework/Headers/NSActionCell.h + + + + NSApplication + NSResponder + + IBFrameworkSource + AppKit.framework/Headers/NSApplication.h + + + + NSApplication + + IBFrameworkSource + AppKit.framework/Headers/NSApplicationScripting.h + + + + NSApplication + + IBFrameworkSource + AppKit.framework/Headers/NSColorPanel.h + + + + NSApplication + + IBFrameworkSource + AppKit.framework/Headers/NSHelpManager.h + + + + NSApplication + + IBFrameworkSource + AppKit.framework/Headers/NSPageLayout.h + + + + NSBox + NSView + + IBFrameworkSource + AppKit.framework/Headers/NSBox.h + + + + NSButton + NSControl + + IBFrameworkSource + AppKit.framework/Headers/NSButton.h + + + + NSButtonCell + NSActionCell + + IBFrameworkSource + AppKit.framework/Headers/NSButtonCell.h + + + + NSCell + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSCell.h + + + + NSControl + NSView + + IBFrameworkSource + AppKit.framework/Headers/NSControl.h + + + + NSFormatter + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSFormatter.h + + + + NSMenu + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSMenu.h + + + + NSMenuItemCell + NSButtonCell + + IBFrameworkSource + AppKit.framework/Headers/NSMenuItemCell.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSAccessibility.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSAlert.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSAnimation.h + + + + NSObject + + + + NSObject + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSBrowser.h + + + + NSObject + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSComboBox.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSComboBoxCell.h + + + + NSObject + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSDatePickerCell.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSDictionaryController.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSDragging.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSDrawer.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSFontManager.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSFontPanel.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSImage.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSKeyValueBinding.h + + + + NSObject + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSNibLoading.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSOutlineView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSPasteboard.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSRuleEditor.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSSavePanel.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSSound.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSSpeechRecognizer.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSSpeechSynthesizer.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSSplitView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTabView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTableView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSText.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTextStorage.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTextView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTokenField.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSTokenFieldCell.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSToolbar.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSToolbarItem.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSView.h + + + + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSWindow.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSArchiver.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSClassDescription.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSConnection.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSError.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSFileManager.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSKeyValueCoding.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSKeyValueObserving.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSKeyedArchiver.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSMetadata.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSNetServices.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSObject.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSObjectScripting.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSPort.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSPortCoder.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSRunLoop.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSScriptClassDescription.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSScriptKeyValueCoding.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSScriptObjectSpecifiers.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSScriptWhoseTests.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSSpellServer.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSStream.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSThread.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSURL.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSURLConnection.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSURLDownload.h + + + + NSObject + + IBFrameworkSource + Foundation.framework/Headers/NSXMLParser.h + + + + NSObject + + IBFrameworkSource + Print.framework/Headers/PDEPluginInterface.h + + + + NSObject + + IBFrameworkSource + QuartzCore.framework/Headers/CAAnimation.h + + + + NSObject + + IBFrameworkSource + QuartzCore.framework/Headers/CALayer.h + + + + NSObject + + IBFrameworkSource + QuartzCore.framework/Headers/CIImageProvider.h + + + + NSObject + + IBFrameworkSource + SecurityInterface.framework/Headers/SFAuthorizationView.h + + + + NSObject + + IBFrameworkSource + SecurityInterface.framework/Headers/SFCertificatePanel.h + + + + NSObject + + IBFrameworkSource + SecurityInterface.framework/Headers/SFChooseIdentityPanel.h + + + + NSPopUpButton + NSButton + + IBFrameworkSource + AppKit.framework/Headers/NSPopUpButton.h + + + + NSPopUpButtonCell + NSMenuItemCell + + IBFrameworkSource + AppKit.framework/Headers/NSPopUpButtonCell.h + + + + NSResponder + + IBFrameworkSource + AppKit.framework/Headers/NSInterfaceStyle.h + + + + NSResponder + NSObject + + IBFrameworkSource + AppKit.framework/Headers/NSResponder.h + + + + NSTextField + NSControl + + IBFrameworkSource + AppKit.framework/Headers/NSTextField.h + + + + NSTextFieldCell + NSActionCell + + IBFrameworkSource + AppKit.framework/Headers/NSTextFieldCell.h + + + + NSView + + IBFrameworkSource + AppKit.framework/Headers/NSClipView.h + + + + NSView + + IBFrameworkSource + AppKit.framework/Headers/NSMenuItem.h + + + + NSView + + IBFrameworkSource + AppKit.framework/Headers/NSRulerView.h + + + + NSView + NSResponder + + + + NSWindow + + + + NSWindow + NSResponder + + + + NSWindow + + IBFrameworkSource + AppKit.framework/Headers/NSWindowScripting.h + + + + NSWindowController + NSResponder + + showWindow: + id + + + IBFrameworkSource + AppKit.framework/Headers/NSWindowController.h + + + + + 0 + + com.apple.InterfaceBuilder.CocoaPlugin.macosx + + + + com.apple.InterfaceBuilder.CocoaPlugin.macosx + + + + com.apple.InterfaceBuilder.CocoaPlugin.InterfaceBuilder3 + + + YES ../../chrome.xcodeproj 3 diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.h b/chrome/browser/cocoa/bookmark_bubble_controller.h index 9aba12f..506af6b 100644 --- a/chrome/browser/cocoa/bookmark_bubble_controller.h +++ b/chrome/browser/cocoa/bookmark_bubble_controller.h @@ -32,15 +32,11 @@ class BookmarkNode; BookmarkModel* model_; // weak const BookmarkNode* node_; // weak - // A mapping from NSComboBox index to parent nodes. - scoped_nsobject parentMapping_; - BOOL alreadyBookmarked_; - scoped_nsobject chooseAnotherFolder_; IBOutlet NSTextField* bigTitle_; // "Bookmark" or "Bookmark Added!" IBOutlet NSTextField* nameTextField_; - IBOutlet NSComboBox* folderComboBox_; + IBOutlet NSPopUpButton* folderPopUpButton_; } // |node| is the bookmark node we edit in this bubble. @@ -61,19 +57,18 @@ class BookmarkNode; - (IBAction)ok:(id)sender; - (IBAction)remove:(id)sender; - (IBAction)cancel:(id)sender; - +- (IBAction)folderChanged:(id)sender; @end // Exposed only for unit testing. @interface BookmarkBubbleController(ExposedForUnitTesting) -- (void)fillInFolderList; -- (void)addFolderNodes:(const BookmarkNode*)parent toComboBox:(NSComboBox*)box; -- (void)updateBookmarkNode; -- (void)setTitle:(NSString*)title parentFolder:(NSString*)folder; +- (void)addFolderNodes:(const BookmarkNode*)parent + toPopUpButton:(NSPopUpButton*)button; +- (void)setTitle:(NSString*)title parentFolder:(const BookmarkNode*)parent; - (void)setParentFolderSelection:(const BookmarkNode*)parent; -- (NSString*)chooseAnotherFolderString; -- (NSComboBox*)folderComboBox; ++ (NSString*)chooseAnotherFolderString; +- (NSPopUpButton*)folderPopUpButton; @end diff --git a/chrome/browser/cocoa/bookmark_bubble_controller.mm b/chrome/browser/cocoa/bookmark_bubble_controller.mm index 5e1d7f1..887d580 100644 --- a/chrome/browser/cocoa/bookmark_bubble_controller.mm +++ b/chrome/browser/cocoa/bookmark_bubble_controller.mm @@ -11,8 +11,30 @@ #include "chrome/browser/metrics/user_metrics.h" #include "grit/generated_resources.h" +// An object to represent the ChooseAnotherFolder item in the pop up. +@interface ChooseAnotherFolder : NSObject +@end + +@implementation ChooseAnotherFolder +@end + +@interface BookmarkBubbleController () +- (void)updateBookmarkNode; +- (void)fillInFolderList; +@end + @implementation BookmarkBubbleController ++ (id)chooseAnotherFolderObject { + // Singleton object to act as a representedObject for the "choose another + // folder" item in the pop up. + static ChooseAnotherFolder* object = nil; + if (!object) { + object = [[ChooseAnotherFolder alloc] init]; + } + return object; +} + - (id)initWithDelegate:(id)delegate parentWindow:(NSWindow*)parentWindow topLeftForBubble:(NSPoint)topLeftForBubble @@ -29,8 +51,6 @@ model_ = model; node_ = node; alreadyBookmarked_ = alreadyBookmarked; - // But this is strong. - parentMapping_.reset([[NSMutableArray alloc] init]); } return self; } @@ -83,7 +103,7 @@ // remove the bookmark. - (IBAction)cancel:(id)sender { if (!alreadyBookmarked_) { - // |-remove:| calls |-close| so we don't have to bother. + // |-remove:| calls |-close| so don't do it. [self remove:sender]; } else { [self ok:sender]; @@ -97,69 +117,30 @@ [self ok:sender]; } -// We are the delegate of the combo box so we can tell when "choose -// another folder" was picked. -- (void)comboBoxSelectionDidChange:(NSNotification*)notification { - NSString* selected = [folderComboBox_ objectValueOfSelectedItem]; - if ([selected isEqual:chooseAnotherFolder_.get()]) { +// The controller is the target of the pop up button box action so it can +// handle when "choose another folder" was picked. +- (IBAction)folderChanged:(id)sender { + DCHECK([sender isEqual:folderPopUpButton_]); + NSMenuItem* selected = [folderPopUpButton_ selectedItem]; + ChooseAnotherFolder* chooseItem = [[self class] chooseAnotherFolderObject]; + if ([[selected representedObject] isEqual:chooseItem]) { UserMetrics::RecordAction(L"BookmarkBubble_EditFromCombobox", model_->profile()); [self showEditor]; } } -// We are the delegate of our own window so we know when we lose key. -// When we lose key status we close, mirroring Windows behavior. +// The controller is the delegate of the window so it receives did resign key +// notifications. When key is resigned mirror Windows behavior and close the +// window. - (void)windowDidResignKey:(NSNotification*)notification { DCHECK_EQ([notification object], [self window]); - // Can't call close from within a window delegate method. We can call - // close after it's finished though. So this will call close for us next - // time through the event loop. + // Can't call close from within a window delegate method. Call close for the + // next time through the event loop. [self performSelector:@selector(ok:) withObject:self afterDelay:0]; } -@end // BookmarkBubbleController - - -@implementation BookmarkBubbleController(ExposedForUnitTesting) - - -// Fill in all information related to the folder combo box. -- (void)fillInFolderList { - [nameTextField_ setStringValue:base::SysWideToNSString(node_->GetTitle())]; - DCHECK([parentMapping_ count] == 0); - DCHECK([folderComboBox_ numberOfItems] == 0); - [self addFolderNodes:model_->root_node() toComboBox:folderComboBox_]; - - // Add "Choose another folder...". Remember it for later to compare against. - chooseAnotherFolder_.reset( - [l10n_util::GetNSStringWithFixup(IDS_BOOMARK_BUBBLE_CHOOSER_ANOTHER_FOLDER) - retain]); - [folderComboBox_ addItemWithObjectValue:chooseAnotherFolder_.get()]; - - // Finally, select the current parent. - NSString* parentTitle = base::SysWideToNSString( - node_->GetParent()->GetTitle()); - [folderComboBox_ selectItemWithObjectValue:parentTitle]; -} - -// For the given folder node, walk the tree and add folder names to -// the given combo box. -- (void)addFolderNodes:(const BookmarkNode*)parent toComboBox:(NSComboBox*)box { - NSString* title = base::SysWideToNSString(parent->GetTitle()); - if ([title length]) { // no title if root - [box addItemWithObjectValue:title]; - [parentMapping_ insertObject:[NSValue valueWithPointer:parent] - atIndex:[box numberOfItems]-1]; - } - for (int i = 0; i < parent->GetChildCount(); i++) { - const BookmarkNode* child = parent->GetChild(i); - if (child->is_folder()) - [self addFolderNodes:child toComboBox:box]; - } -} - // Look at the dialog; if the user has changed anything, update the // bookmark node to reflect this. - (void)updateBookmarkNode { @@ -175,55 +156,89 @@ } // Then the parent folder. const BookmarkNode* oldParent = node_->GetParent(); - NSInteger selectedIndex = [folderComboBox_ indexOfSelectedItem]; - if (selectedIndex == -1) // No selection ever made. - return; - - if ((NSUInteger)selectedIndex == [parentMapping_ count]) { + NSMenuItem* selectedItem = [folderPopUpButton_ selectedItem]; + id representedObject = [selectedItem representedObject]; + if ([representedObject isEqual:[[self class] chooseAnotherFolderObject]]) { // "Choose another folder..." return; } - const BookmarkNode* newParent = static_cast( - [[parentMapping_ objectAtIndex:selectedIndex] pointerValue]); + const BookmarkNode* newParent = + static_cast([representedObject pointerValue]); DCHECK(newParent); if (oldParent != newParent) { - int index = newParent->GetChildCount(); - model_->Move(node_, newParent, index); - UserMetrics::RecordAction(L"BookmarkBubble_ChangeParent", - model_->profile()); + int index = newParent->GetChildCount(); + model_->Move(node_, newParent, index); + UserMetrics::RecordAction(L"BookmarkBubble_ChangeParent", + model_->profile()); } } -- (void)setTitle:(NSString*)title parentFolder:(NSString*)folder { - [nameTextField_ setStringValue:title]; - [folderComboBox_ selectItemWithObjectValue:folder]; +// Fill in all information related to the folder pop up button. +- (void)fillInFolderList { + [nameTextField_ setStringValue:base::SysWideToNSString(node_->GetTitle())]; + DCHECK([folderPopUpButton_ numberOfItems] == 0); + [self addFolderNodes:model_->root_node() toPopUpButton:folderPopUpButton_]; + NSMenu* menu = [folderPopUpButton_ menu]; + NSString* title = [[self class] chooseAnotherFolderString]; + NSMenuItem *item = [menu addItemWithTitle:title + action:NULL + keyEquivalent:@""]; + ChooseAnotherFolder* obj = [[self class] chooseAnotherFolderObject]; + [item setRepresentedObject:obj]; + // Finally, select the current parent. + NSValue* parentValue = [NSValue valueWithPointer:node_->GetParent()]; + NSInteger idx = [menu indexOfItemWithRepresentedObject:parentValue]; + [folderPopUpButton_ selectItemAtIndex:idx]; } -// Pick a specific parent node in the selection by finding the right -// combo box index. -- (void)setParentFolderSelection:(const BookmarkNode*)parent { - // Expectation: we have a parent mapping for all items in the - // folderComboBox except the last one ("Choose another folder..."). - DCHECK((NSInteger)[parentMapping_ count] == - [folderComboBox_ numberOfItems]-1); - for (NSUInteger i = 0; i < [parentMapping_ count]; i++) { - const BookmarkNode* possible = static_cast( - [[parentMapping_ objectAtIndex:i] pointerValue]); - DCHECK(possible); - if (possible == parent) { - [folderComboBox_ selectItemAtIndex:i]; - return; - } +@end // BookmarkBubbleController + + +@implementation BookmarkBubbleController(ExposedForUnitTesting) + ++ (NSString*)chooseAnotherFolderString { + return l10n_util::GetNSStringWithFixup( + IDS_BOOMARK_BUBBLE_CHOOSER_ANOTHER_FOLDER); +} + +// For the given folder node, walk the tree and add folder names to +// the given pop up button. +- (void)addFolderNodes:(const BookmarkNode*)parent + toPopUpButton:(NSPopUpButton*)button { + NSString* title = base::SysWideToNSString(parent->GetTitle()); + if ([title length]) { // no title if root + NSMenu* menu = [button menu]; + NSMenuItem* item = [menu addItemWithTitle:title + action:NULL + keyEquivalent:@""]; + [item setRepresentedObject:[NSValue valueWithPointer:parent]]; + } + for (int i = 0; i < parent->GetChildCount(); i++) { + const BookmarkNode* child = parent->GetChild(i); + if (child->is_folder()) + [self addFolderNodes:child toPopUpButton:button]; } - NOTREACHED(); } -- (NSString*)chooseAnotherFolderString { - return chooseAnotherFolder_.get(); +- (void)setTitle:(NSString*)title parentFolder:(const BookmarkNode*)parent { + [nameTextField_ setStringValue:title]; + [self setParentFolderSelection:parent]; +} + +// Pick a specific parent node in the selection by finding the right +// pop up button index. +- (void)setParentFolderSelection:(const BookmarkNode*)parent { + // Expectation: There is a parent mapping for all items in the + // folderPopUpButton except the last one ("Choose another folder..."). + NSMenu* menu = [folderPopUpButton_ menu]; + NSValue* parentValue = [NSValue valueWithPointer:parent]; + NSInteger idx = [menu indexOfItemWithRepresentedObject:parentValue]; + DCHECK(idx != -1); + [folderPopUpButton_ selectItemAtIndex:idx]; } -- (NSComboBox*)folderComboBox { - return folderComboBox_; +- (NSPopUpButton*)folderPopUpButton { + return folderPopUpButton_; } @end // implementation BookmarkBubbleController(ExposedForUnitTesting) diff --git a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm index d632d92..2295d48 100644 --- a/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bubble_controller_unittest.mm @@ -118,28 +118,38 @@ TEST_F(BookmarkBubbleControllerTest, TestBubbleWindow) { TEST_F(BookmarkBubbleControllerTest, TestFillInFolder) { // Create some folders, including a nested folder BookmarkModel* model = GetBookmarkModel(); - const BookmarkNode* node1 = model->AddGroup(model->GetBookmarkBarNode(), - 0, L"one"); - const BookmarkNode* node2 = model->AddGroup(model->GetBookmarkBarNode(), - 1, L"two"); - const BookmarkNode* node3 = model->AddGroup(model->GetBookmarkBarNode(), - 2, L"three"); - const BookmarkNode* node4 = model->AddGroup(node2, - 0, L"sub"); - model->AddURL(node1, 0, L"title1", GURL("http://www.google.com")); - model->AddURL(node3, 0, L"title2", GURL("http://www.google.com")); - model->AddURL(node4, 0, L"title3", GURL("http://www.google.com/reader")); + EXPECT_TRUE(model); + const BookmarkNode* bookmarkBarNode = model->GetBookmarkBarNode(); + EXPECT_TRUE(bookmarkBarNode); + const BookmarkNode* node1 = model->AddGroup(bookmarkBarNode, 0, L"one"); + EXPECT_TRUE(node1); + const BookmarkNode* node2 = model->AddGroup(bookmarkBarNode, 1, L"two"); + EXPECT_TRUE(node2); + const BookmarkNode* node3 = model->AddGroup(bookmarkBarNode, 2, L"three"); + EXPECT_TRUE(node3); + const BookmarkNode* node4 = model->AddGroup(node2, 0, L"sub"); + EXPECT_TRUE(node4); + const BookmarkNode* node5 = + model->AddURL(node1, 0, L"title1", GURL("http://www.google.com")); + EXPECT_TRUE(node5); + const BookmarkNode* node6 = + model->AddURL(node3, 0, L"title2", GURL("http://www.google.com")); + EXPECT_TRUE(node6); + const BookmarkNode* node7 = + model->AddURL(node4, 0, L"title3", GURL("http://www.google.com/reader")); + EXPECT_TRUE(node7); BookmarkBubbleController* controller = ControllerForNode(node4); EXPECT_TRUE(controller); - NSArray* items = [[controller folderComboBox] objectValues]; - EXPECT_TRUE([items containsObject:@"one"]); - EXPECT_TRUE([items containsObject:@"two"]); - EXPECT_TRUE([items containsObject:@"three"]); - EXPECT_TRUE([items containsObject:@"sub"]); - EXPECT_FALSE([items containsObject:@"title1"]); - EXPECT_FALSE([items containsObject:@"title2"]); + NSArray* titles = + [[[controller folderPopUpButton] itemArray] valueForKey:@"title"]; + EXPECT_TRUE([titles containsObject:@"one"]); + EXPECT_TRUE([titles containsObject:@"two"]); + EXPECT_TRUE([titles containsObject:@"three"]); + EXPECT_TRUE([titles containsObject:@"sub"]); + EXPECT_FALSE([titles containsObject:@"title1"]); + EXPECT_FALSE([titles containsObject:@"title2"]); } // Click on edit; bubble gets closed. @@ -179,17 +189,23 @@ TEST_F(BookmarkBubbleControllerTest, TestClose) { // User changes title and parent folder in the UI TEST_F(BookmarkBubbleControllerTest, TestUserEdit) { BookmarkModel* model = GetBookmarkModel(); - const BookmarkNode* node = model->AddURL(model->GetBookmarkBarNode(), + EXPECT_TRUE(model); + const BookmarkNode* bookmarkBarNode = model->GetBookmarkBarNode(); + EXPECT_TRUE(bookmarkBarNode); + const BookmarkNode* node = model->AddURL(bookmarkBarNode, 0, L"short-title", GURL("http://www.google.com")); - model->AddGroup(model->GetBookmarkBarNode(), 0, L"grandma"); - model->AddGroup(model->GetBookmarkBarNode(), 0, L"grandpa"); + const BookmarkNode* grandma = model->AddGroup(bookmarkBarNode, 0, L"grandma"); + EXPECT_TRUE(grandma); + const BookmarkNode* grandpa = model->AddGroup(bookmarkBarNode, 0, L"grandpa"); + EXPECT_TRUE(grandpa); + BookmarkBubbleController* controller = ControllerForNode(node); EXPECT_TRUE(controller); // simulate a user edit - [controller setTitle:@"oops" parentFolder:@"grandma"]; + [controller setTitle:@"oops" parentFolder:grandma]; [controller edit:controller]; // Make sure bookmark has changed @@ -199,29 +215,58 @@ TEST_F(BookmarkBubbleControllerTest, TestUserEdit) { // Confirm happiness with parent nodes that have the same name. TEST_F(BookmarkBubbleControllerTest, TestNewParentSameName) { + BookmarkModel* model = GetBookmarkModel(); + EXPECT_TRUE(model); + const BookmarkNode* bookmarkBarNode = model->GetBookmarkBarNode(); + EXPECT_TRUE(bookmarkBarNode); for (int i=0; i<2; i++) { - BookmarkModel* model = GetBookmarkModel(); - const BookmarkNode* node = model->AddURL(model->GetBookmarkBarNode(), + const BookmarkNode* node = model->AddURL(bookmarkBarNode, 0, L"short-title", GURL("http://www.google.com")); - model->AddGroup(model->GetBookmarkBarNode(), 0, L"NAME"); - model->AddGroup(model->GetBookmarkBarNode(), 0, L"NAME"); - model->AddGroup(model->GetBookmarkBarNode(), 0, L"NAME"); + EXPECT_TRUE(node); + const BookmarkNode* group = model->AddGroup(bookmarkBarNode, 0, L"NAME"); + EXPECT_TRUE(group); + group = model->AddGroup(bookmarkBarNode, 0, L"NAME"); + EXPECT_TRUE(group); + group = model->AddGroup(bookmarkBarNode, 0, L"NAME"); + EXPECT_TRUE(group); BookmarkBubbleController* controller = ControllerForNode(node); EXPECT_TRUE(controller); // simulate a user edit - [controller setParentFolderSelection: - model->GetBookmarkBarNode()->GetChild(i)]; + [controller setParentFolderSelection:bookmarkBarNode->GetChild(i)]; [controller edit:controller]; // Make sure bookmark has changed, and that the parent is what we // expect. This proves nobody did searching based on name. - EXPECT_EQ(node->GetParent(), model->GetBookmarkBarNode()->GetChild(i)); + EXPECT_EQ(node->GetParent(), bookmarkBarNode->GetChild(i)); } } +// Confirm happiness with nodes with the same Name +TEST_F(BookmarkBubbleControllerTest, TestDuplicateNodeNames) { + BookmarkModel* model = GetBookmarkModel(); + const BookmarkNode* bookmarkBarNode = model->GetBookmarkBarNode(); + EXPECT_TRUE(bookmarkBarNode); + const BookmarkNode* node1 = model->AddGroup(bookmarkBarNode, 0, L"NAME"); + EXPECT_TRUE(node1); + const BookmarkNode* node2 = model->AddGroup(bookmarkBarNode, 0, L"NAME"); + EXPECT_TRUE(node2); + BookmarkBubbleController* controller = ControllerForNode(bookmarkBarNode); + EXPECT_TRUE(controller); + + NSPopUpButton* button = [controller folderPopUpButton]; + [controller setParentFolderSelection:node1]; + NSMenuItem* item = [button selectedItem]; + id itemObject = [item representedObject]; + EXPECT_TRUE([itemObject isEqual:[NSValue valueWithPointer:node1]]); + [controller setParentFolderSelection:node2]; + item = [button selectedItem]; + itemObject = [item representedObject]; + EXPECT_TRUE([itemObject isEqual:[NSValue valueWithPointer:node2]]); +} + // Click the "remove" button TEST_F(BookmarkBubbleControllerTest, TestRemove) { BookmarkModel* model = GetBookmarkModel(); @@ -240,7 +285,7 @@ TEST_F(BookmarkBubbleControllerTest, TestRemove) { } // Confirm picking "choose another folder" caused edit: to be called. -TEST_F(BookmarkBubbleControllerTest, ComboSelectionChanged) { +TEST_F(BookmarkBubbleControllerTest, PopUpSelectionChanged) { BookmarkModel* model = GetBookmarkModel(); GURL gurl("http://www.google.com"); const BookmarkNode* node = model->AddURL(model->GetBookmarkBarNode(), @@ -249,9 +294,10 @@ TEST_F(BookmarkBubbleControllerTest, ComboSelectionChanged) { BookmarkBubbleController* controller = ControllerForNode(node); EXPECT_TRUE(controller); - NSString* chooseAnotherFolder = [controller chooseAnotherFolderString]; + NSPopUpButton* button = [controller folderPopUpButton]; + [button selectItemWithTitle:[[controller class] chooseAnotherFolderString]]; EXPECT_EQ([delegate_ edits], 0); - [controller setTitle:@"DOH!" parentFolder:chooseAnotherFolder]; + [button sendAction:[button action] to:[button target]]; EXPECT_EQ([delegate_ edits], 1); } -- cgit v1.1