diff options
| -rw-r--r-- | chrome/browser/bookmarks/bookmark_model_unittest.cc | 47 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_controller.h | 19 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_controller.mm | 410 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_controller_unittest.mm | 235 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_folder_controller.h | 10 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_folder_controller.mm | 271 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm | 574 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_folder_view.mm | 5 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm | 15 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_unittest_helper.h | 48 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_bar_unittest_helper.mm | 67 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_button.h | 30 | ||||
| -rw-r--r-- | chrome/browser/cocoa/bookmark_button.mm | 1 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 4 | ||||
| -rw-r--r-- | chrome/test/model_test_utils.cc | 82 | ||||
| -rw-r--r-- | chrome/test/model_test_utils.h | 40 |
16 files changed, 1638 insertions, 220 deletions
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc index f1c9c93..9d85f1f 100644 --- a/chrome/browser/bookmarks/bookmark_model_unittest.cc +++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc @@ -17,6 +17,7 @@ #include "chrome/common/chrome_paths.h" #include "chrome/common/notification_registrar.h" #include "chrome/common/notification_service.h" +#include "chrome/test/model_test_utils.h" #include "chrome/test/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" @@ -36,24 +37,6 @@ void SwapDateAdded(BookmarkNode* n1, BookmarkNode* n2) { n2->set_date_added(tmp); } -// Helper function used to verify node arrangements. -std::wstring ModelStringFromNode(const BookmarkNode* node) { - // Since the children of the node are not available as a vector, - // we'll just have to do it the hard way. - int childCount = node->GetChildCount(); - std::wstring childString; - for (int i = 0; i < childCount; ++i) { - const BookmarkNode* child = node->GetChild(i); - if (child->is_folder()) { - childString += child->GetTitle() + L":[ " + ModelStringFromNode(child) - + L"] "; - } else { - childString += child->GetTitle() + L" "; - } - } - return childString; -} - } // anonymous namespace class BookmarkModelTest : public testing::Test, public BookmarkModelObserver { @@ -326,61 +309,53 @@ TEST_F(BookmarkModelTest, Move) { TEST_F(BookmarkModelTest, Copy) { const BookmarkNode* root = model.GetBookmarkBarNode(); - model.AddURL(root, 0, L"a", GURL("http://a.com")); - const BookmarkNode* node1 = model.AddGroup(root, 1, L"1"); - model.AddURL(node1, 0, L"b", GURL("http://b.com")); - model.AddURL(node1, 1, L"c", GURL("http://c.com")); - model.AddURL(root, 2, L"d", GURL("http://d.com")); - const BookmarkNode* node2 = model.AddGroup(root, 3, L"2"); - model.AddURL(node2, 0, L"e", GURL("http://e.com")); - model.AddURL(node2, 1, L"f", GURL("http://f.com")); - model.AddURL(node2, 2, L"g", GURL("http://g.com")); - model.AddURL(root, 4, L"h", GURL("http://d.com")); + static const std::wstring model_string(L"a 1:[ b c ] d 2:[ e f g ] h "); + model_test_utils::AddNodesFromModelString(model, root, model_string); // Validate initial model. - std::wstring actualModelString = ModelStringFromNode(root); - EXPECT_EQ(L"a 1:[ b c ] d 2:[ e f g ] h ", ModelStringFromNode(root)); + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); // Copy 'd' to be after '1:b': URL item from bar to folder. const BookmarkNode* nodeToCopy = root->GetChild(2); const BookmarkNode* destination = root->GetChild(1); model.Copy(nodeToCopy, destination, 1); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a 1:[ b d c ] d 2:[ e f g ] h ", actualModelString); // Copy '1:d' to be after 'a': URL item from folder to bar. const BookmarkNode* group = root->GetChild(1); nodeToCopy = group->GetChild(1); model.Copy(nodeToCopy, root, 1); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a d 1:[ b d c ] d 2:[ e f g ] h ", actualModelString); // Copy '1' to be after '2:e': Folder from bar to folder. nodeToCopy = root->GetChild(2); destination = root->GetChild(4); model.Copy(nodeToCopy, destination, 1); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f g ] h ", actualModelString); // Copy '2:1' to be after '2:f': Folder within same folder. group = root->GetChild(4); nodeToCopy = group->GetChild(1); model.Copy(nodeToCopy, group, 3); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h ", actualModelString); // Copy first 'd' to be after 'h': URL item within the bar. nodeToCopy = root->GetChild(1); model.Copy(nodeToCopy, root, 6); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a d 1:[ b d c ] d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ", actualModelString); // Copy '2' to be after 'a': Folder within the bar. nodeToCopy = root->GetChild(4); model.Copy(nodeToCopy, root, 1); - actualModelString = ModelStringFromNode(root); + actualModelString = model_test_utils::ModelStringFromNode(root); EXPECT_EQ(L"a 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] d 1:[ b d c ] " L"d 2:[ e 1:[ b d c ] f 1:[ b d c ] g ] h d ", actualModelString); diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h index d9183b4..29952b8 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_controller.h @@ -44,13 +44,17 @@ const CGFloat kDefaultBookmarkWidth = 150.0; // TODO(jrg): http://crbug.com/36276 to get final sizes. // Used as a min/max width for buttons on menus (not on the bar). -const CGFloat kBookmarkMenuButtonMinimumWidth = kDefaultBookmarkWidth; +const CGFloat kBookmarkMenuButtonMinimumWidth = 100.0; const CGFloat kBookmarkMenuButtonMaximumWidth = 485.0; const CGFloat kBookmarkVerticalPadding = 2.0; const CGFloat kBookmarkHorizontalPadding = 1.0; // (end magic numbers from Cole) +// Make subfolder menus overlap their parent menu a bit to give a better +// perception of a menuing system. +const CGFloat kBookmarkMenuOverlap = 1.0; + // Delay before opening a subfolder (and closing the previous one) // when hovering over a folder button. const NSTimeInterval kHoverOpenDelay = 0.3; @@ -208,11 +212,7 @@ willAnimateFromState:(bookmarks::VisualState)oldState // Since we create everything before doing layout we can't be sure // that all bookmark buttons we create will be visible. Thus, // [buttons_ count] isn't a definitive check. - int bookmarkBarDisplayedButtons_; - - // We only build the off-the-side menu on demand. - // We flag a need to rebuild with this variable. - BOOL needToRebuildOffTheSideMenu_; + int displayedButtonCount_; } @property(readonly, nonatomic) bookmarks::VisualState visualState; @@ -264,9 +264,6 @@ willAnimateFromState:(bookmarks::VisualState)oldState // Provide a favIcon for a bookmark node. May return nil. - (NSImage*)favIconForNode:(const BookmarkNode*)node; -// Provide a contextual menu for a bookmark node. May return nil. -- (NSMenu*)contextMenuForNode:(const BookmarkNode*)node; - // Actions for manipulating bookmarks. // Open a normal bookmark or folder from a button, ... - (IBAction)openBookmark:(id)sender; @@ -318,7 +315,7 @@ willAnimateFromState:(bookmarks::VisualState)oldState - (NSCell*)cellForBookmarkNode:(const BookmarkNode*)node; - (void)clearBookmarkBar; - (BookmarkBarView*)buttonView; -- (NSArray*)buttons; +- (NSMutableArray*)buttons; - (NSRect)frameForBookmarkButtonFromCell:(NSCell*)cell xOffset:(int*)xOffset; - (void)checkForBookmarkButtonGrowth:(NSButton*)button; - (void)frameDidChange; @@ -336,6 +333,8 @@ willAnimateFromState:(bookmarks::VisualState)oldState - (BookmarkButton*)buttonForDroppingOnAtPoint:(NSPoint)point; - (BOOL)isEventAnExitEvent:(NSEvent*)event; - (id)folderTarget; +- (int)displayedButtonCount; + @end // The (internal) |NSPasteboard| type string for bookmark button drags, used for diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm index 1ebe272..809cba4 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller.mm @@ -138,6 +138,11 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // Modifies self->buttons_. Do not add more buttons than will fit on the view. - (void)addNodesToButtonList:(const BookmarkNode*)node; +// Create an autoreleased button appropriate for insertion into the bookmark +// bar. Update |xOffset| with the offset appropriate for the subsequent button. +- (BookmarkButton*)buttonForNode:(const BookmarkNode*)node + xOffset:(int*)xOffset; + // Puts stuff into the final visual state without animating, stopping a running // animation if necessary. - (void)finalizeVisualState; @@ -161,6 +166,14 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; - (int)indexForDragOfButton:(BookmarkButton*)sourceButton toPoint:(NSPoint)point; +// Add or remove buttons to/from the bar until it is filled but not overflowed. +- (void)redistributeButtonsOnBarAsNeeded; + +// Determine the nature of the bookmark bar contents based on the number of +// buttons showing. If too many then show the off-the-side list, if none +// then show the no items label. +- (void)reconfigureBookmarkBar; + - (void)addNode:(const BookmarkNode*)child toMenu:(NSMenu*)menu; - (void)addFolderNode:(const BookmarkNode*)node toMenu:(NSMenu*)menu; - (void)tagEmptyMenu:(NSMenu*)menu; @@ -368,11 +381,11 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // Configure the off-the-side button (e.g. specify the node range, // check if we should enable or disable it, etc). - (void)configureOffTheSideButtonContentsAndVisibility { - [[offTheSideButton_ cell] setStartingChildIndex:bookmarkBarDisplayedButtons_]; + [[offTheSideButton_ cell] setStartingChildIndex:displayedButtonCount_]; [[offTheSideButton_ cell] setBookmarkNode:bookmarkModel_->GetBookmarkBarNode()]; int bookmarkChildren = bookmarkModel_->GetBookmarkBarNode()->GetChildCount(); - if (bookmarkChildren > bookmarkBarDisplayedButtons_) { + if (bookmarkChildren > displayedButtonCount_) { [offTheSideButton_ setHidden:NO]; } else { [offTheSideButton_ setHidden:YES]; @@ -387,18 +400,8 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; - (void)frameDidChange { if (!bookmarkModel_->IsLoaded()) return; - CGFloat width = NSWidth([[self view] frame]); - if (width > savedFrameWidth_) { - savedFrameWidth_ = width; - [self clearBookmarkBar]; - [self addNodesToButtonList:bookmarkModel_->GetBookmarkBarNode()]; - [self updateTheme:[[[self view] window] themeProvider]]; - [self addButtonsToView]; - } - [self positionOffTheSideButton]; - [self addButtonsToView]; - [self configureOffTheSideButtonContentsAndVisibility]; - [self centerNoItemsLabel]; + [self updateTheme:[[[self view] window] themeProvider]]; + [self reconfigureBookmarkBar]; } // Close all bookmark folders. "Folder" here is the fake menu for @@ -449,6 +452,11 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // Else open a new one if it makes sense to do so. if ([sender bookmarkNode]->is_folder()) [folderTarget_ openBookmarkFolderFromButton:sender]; +#if 1 + else + // We're over a non-folder bookmark so close any old folders. + [self closeAllBookmarkFolders]; +#endif } // BookmarkButtonDelegate protocol implementation. @@ -528,6 +536,9 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; return folderTarget_.get(); } +- (int)displayedButtonCount { + return displayedButtonCount_; +} // Keep the "no items" label centered in response to a frame size change. - (void)centerNoItemsLabel { @@ -742,11 +753,11 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; DCHECK(afterNode); int index = afterNode->GetParent()->IndexOfChild(afterNode); // Make sure we don't get confused by buttons which aren't visible. - return std::min(index, bookmarkBarDisplayedButtons_); + return std::min(index, displayedButtonCount_); } // If nothing is to my right I am at the end! - return bookmarkBarDisplayedButtons_; + return displayedButtonCount_; } - (BOOL)dragButton:(BookmarkButton*)sourceButton @@ -776,11 +787,10 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12; // Be sure we don't try and drop a folder into itself. if (sourceNode != destParent) { - if (copy) { + if (copy) bookmarkModel_->Copy(sourceNode, destParent, destIndex); - } else { + else bookmarkModel_->Move(sourceNode, destParent, destIndex); - } } [self closeAllBookmarkFolders]; // For a hover open, if needed. @@ -862,6 +872,21 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { - (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)info { NSPoint point = [info draggingLocation]; BookmarkButton* button = [self buttonForDroppingOnAtPoint:point]; + + // Don't allow drops that would result in cycles. + if (button) { + NSData* data = [[info draggingPasteboard] + dataForType:kBookmarkButtonDragType]; + if (data && [info draggingSource]) { + BookmarkButton* sourceButton = nil; + [data getBytes:&sourceButton length:sizeof(sourceButton)]; + const BookmarkNode* sourceNode = [sourceButton bookmarkNode]; + const BookmarkNode* destNode = [button bookmarkNode]; + if (destNode->HasAncestor(sourceNode)) + button = nil; + } + } + if ([button isFolder]) { if (hoverButton_ == button) { return NSDragOperationMove; // already open or timed to open @@ -916,7 +941,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { toPoint:(NSPoint)point { CGFloat x = 0; int destIndex = [self indexForDragOfButton:sourceButton toPoint:point]; - int numButtons = bookmarkBarDisplayedButtons_; + int numButtons = displayedButtonCount_; // If it's a drop strictly between existing buttons ... if (destIndex >= 0 && destIndex < numButtons) { @@ -1121,7 +1146,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { - (IBAction)openOffTheSideFolderFromButton:(id)sender { DCHECK([sender isKindOfClass:[BookmarkButton class]]); DCHECK([[sender cell] isKindOfClass:[BookmarkButtonCell class]]); - [[sender cell] setStartingChildIndex:bookmarkBarDisplayedButtons_]; + [[sender cell] setStartingChildIndex:displayedButtonCount_]; [folderTarget_ openBookmarkFolderFromButton:sender]; } @@ -1407,8 +1432,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [buttons_ makeObjectsPerformSelector:@selector(removeFromSuperview)]; [buttons_ removeAllObjects]; [self clearMenuTagMap]; - needToRebuildOffTheSideMenu_ = YES; - bookmarkBarDisplayedButtons_ = 0; + displayedButtonCount_ = 0; // Make sure there are no stale pointers in the pasteboard. This // can be important if a bookmark is deleted (via bookmark sync) @@ -1428,7 +1452,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // TODO(jrg): move much of the cell config into the BookmarkButtonCell class. - (NSCell*)cellForBookmarkNode:(const BookmarkNode*)node { NSImage* image = node ? [self favIconForNode:node] : nil; - NSMenu* menu = [self contextMenuForNode:node]; + NSMenu* menu = node && node->is_folder() ? buttonFolderContextMenu_ : + buttonContextMenu_; BookmarkButtonCell* cell = [BookmarkButtonCell buttonCellForNode:node contextMenu:menu cellText:nil @@ -1440,11 +1465,6 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { return cell; } -- (NSMenu*)contextMenuForNode:(const BookmarkNode*)node { - return node && node->is_folder() ? buttonFolderContextMenu_ : - buttonContextMenu_; -} - // Return an appropriate width for the given bookmark button cell. // The "+2" is needed because, sometimes, Cocoa is off by a tad. // Example: for a bookmark named "Moma" or "SFGate", it is one pixel @@ -1461,12 +1481,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // for the next button. - (NSRect)frameForBookmarkButtonFromCell:(NSCell*)cell xOffset:(int*)xOffset { + DCHECK(xOffset); NSRect bounds = [buttonView_ bounds]; - // TODO(erg,jrg): There used to be an if statement here, comparing the height - // to 0. This essentially broke sizing because we're dealing with floats and - // the height wasn't precisely zero. The previous author wrote that they were - // doing this because of an animator, but we are not doing animations for beta - // so we do not care. bounds.size.height = bookmarks::kBookmarkButtonHeight; NSRect frame = NSInsetRect(bounds, @@ -1525,42 +1541,45 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { int xOffset = 0; for (int i = 0; i < node->GetChildCount(); i++) { const BookmarkNode* child = node->GetChild(i); + BookmarkButton* button = [self buttonForNode:child xOffset:&xOffset]; + if (NSMinX([button frame]) >= maxViewX) + break; + [buttons_ addObject:button]; + } +} - NSCell* cell = [self cellForBookmarkNode:child]; - NSRect frame = [self frameForBookmarkButtonFromCell:cell xOffset:&xOffset]; +- (BookmarkButton*)buttonForNode:(const BookmarkNode*)node + xOffset:(int*)xOffset { + NSCell* cell = [self cellForBookmarkNode:node]; + NSRect frame = [self frameForBookmarkButtonFromCell:cell xOffset:xOffset]; - // Break early if the button is off the end of the parent view. - if (NSMinX(frame) >= maxViewX) - break; + scoped_nsobject<BookmarkButton> + button([[BookmarkButton alloc] initWithFrame:frame]); + DCHECK(button.get()); - scoped_nsobject<BookmarkButton> - button([[BookmarkButton alloc] initWithFrame:frame]); - DCHECK(button.get()); - [buttons_ addObject:button]; + // [NSButton setCell:] warns to NOT use setCell: other than in the + // initializer of a control. However, we are using a basic + // NSButton whose initializer does not take an NSCell as an + // object. To honor the assumed semantics, we do nothing with + // NSButton between alloc/init and setCell:. + [button setCell:cell]; + [button setDelegate:self]; - // [NSButton setCell:] warns to NOT use setCell: other than in the - // initializer of a control. However, we are using a basic - // NSButton whose initializer does not take an NSCell as an - // object. To honor the assumed semantics, we do nothing with - // NSButton between alloc/init and setCell:. - [button setCell:cell]; - [button setDelegate:self]; - - if (child->is_folder()) { - [button setTarget:self]; - [button setAction:@selector(openBookmarkFolderFromButton:)]; - } else { - // Make the button do something - [button setTarget:self]; - [button setAction:@selector(openBookmark:)]; - // Add a tooltip. - NSString* title = base::SysWideToNSString(child->GetTitle()); - std::string url_string = child->GetURL().possibly_invalid_spec(); - NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", title, - url_string.c_str()]; - [button setToolTip:tooltip]; - } + if (node->is_folder()) { + [button setTarget:self]; + [button setAction:@selector(openBookmarkFolderFromButton:)]; + } else { + // Make the button do something + [button setTarget:self]; + [button setAction:@selector(openBookmark:)]; + // Add a tooltip. + NSString* title = base::SysWideToNSString(node->GetTitle()); + std::string url_string = node->GetURL().possibly_invalid_spec(); + NSString* tooltip = [NSString stringWithFormat:@"%@\n%s", title, + url_string.c_str()]; + [button setToolTip:tooltip]; } + return [[button.get() retain] autorelease]; } // Add non-bookmark buttons to the view. This includes the chevron @@ -1577,21 +1596,23 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // Add bookmark buttons to the view only if they are completely // visible and don't overlap the "other bookmarks". Remove buttons -// which are clipped. Called when building the bookmark bar and when -// the window resizes. +// which are clipped. Called when building the bookmark bar the first time. - (void)addButtonsToView { - NSView* superview = nil; - bookmarkBarDisplayedButtons_ = 0; - for (NSButton* button in buttons_.get()) { - superview = [button superview]; - if (NSMaxX([button frame]) <= NSMinX([offTheSideButton_ frame])) { - if (!superview) - [buttonView_ addSubview:button]; - bookmarkBarDisplayedButtons_++; - } else { - if (superview) - [button removeFromSuperview]; - } + displayedButtonCount_ = 0; + NSMutableArray* buttons = [self buttons]; + for (NSButton* button in buttons) { + if (NSMaxX([button frame]) > (NSMinX([offTheSideButton_ frame]) - + bookmarks::kBookmarkHorizontalPadding)) + break; + [buttonView_ addSubview:button]; + ++displayedButtonCount_; + } + NSUInteger removalCount = + [buttons count] - (NSUInteger)displayedButtonCount_; + if (removalCount > 0) { + NSRange removalRange = + NSMakeRange(displayedButtonCount_, removalCount); + [buttons removeObjectsInRange:removalRange]; } } @@ -1668,23 +1689,37 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { // http://crbug.com/38665 } -// TODO(jrg): for now this is brute force. - (void)nodeMoved:(BookmarkModel*)model oldParent:(const BookmarkNode*)oldParent oldIndex:(int)oldIndex newParent:(const BookmarkNode*)newParent newIndex:(int)newIndex { - [self loaded:model]; + const BookmarkNode* movedNode = newParent->GetChild(newIndex); + id<BookmarkButtonControllerProtocol> oldController = + [self controllerForNode:oldParent]; + id<BookmarkButtonControllerProtocol> newController = + [self controllerForNode:newParent]; + if (newController == oldController) { + [oldController moveButtonFromIndex:oldIndex toIndex:newIndex]; + } else { + [oldController removeButton:oldIndex animate:NO]; + [newController addButtonForNode:movedNode atIndex:newIndex]; + } } -// TODO(jrg): for now this is brute force. - (void)nodeAdded:(BookmarkModel*)model - parent:(const BookmarkNode*)oldParent index:(int)index { - [self loaded:model]; + parent:(const BookmarkNode*)newParent index:(int)newIndex { + const BookmarkNode* newNode = newParent->GetChild(newIndex); + id<BookmarkButtonControllerProtocol> newController = + [self controllerForNode:newParent]; + [newController addButtonForNode:newNode atIndex:newIndex]; } -// TODO(jrg): for now this is brute force. - (void)nodeRemoved:(BookmarkModel*)model parent:(const BookmarkNode*)oldParent index:(int)index { - [self loaded:model]; + // Locate the parent node. The parent may not be showing, in which case + // we do nothing. + id<BookmarkButtonControllerProtocol> parentController = + [self controllerForNode:oldParent]; + [parentController removeButton:index animate:YES]; } // TODO(jrg): for now this is brute force. @@ -1722,7 +1757,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [self loaded:model]; } -- (NSArray*)buttons { +- (NSMutableArray*)buttons { return buttons_.get(); } @@ -1916,4 +1951,205 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { [[self folderTarget] fillPasteboard:pboard forDragOfButton:button]; } +- (id<BookmarkButtonControllerProtocol>)controllerForNode: + (const BookmarkNode*)node { + // See if it's in the bar, then if it is in the hierarchy of visible + // folder menus. + if (bookmarkModel_->GetBookmarkBarNode() == node) + return self; + return [folderController_ controllerForNode:node]; +} + +- (void)reconfigureBookmarkBar { + [self redistributeButtonsOnBarAsNeeded]; + [self positionOffTheSideButton]; + [self configureOffTheSideButtonContentsAndVisibility]; + [self centerNoItemsLabel]; +} + +- (void)redistributeButtonsOnBarAsNeeded { + const BookmarkNode* node = bookmarkModel_->GetBookmarkBarNode(); + NSInteger barCount = node->GetChildCount(); + + // Determine the current maximum extent of the visible buttons. + CGFloat maxViewX = NSMaxX([[self view] bounds]); + NSButton* otherBookmarksButton = otherBookmarksButton_.get(); + // If necessary, pull in the width to account for the Other Bookmarks button. + if (otherBookmarksButton_) + maxViewX = [otherBookmarksButton frame].origin.x - + bookmarks::kBookmarkHorizontalPadding; + // If we're already overflowing, then we need to account for the chevron. + if (barCount > displayedButtonCount_) + maxViewX = [offTheSideButton_ frame].origin.x - + bookmarks::kBookmarkHorizontalPadding; + + // As a result of pasting or dragging, the bar may now have more buttons + // than will fit so remove any which overflow. They will be shown in + // the off-the-side folder. + while (displayedButtonCount_ > 0) { + BookmarkButton* button = [buttons_ lastObject]; + if (NSMaxX([button frame]) < maxViewX) + break; + [buttons_ removeLastObject]; + [button removeFromSuperview]; + --displayedButtonCount_; + } + + // As a result of cutting, deleting and dragging, the bar may now have room + // for more buttons. + int xOffset = displayedButtonCount_ > 0 ? + NSMaxX([[buttons_ lastObject] frame]) + + bookmarks::kBookmarkHorizontalPadding : 0; + for (int i = displayedButtonCount_; i < barCount; ++i) { + const BookmarkNode* child = node->GetChild(i); + BookmarkButton* button = [self buttonForNode:child xOffset:&xOffset]; + // If we're testing against the last possible button then account + // for the chevron no longer needing to be shown. + if (i == barCount + 1) + maxViewX += NSWidth([offTheSideButton_ frame]) + + bookmarks::kBookmarkHorizontalPadding; + if (NSMaxX([button frame]) >= maxViewX) + break; + ++displayedButtonCount_; + [buttons_ addObject:button]; + [buttonView_ addSubview:button]; + } +} + +#pragma mark BookmarkButtonControllerProtocol + +- (void)addButtonForNode:(const BookmarkNode*)node + atIndex:(NSInteger)buttonIndex { + int newOffset = 0; + if (buttonIndex == -1) + buttonIndex = [buttons_ count]; // New button goes at the end. + if (buttonIndex <= (NSInteger)[buttons_ count]) { + if (buttonIndex) { + BookmarkButton* targetButton = [buttons_ objectAtIndex:buttonIndex - 1]; + NSRect targetFrame = [targetButton frame]; + newOffset = targetFrame.origin.x + NSWidth(targetFrame) + + bookmarks::kBookmarkHorizontalPadding; + } + BookmarkButton* newButton = [self buttonForNode:node xOffset:&newOffset]; + CGFloat xOffset = + NSWidth([newButton frame]) + bookmarks::kBookmarkHorizontalPadding; + NSUInteger buttonCount = [buttons_ count]; + for (NSUInteger i = buttonIndex; i < buttonCount; ++i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSPoint buttonOrigin = [button frame].origin; + buttonOrigin.x += xOffset; + [button setFrameOrigin:buttonOrigin]; + } + ++displayedButtonCount_; + [buttons_ insertObject:newButton atIndex:buttonIndex]; + [buttonView_ addSubview:newButton]; + + // See if any buttons need to be pushed off to or brought in from the side. + [self reconfigureBookmarkBar]; + } else { + // A button from somewhere else (not the bar) is being moved to the + // off-the-side so insure it gets redrawn if its showing. + [self reconfigureBookmarkBar]; + [folderController_ reconfigureMenu]; + } +} + +// TODO(mrossetti): jrg wants this broken up into smaller functions. +- (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex { + if (fromIndex != toIndex) { + NSInteger buttonCount = (NSInteger)[buttons_ count]; + if (toIndex == -1) + toIndex = buttonCount; + // See if we have a simple move within the bar, which will be the case if + // both button indexes are in the visible space. + if (fromIndex < buttonCount && toIndex < buttonCount) { + BookmarkButton* movedButton = [buttons_ objectAtIndex:fromIndex]; + NSRect movedFrame = [movedButton frame]; + NSPoint toOrigin = movedFrame.origin; + CGFloat xOffset = + NSWidth(movedFrame) + bookmarks::kBookmarkHorizontalPadding; + // Hide the button to reduce flickering while drawing the window. + [movedButton setHidden:YES]; + [buttons_ removeObjectAtIndex:fromIndex]; + if (fromIndex < toIndex) { + // Move the button from left to right within the bar. + BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex - 1]; + NSRect toFrame = [targetButton frame]; + toOrigin.x = toFrame.origin.x - NSWidth(movedFrame) + NSWidth(toFrame); + for (NSInteger i = fromIndex; i < toIndex; ++i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect frame = [button frame]; + frame.origin.x -= xOffset; + [button setFrameOrigin:frame.origin]; + } + } else { + // Move the button from right to left within the bar. + BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex]; + toOrigin = [targetButton frame].origin; + for (NSInteger i = fromIndex - 1; i >= toIndex; --i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect buttonFrame = [button frame]; + buttonFrame.origin.x += xOffset; + [button setFrameOrigin:buttonFrame.origin]; + } + } + [buttons_ insertObject:movedButton atIndex:toIndex]; + [movedButton setFrameOrigin:toOrigin]; + [movedButton setHidden:NO]; + } else if (fromIndex < buttonCount) { + // A button is being removed from the bar and added to off-the-side. + // By now the node has already been inserted into the model so the + // button to be added is represented by |toIndex|. Things get + // complicated because the off-the-side is showing and must be redrawn + // while possibly re-laying out the bookmark bar. + [self removeButton:fromIndex animate:NO]; + [self reconfigureBookmarkBar]; + [folderController_ reconfigureMenu]; + } else if (toIndex < buttonCount) { + // A button is being added to the bar and removed from off-the-side. + // By now the node has already been inserted into the model so the + // button to be added is represented by |toIndex|. + const BookmarkNode* node = bookmarkModel_->GetBookmarkBarNode(); + const BookmarkNode* movedNode = node->GetChild(toIndex); + DCHECK(movedNode); + [self addButtonForNode:movedNode atIndex:toIndex]; + [self reconfigureBookmarkBar]; + } else { + // A button is being moved within the off-the-side. + fromIndex -= buttonCount; + toIndex -= buttonCount; + [folderController_ moveButtonFromIndex:fromIndex toIndex:toIndex]; + } + } +} + +- (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)animate { + if (buttonIndex < (NSInteger)[buttons_ count]) { + BookmarkButton* oldButton = [buttons_ objectAtIndex:buttonIndex]; + NSRect poofFrame = [oldButton bounds]; + NSPoint poofPoint = NSMakePoint(NSMidX(poofFrame), NSMidY(poofFrame)); + poofPoint = [oldButton convertPoint:poofPoint toView:nil]; + poofPoint = [[oldButton window] convertBaseToScreen:poofPoint]; + NSRect oldFrame = [oldButton frame]; + [oldButton removeFromSuperview]; + if (animate) + NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint, + NSZeroSize, nil, nil, nil); + CGFloat xOffset = NSWidth(oldFrame) + bookmarks::kBookmarkHorizontalPadding; + [buttons_ removeObjectAtIndex:buttonIndex]; + NSUInteger buttonCount = [buttons_ count]; + for (NSUInteger i = buttonIndex; i < buttonCount; ++i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect buttonFrame = [button frame]; + buttonFrame.origin.x -= xOffset; + [button setFrame:buttonFrame]; + // If this button is showing its menu then we need to move the menu, too. + if (button == [folderController_ parentButton]) + [folderController_ offsetFolderMenuWindow:NSMakeSize(xOffset, 0.0)]; + } + --displayedButtonCount_; + [self reconfigureBookmarkBar]; + } +} + @end diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm index 8bf9a27..996e677 100644 --- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm @@ -11,6 +11,7 @@ #import "chrome/browser/cocoa/bookmark_bar_constants.h" #import "chrome/browser/cocoa/bookmark_bar_controller.h" #import "chrome/browser/cocoa/bookmark_bar_folder_window.h" +#import "chrome/browser/cocoa/bookmark_bar_unittest_helper.h" #import "chrome/browser/cocoa/bookmark_bar_view.h" #import "chrome/browser/cocoa/bookmark_button.h" #import "chrome/browser/cocoa/bookmark_button_cell.h" @@ -20,6 +21,7 @@ #include "chrome/browser/cocoa/test_event_utils.h" #import "chrome/browser/cocoa/view_resizer_pong.h" #include "chrome/common/pref_names.h" +#include "chrome/test/model_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" #import "third_party/ocmock/OCMock/OCMock.h" @@ -171,53 +173,75 @@ class FakeTheme : public ThemeProvider { namespace { -static const int kContentAreaHeight = 500; -static const int kInfoBarViewHeight = 30; - -class BookmarkBarControllerTest : public CocoaTest { +class BookmarkBarControllerTestBase : public CocoaTest { public: - BookmarkBarControllerTest() { + BrowserTestHelper helper_; + scoped_nsobject<NSView> parent_view_; + scoped_nsobject<ViewResizerPong> resizeDelegate_; + scoped_nsobject<BookmarkBarController> bar_; + + BookmarkBarControllerTestBase() { resizeDelegate_.reset([[ViewResizerPong alloc] init]); NSRect parent_frame = NSMakeRect(0, 0, 800, 50); parent_view_.reset([[NSView alloc] initWithFrame:parent_frame]); [parent_view_ setHidden:YES]; - bar_.reset( - [[BookmarkBarControllerNoOpen alloc] - initWithBrowser:helper_.browser() - initialWidth:NSWidth(parent_frame) - delegate:nil - resizeDelegate:resizeDelegate_.get()]); - + bar_.reset([[BookmarkBarController alloc] + initWithBrowser:helper_.browser() + initialWidth:NSWidth(parent_frame) + delegate:nil + resizeDelegate:resizeDelegate_.get()]); InstallAndToggleBar(bar_.get()); - - // Create a menu/item to act like a sender - menu_.reset([[BookmarkMenu alloc] initWithTitle:@"I_dont_care"]); - menu_item_.reset([[NSMenuItem alloc] - initWithTitle:@"still_dont_care" - action:NULL - keyEquivalent:@""]); - cell_.reset([[NSButtonCell alloc] init]); - [menu_item_ setMenu:menu_.get()]; - [menu_ setDelegate:cell_.get()]; } void InstallAndToggleBar(BookmarkBarController* bar) { // Force loading of the nib. [bar view]; // Awkwardness to look like we've been installed. + for (NSView* subView in [parent_view_ subviews]) + [subView removeFromSuperview]; [parent_view_ addSubview:[bar view]]; NSRect frame = [[[bar view] superview] frame]; frame.origin.y = 100; [[[bar view] superview] setFrame:frame]; // Make sure it's on in a window so viewDidMoveToWindow is called - [[test_window() contentView] addSubview:parent_view_]; + NSView* contentView = [test_window() contentView]; + if (![parent_view_ isDescendantOf:contentView]) + [contentView addSubview:parent_view_]; // Make sure it's open so certain things aren't no-ops. [bar updateAndShowNormalBar:YES showDetachedBar:NO withAnimation:NO]; } +}; + +class BookmarkBarControllerTest : public BookmarkBarControllerTestBase { + public: + scoped_nsobject<BookmarkMenu> menu_; + scoped_nsobject<NSMenuItem> menu_item_; + scoped_nsobject<NSButtonCell> cell_; + + BookmarkBarControllerTest() { + bar_.reset( + [[BookmarkBarControllerNoOpen alloc] + initWithBrowser:helper_.browser() + initialWidth:NSWidth([parent_view_ frame]) + delegate:nil + resizeDelegate:resizeDelegate_.get()]); + + InstallAndToggleBar(bar_.get()); + + // Create a menu/item to act like a sender + menu_.reset([[BookmarkMenu alloc] initWithTitle:@"I_dont_care"]); + menu_item_.reset([[NSMenuItem alloc] + initWithTitle:@"still_dont_care" + action:NULL + keyEquivalent:@""]); + cell_.reset([[NSButtonCell alloc] init]); + [menu_item_ setMenu:menu_.get()]; + [menu_ setDelegate:cell_.get()]; + } // Return a menu item that points to the given URL. NSMenuItem* ItemForBookmarkBarMenu(GURL& gurl) { @@ -235,13 +259,9 @@ class BookmarkBarControllerTest : public CocoaTest { return menu_item_; } - BrowserTestHelper helper_; - scoped_nsobject<NSView> parent_view_; - scoped_nsobject<ViewResizerPong> resizeDelegate_; - scoped_nsobject<BookmarkBarControllerNoOpen> bar_; - scoped_nsobject<BookmarkMenu> menu_; - scoped_nsobject<NSMenuItem> menu_item_; - scoped_nsobject<NSButtonCell> cell_; + BookmarkBarControllerNoOpen* noOpenBar() { + return (BookmarkBarControllerNoOpen*)bar_.get(); + } }; TEST_F(BookmarkBarControllerTest, ShowWhenShowBookmarkBarTrue) { @@ -422,8 +442,8 @@ TEST_F(BookmarkBarControllerTest, OffTheSideButtonHidden) { EXPECT_TRUE([bar_ offTheSideButtonIsHidden]); } + const BookmarkNode* parent = model->GetBookmarkBarNode(); for (int i = 0; i < 20; i++) { - const BookmarkNode* parent = model->GetBookmarkBarNode(); model->AddURL(parent, parent->GetChildCount(), L"super duper wide title", GURL("http://superfriends.hall-of-justice.edu")); @@ -534,8 +554,8 @@ TEST_F(BookmarkBarControllerTest, OpenBookmark) { [cell setRepresentedObject:[NSValue valueWithPointer:node.get()]]; [bar_ openBookmark:button]; - EXPECT_EQ(bar_.get()->urls_[0], node->GetURL()); - EXPECT_EQ(bar_.get()->dispositions_[0], CURRENT_TAB); + EXPECT_EQ(noOpenBar()->urls_[0], node->GetURL()); + EXPECT_EQ(noOpenBar()->dispositions_[0], CURRENT_TAB); } // Confirm opening of bookmarks works from the menus (different @@ -556,8 +576,8 @@ TEST_F(BookmarkBarControllerTest, OpenBookmarkFromMenus) { GURL gurl(urls[i]); [bar_ performSelector:selectors[i] withObject:ItemForBookmarkBarMenu(gurl)]; - EXPECT_EQ(bar_.get()->urls_[0], gurl); - EXPECT_EQ(bar_.get()->dispositions_[0], dispositions[i]); + EXPECT_EQ(noOpenBar()->urls_[0], gurl); + EXPECT_EQ(noOpenBar()->dispositions_[0], dispositions[i]); [bar_ clear]; } } @@ -616,7 +636,6 @@ TEST_F(BookmarkBarControllerTest, TestAddRemoveAndClear) { TEST_F(BookmarkBarControllerTest, TestButtonLimits) { BookmarkModel* model = helper_.profile()->GetBookmarkModel(); EXPECT_EQ(0U, [[bar_ buttons] count]); - // Add one; make sure we see it. const BookmarkNode* parent = model->GetBookmarkBarNode(); model->AddURL(parent, parent->GetChildCount(), @@ -625,7 +644,7 @@ TEST_F(BookmarkBarControllerTest, TestButtonLimits) { // Add 30 which we expect to be 'too many'. Make sure we don't see // 30 buttons. - [bar_ clearBookmarkBar]; + model->Remove(parent, 0); EXPECT_EQ(0U, [[bar_ buttons] count]); for (int i=0; i<30; i++) { model->AddURL(parent, parent->GetChildCount(), @@ -767,7 +786,7 @@ TEST_F(BookmarkBarControllerTest, MiddleClick) { EXPECT_TRUE(first); [first otherMouseUp:test_event_utils::MakeMouseEvent(NSOtherMouseUp, 0)]; - EXPECT_EQ(bar_.get()->urls_.size(), 1U); + EXPECT_EQ(noOpenBar()->urls_.size(), 1U); } TEST_F(BookmarkBarControllerTest, DisplaysHelpMessageOnEmpty) { @@ -1239,12 +1258,12 @@ TEST_F(BookmarkBarControllerTest, NodeDeletedWhileMenuIsOpen) { GURL("http://www.google.com")); NSMenuItem* item = ItemForBookmarkBarMenu(initialNode); - EXPECT_EQ(0U, bar_.get()->urls_.size()); + EXPECT_EQ(0U, noOpenBar()->urls_.size()); // Basic check of the menu item and an IBOutlet it can call. EXPECT_EQ(initialNode, [bar_ nodeFromMenuItem:item]); [bar_ openBookmarkInNewWindow:item]; - EXPECT_EQ(1U, bar_.get()->urls_.size()); + EXPECT_EQ(1U, noOpenBar()->urls_.size()); [bar_ clear]; // Now delete the node and make sure things are happy (no crash, @@ -1254,7 +1273,7 @@ TEST_F(BookmarkBarControllerTest, NodeDeletedWhileMenuIsOpen) { // Should not crash by referencing a deleted node. [bar_ openBookmarkInNewWindow:item]; // Confirm the above did nothing in case it somehow didn't crash. - EXPECT_EQ(0U, bar_.get()->urls_.size()); + EXPECT_EQ(0U, noOpenBar()->urls_.size()); // Confirm some more non-crashes. [bar_ openBookmarkInNewForegroundTab:item]; @@ -1300,6 +1319,60 @@ TEST_F(BookmarkBarControllerTest, CloseFolderOnAnimate) { EXPECT_FALSE([bar_ folderController]); } +TEST_F(BookmarkBarControllerTest, MoveRemoveAddButtons) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b ] 3b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Remember how many buttons are showing. + int oldDisplayedButtons = [bar_ displayedButtonCount]; + NSArray* buttons = [bar_ buttons]; + + // Move a button around a bit. + [bar_ moveButtonFromIndex:0 toIndex:2]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"1b"]); + EXPECT_EQ(oldDisplayedButtons, [bar_ displayedButtonCount]); + [bar_ moveButtonFromIndex:2 toIndex:0]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"3b"]); + EXPECT_EQ(oldDisplayedButtons, [bar_ displayedButtonCount]); + + // Add a couple of buttons. + const BookmarkNode* parent = root->GetChild(1); // Purloin an existing node. + const BookmarkNode* node = parent->GetChild(0); + [bar_ addButtonForNode:node atIndex:0]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f"]); + EXPECT_TRUE([[[buttons objectAtIndex:3] title] isEqualToString:@"3b"]); + EXPECT_EQ(oldDisplayedButtons + 1, [bar_ displayedButtonCount]); + node = parent->GetChild(1); + [bar_ addButtonForNode:node atIndex:-1]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f"]); + EXPECT_TRUE([[[buttons objectAtIndex:3] title] isEqualToString:@"3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:4] title] isEqualToString:@"2f2b"]); + EXPECT_EQ(oldDisplayedButtons + 2, [bar_ displayedButtonCount]); + + // Remove a couple of buttons. + [bar_ removeButton:4 animate:NO]; + [bar_ removeButton:1 animate:NO]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"3b"]); + EXPECT_EQ(oldDisplayedButtons, [bar_ displayedButtonCount]); +} + + class BookmarkBarControllerOpenAllTest : public BookmarkBarControllerTest { public: BookmarkBarControllerOpenAllTest() { @@ -1394,14 +1467,14 @@ TEST_F(BookmarkBarControllerOpenAllTest, CommandClickOnFolder) { [[[fakeApp stub] andReturn:commandClick] currentEvent]; id oldApp = NSApp; NSApp = fakeApp; - size_t originalDispositionCount = bar_.get()->dispositions_.size(); + size_t originalDispositionCount = noOpenBar()->dispositions_.size(); // Click! [first performClick:first]; - size_t dispositionCount = bar_.get()->dispositions_.size(); + size_t dispositionCount = noOpenBar()->dispositions_.size(); EXPECT_EQ(originalDispositionCount+1, dispositionCount); - EXPECT_EQ(bar_.get()->dispositions_[dispositionCount-1], NEW_BACKGROUND_TAB); + EXPECT_EQ(noOpenBar()->dispositions_[dispositionCount-1], NEW_BACKGROUND_TAB); // Replace NSApp NSApp = oldApp; @@ -1469,4 +1542,78 @@ TEST_F(BookmarkBarControllerNotificationTest, DeregistersForNotifications) { // TODO(viettrungluu): figure out how to test animations. +class BookmarkBarControllerDragDropTest : public BookmarkBarControllerTestBase { + public: + BookmarkBarControllerDragDropTest() { + InstallAndToggleBar(bar_.get()); + } +}; + +TEST_F(BookmarkBarControllerDragDropTest, DragMoveBarBookmarkToOffTheSide) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1bWithLongName 2fWithLongName:[ " + "2f1bWithLongName 2f2fWithLongName:[ 2f2f1bWithLongName " + "2f2f2bWithLongName 2f2f3bWithLongName 2f4b ] 2f3bWithLongName ] " + "3bWithLongName 4bWithLongName 5bWithLongName 6bWithLongName " + "7bWithLongName 8bWithLongName 9bWithLongName 10bWithLongName " + "11bWithLongName 12bWithLongName 13b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Insure that the off-the-side is showing. + ASSERT_FALSE([bar_ offTheSideButtonIsHidden]); + + // Remember how many buttons are showing and are available. + int oldDisplayedButtons = [bar_ displayedButtonCount]; + int oldChildCount = root->GetChildCount(); + + // Pop up the off-the-side menu. + BookmarkButton* otsButton = (BookmarkButton*)[bar_ offTheSideButton]; + ASSERT_TRUE(otsButton); + [[otsButton target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:otsButton]; + BookmarkBarFolderController* otsController = [bar_ folderController]; + EXPECT_TRUE(otsController); + NSWindow* toWindow = [otsController window]; + EXPECT_TRUE(toWindow); + BookmarkButton* draggedButton = [bar_ buttonWithTitleEqualTo:@"3bWithLongName"]; + ASSERT_TRUE(draggedButton); + int oldOTSCount = (int)[[otsController buttons] count]; + EXPECT_EQ(oldOTSCount, oldChildCount - oldDisplayedButtons); + BookmarkButton* targetButton = [[otsController buttons] objectAtIndex:0]; + ASSERT_TRUE(targetButton); + [otsController dragButton:draggedButton + to:[targetButton center] + copy:YES]; + // There should still be the same number of buttons in the bar + // and off-the-side should have one more. + int newDisplayedButtons = [bar_ displayedButtonCount]; + int newChildCount = root->GetChildCount(); + int newOTSCount = (int)[[otsController buttons] count]; + EXPECT_EQ(oldDisplayedButtons, newDisplayedButtons); + EXPECT_EQ(oldChildCount + 1, newChildCount); + EXPECT_EQ(oldOTSCount + 1, newOTSCount); + EXPECT_EQ(newOTSCount, newChildCount - newDisplayedButtons); +} + +TEST_F(BookmarkBarControllerDragDropTest, ControllerForNode) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b ] 3b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Find the main bar controller. + const void* expectedController = bar_; + const void* actualController = [bar_ controllerForNode:root]; + EXPECT_EQ(expectedController, actualController); +} + } // namespace diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.h b/chrome/browser/cocoa/bookmark_bar_folder_controller.h index fef2a56..8b88253 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_controller.h +++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.h @@ -110,6 +110,16 @@ // Return the parent button that owns the bookmark folder we represent. - (BookmarkButton*)parentButton; +// Offset our folder menu window. This is usually needed in response to a +// parent folder menu window or the bookmark bar changing position due to +// the dragging of a bookmark node from the parent into this folder menu. +- (void)offsetFolderMenuWindow:(NSSize)offset; + +// Re-layout the window menu in case some buttons were added or removed, +// specifically as a result of the bookmark bar changing configuration +// and altering the contents of the off-the-side folder. +- (void)reconfigureMenu; + // Actions from a context menu over a button or folder. - (IBAction)cutBookmark:(id)sender; - (IBAction)copyBookmark:(id)sender; diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm index 29d7185..461d555 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm @@ -17,7 +17,6 @@ #import "chrome/browser/cocoa/browser_window_controller.h" #import "chrome/browser/cocoa/event_utils.h" - namespace { // Frequency of the scrolling timer in seconds. @@ -57,6 +56,12 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; - (void)removeScrollTracking; - (void)endScroll; - (void)addScrollTimerWithDelta:(CGFloat)delta; + +// Determine the best button width (which will be the widest button or the +// maximum allowable button width, whichever is less) and resize all buttons. +// Return the new width (so that the window can be adjusted, if necessary). +- (CGFloat)adjustButtonWidths; + @end @implementation BookmarkBarFolderController @@ -69,6 +74,10 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; ofType:@"nib"]; if ((self = [super initWithWindowNibPath:nibPath owner:self])) { parentButton_.reset([button retain]); +#if 1 + // We want the button to remain bordered as part of the menu path. + [[button cell] setShowsBorderOnlyWhileMouseInside:NO]; +#endif parentController_.reset([parentController retain]); barController_ = barController; // WEAK buttons_.reset([[NSMutableArray alloc] init]); @@ -82,6 +91,11 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; } - (void)dealloc { +#if 1 + // The button is no longer part of the menu path. + [[parentButton_ cell] setShowsBorderOnlyWhileMouseInside:YES]; + [parentButton_ setNeedsDisplay]; +#endif [self removeScrollTracking]; [self endScroll]; [hoverState_ draggingExited]; @@ -143,9 +157,13 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; // The "+2" is needed because, sometimes, Cocoa is off by a tad when // returning the value it thinks it needs. CGFloat desired = [cell cellSize].width + 2; - frame.size.width = std::min( - std::max(bookmarks::kBookmarkMenuButtonMinimumWidth, desired), - bookmarks::kBookmarkMenuButtonMaximumWidth); + // The width is determined from the maximum of the proposed width + // (provided in |frame|) or the natural width of the title, then + // limited by the abolute minimum and maximum allowable widths. + frame.size.width = + std::min(std::max(bookmarks::kBookmarkMenuButtonMinimumWidth, + std::max(frame.size.width, desired)), + bookmarks::kBookmarkMenuButtonMaximumWidth); BookmarkButton* button = [[[BookmarkButton alloc] initWithFrame:frame] autorelease]; @@ -210,11 +228,13 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; bookmarkBarBottomLeftInScreen.y); } else { // Our parent controller is another BookmarkBarFolderController. - // In this case, start ot the RIGHT of the parent button. + // In this case, start with a slight overlap on the RIGHT of the + // parent button, which looks much more menu-like than with none. // Start to RIGHT of the button. // TODO(jrg): If too far to right, pop left again. // http://crbug.com/36225 - newWindowTopLeft.x = NSMaxX([[parentButton_ window] frame]); + newWindowTopLeft.x = NSMaxX([[parentButton_ window] frame]) - + bookmarks::kBookmarkMenuOverlap; NSPoint top = NSMakePoint(0, (NSMaxY([parentButton_ frame]) + bookmarks::kBookmarkVerticalPadding)); NSPoint topOfWindow = @@ -254,7 +274,7 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; NSRect buttonsOuterFrame = NSMakeRect( bookmarks::kBookmarkHorizontalPadding, height - (bookmarks::kBookmarkBarHeight - - bookmarks::kBookmarkHorizontalPadding), + bookmarks::kBookmarkVerticalPadding), bookmarks::kDefaultBookmarkWidth, (bookmarks::kBookmarkBarHeight - 2 * bookmarks::kBookmarkVerticalPadding)); @@ -281,24 +301,10 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; } } - // Now that we have all our buttons we can determine the real size - // of our window. - CGFloat width = 0.0; - for (BookmarkButton* button in buttons_.get()) { - width = std::max(width, NSWidth([button bounds])); - } - width = std::min(width, bookmarks::kBookmarkMenuButtonMaximumWidth); - - // Things look and feel more menu-like if all the buttons are the - // full width of the window, especially if there are submenus. - for (BookmarkButton* button in buttons_.get()) { - NSRect buttonFrame = [button frame]; - buttonFrame.size.width = width; - [button setFrame:buttonFrame]; - } - width += (2 * bookmarks::kBookmarkVerticalPadding); - - // Finally, set our window size (make sure it fits on screen). + // Now that all buttons have been installed, adjust their sizes to be + // consistent, determine the best size for the window, and set the window. + CGFloat width = + [self adjustButtonWidths] + (2 * bookmarks::kBookmarkVerticalPadding); NSRect windowFrame = NSMakeRect(newWindowTopLeft.x, newWindowTopLeft.y - height, width + kScrollViewContentWidthMargin, @@ -341,6 +347,169 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; [self configureWindowLevel]; } +- (void)offsetFolderMenuWindow:(NSSize)offset { + NSWindow* window = [self window]; + NSRect windowFrame = [window frame]; + windowFrame.origin.x -= offset.width; + windowFrame.origin.y += offset.height; // Yes, in the opposite direction! + [window setFrame:windowFrame display:YES]; + [folderController_ offsetFolderMenuWindow:offset]; +} + +// TODO(mrossetti): See if the following can be moved into view's viewWillDraw:. +- (CGFloat)adjustButtonWidths { + CGFloat width = 0.0; + for (BookmarkButton* button in buttons_.get()) { + width = std::max(width, NSWidth([button bounds])); + } + width = std::min(width, bookmarks::kBookmarkMenuButtonMaximumWidth); + // Things look and feel more menu-like if all the buttons are the + // full width of the window, especially if there are submenus. + for (BookmarkButton* button in buttons_.get()) { + NSRect buttonFrame = [button frame]; + buttonFrame.size.width = width; + [button setFrame:buttonFrame]; + } + return width; +} + +#pragma mark BookmarkButtonControllerProtocol + +- (void)addButtonForNode:(const BookmarkNode*)node + atIndex:(NSInteger)buttonIndex { + if (buttonIndex == -1) + buttonIndex = [buttons_ count]; + + // Remember the last moved button's frame or the default. + NSRect buttonFrame = NSMakeRect(bookmarks::kBookmarkHorizontalPadding, + NSHeight([mainView_ frame]) + bookmarks::kBookmarkBarHeight + + bookmarks::kBookmarkVerticalPadding, + NSWidth([mainView_ frame]) - 2 * bookmarks::kBookmarkHorizontalPadding + - kScrollViewContentWidthMargin, + bookmarks::kBookmarkBarHeight - 2 * bookmarks::kBookmarkVerticalPadding); + BookmarkButton* button = nil; // Remember so it can be inavalidated. + for (NSInteger i = 0; i < buttonIndex; ++i) { + button = [buttons_ objectAtIndex:i]; + buttonFrame = [button frame]; + buttonFrame.origin.y += bookmarks::kBookmarkBarHeight; + [button setFrame:buttonFrame]; + } + [[button cell] mouseExited:nil]; // De-highlight. + buttonFrame.origin.y -= bookmarks::kBookmarkBarHeight; + BookmarkButton* newButton = [self makeButtonForNode:node + frame:buttonFrame]; + [buttons_ insertObject:newButton atIndex:buttonIndex]; + [mainView_ addSubview:newButton]; + + // Close any child folder(s) which may still be open. + [self closeBookmarkFolder:self]; + + // Update all button widths in case more width is needed. + CGFloat windowWidth = + [self adjustButtonWidths] + (2 * bookmarks::kBookmarkVerticalPadding) + + kScrollViewContentWidthMargin; + + // Update vertical metrics of the window and the main view (using sizers + // does not do what we want). + NSWindow* window = [self window]; + NSRect frame = [mainView_ frame]; + const CGFloat verticalDelta = + bookmarks::kBookmarkBarHeight - bookmarks::kBookmarkVerticalPadding; + frame.size.height += verticalDelta; + frame.size.width = windowWidth; + [mainView_ setFrame:frame]; + frame = [window frame]; + frame.origin.y -= bookmarks::kBookmarkBarHeight; + frame.size.height += bookmarks::kBookmarkBarHeight; + frame.size.width = windowWidth; + [window setFrame:frame display:YES]; +} + +- (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex { + if (fromIndex != toIndex) { + if (toIndex == -1) + toIndex = [buttons_ count]; + BookmarkButton* movedButton = [buttons_ objectAtIndex:fromIndex]; + [buttons_ removeObjectAtIndex:fromIndex]; + NSRect movedFrame = [movedButton frame]; + NSPoint toOrigin = movedFrame.origin; + [movedButton setHidden:YES]; + if (fromIndex < toIndex) { + BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex - 1]; + toOrigin = [targetButton frame].origin; + for (NSInteger i = fromIndex; i < toIndex; ++i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect frame = [button frame]; + frame.origin.y += bookmarks::kBookmarkBarHeight; + [button setFrameOrigin:frame.origin]; + } + } else { + BookmarkButton* targetButton = [buttons_ objectAtIndex:toIndex]; + toOrigin = [targetButton frame].origin; + for (NSInteger i = fromIndex - 1; i >= toIndex; --i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect buttonFrame = [button frame]; + buttonFrame.origin.y -= bookmarks::kBookmarkBarHeight; + [button setFrameOrigin:buttonFrame.origin]; + } + } + [buttons_ insertObject:movedButton atIndex:toIndex]; + [movedButton setFrameOrigin:toOrigin]; + [movedButton setHidden:NO]; + } +} + +// TODO(jrg): Refactor BookmarkBarFolder common code. http://crbug.com/35966 +- (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)animate { + // TODO(mrossetti): Get disappearing animation to work. http://crbug.com/42360 + NSWindow* window = [self window]; + BookmarkButton* oldButton = [buttons_ objectAtIndex:buttonIndex]; + NSRect poofFrame = [oldButton bounds]; + NSPoint poofPoint = NSMakePoint(NSMidX(poofFrame), NSMidY(poofFrame)); + poofPoint = [oldButton convertPoint:poofPoint toView:nil]; + poofPoint = [[oldButton window] convertBaseToScreen:poofPoint]; + [oldButton removeFromSuperview]; + if (animate) + NSShowAnimationEffect(NSAnimationEffectDisappearingItemDefault, poofPoint, + NSZeroSize, nil, nil, nil); + [buttons_ removeObjectAtIndex:buttonIndex]; + for (NSInteger i = 0; i < buttonIndex; ++i) { + BookmarkButton* button = [buttons_ objectAtIndex:i]; + NSRect buttonFrame = [button frame]; + buttonFrame.origin.y -= bookmarks::kBookmarkBarHeight; + [button setFrame:buttonFrame]; + } + // Search for and adjust submenus, if necessary. + NSInteger buttonCount = [buttons_ count]; + BookmarkButton* subButton = [folderController_ parentButton]; + for (NSInteger i = buttonIndex; i < buttonCount; ++i) { + BookmarkButton* aButton = [buttons_ objectAtIndex:i]; + // If this button is showing its menu then we need to move the menu, too. + if (aButton == subButton) + [folderController_ offsetFolderMenuWindow:NSMakeSize(0.0, + bookmarks::kBookmarkBarHeight)]; + } + + // Resize the window and the main view (using sizers does not work). + NSRect frame = [window frame]; + frame.origin.y += bookmarks::kBookmarkBarHeight; + frame.size.height -= bookmarks::kBookmarkBarHeight; + [window setFrame:frame display:YES]; + frame = [mainView_ frame]; + frame.origin.y += bookmarks::kBookmarkBarHeight; + frame.size.height -= bookmarks::kBookmarkBarHeight; + [mainView_ setFrame:frame]; +} + +- (id<BookmarkButtonControllerProtocol>)controllerForNode: + (const BookmarkNode*)node { + // See if we are holding this node, otherwise see if it is in our + // hierarchy of visible folder menus. + if ([parentButton_ bookmarkNode] == node) + return self; + return [folderController_ controllerForNode:node]; +} + // Start a "scroll up" timer. - (void)beginScrollWindowUp { [self addScrollTimerWithDelta:kBookmarkBarFolderScrollAmount]; @@ -381,10 +550,12 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; // view that contains the button. It appears that a mouseExited: // gets lost, so the button stays highlit forever. We accomodate // here. +#if 0 if (buttonThatMouseIsIn_) { [[buttonThatMouseIsIn_ cell] setShowsBorderOnlyWhileMouseInside:NO]; [[buttonThatMouseIsIn_ cell] setShowsBorderOnlyWhileMouseInside:YES]; } +#endif // We update the window size after shifting the scroll to avoid a race. CGFloat screenHeightMinusMargin = (NSHeight(screenFrame) - @@ -540,6 +711,9 @@ const CGFloat kScrollWindowVerticalMargin = 0.0; - (void)fillPasteboard:(NSPasteboard*)pboard forDragOfButton:(BookmarkButton*)button { [[self folderTarget] fillPasteboard:pboard forDragOfButton:button]; + + // Close our folder menu and submenus since we know we're going to be dragged. + [self closeBookmarkFolder:self]; } // Find something like std::is_between<T>? I can't believe one doesn't exist. @@ -588,6 +762,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { return nil; } +// TODO(jrg): Refactor BookmarkBarFolder common code. http://crbug.com/35966 // Most of the work (e.g. drop indicator) is taken care of in the // folder_view. Here we handle hover open issues for subfolders. // Caution: there are subtle differences between this one and @@ -596,6 +771,19 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { NSPoint currentLocation = [info draggingLocation]; BookmarkButton* button = [self buttonForDroppingOnAtPoint:currentLocation]; + // Don't allow drops that would result in cycles. + if (button) { + NSData* data = [[info draggingPasteboard] + dataForType:kBookmarkButtonDragType]; + if (data && [info draggingSource]) { + BookmarkButton* sourceButton = nil; + [data getBytes:&sourceButton length:sizeof(sourceButton)]; + const BookmarkNode* sourceNode = [sourceButton bookmarkNode]; + const BookmarkNode* destNode = [button bookmarkNode]; + if (destNode->HasAncestor(sourceNode)) + button = nil; + } + } // Delegate handling of dragging over a button to the |hoverState_| member. return [hoverState_ draggingEnteredButton:button]; } @@ -698,17 +886,19 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { destIndex += [[parentButton_ cell] startingChildIndex]; } - if (copy) - [self bookmarkModel]->Copy(sourceNode, destParent, destIndex); - else - [self bookmarkModel]->Move(sourceNode, destParent, destIndex); - - [self closeAllBookmarkFolders]; // For a hover open, if needed. - - // Movement of a node triggers observers (like us) to rebuild the - // bar so we don't have to do so explicitly. + // Prevent cycles. + BOOL wasCopiedOrMoved = NO; + if (!destParent->HasAncestor(sourceNode)) { + if (copy) + [self bookmarkModel]->Copy(sourceNode, destParent, destIndex); + else + [self bookmarkModel]->Move(sourceNode, destParent, destIndex); + wasCopiedOrMoved = YES; + // Movement of a node triggers observers (like us) to rebuild the + // bar so we don't have to do so explicitly. + } - return YES; + return wasCopiedOrMoved; } // Return YES if we should show the drop indicator, else NO. @@ -844,14 +1034,22 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { } } +- (void)reconfigureMenu { + for (BookmarkButton* button in buttons_.get()) + [button removeFromSuperview]; + [buttons_ removeAllObjects]; + [self configureWindow]; +} + #pragma mark Methods Forwarded to BookmarkBarController - (IBAction)cutBookmark:(id)sender { + [self closeBookmarkFolder:self]; [barController_ cutBookmark:sender]; } - (IBAction)copyBookmark:(id)sender { - [barController_ cutBookmark:sender]; + [barController_ copyBookmark:sender]; } - (IBAction)pasteBookmark:(id)sender { @@ -859,6 +1057,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) { } - (IBAction)deleteBookmark:(id)sender { + [self closeBookmarkFolder:self]; [barController_ deleteBookmark:sender]; } diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm index a0f8d8e..65b0c90 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_controller_unittest.mm @@ -6,11 +6,15 @@ #include "base/basictypes.h" #include "base/scoped_nsobject.h" +#import "chrome/browser/cocoa/bookmark_bar_constants.h" // namespace bookmarks #import "chrome/browser/cocoa/bookmark_bar_controller.h" #import "chrome/browser/cocoa/bookmark_bar_folder_button_cell.h" #import "chrome/browser/cocoa/bookmark_bar_folder_controller.h" +#import "chrome/browser/cocoa/bookmark_bar_unittest_helper.h" #include "chrome/browser/cocoa/browser_test_helper.h" #import "chrome/browser/cocoa/cocoa_test_helper.h" +#import "chrome/browser/cocoa/view_resizer_pong.h" +#include "chrome/test/model_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" @@ -222,7 +226,8 @@ TEST_F(BookmarkBarFolderControllerTest, Position) { [bbfc2 window]; [bbfc2 setRealTopLeft:YES]; pt = [bbfc2 windowTopLeft]; - EXPECT_EQ(pt.x, NSMaxX([[bbfc.get() window] frame])); + // We're now overlapping the window a bit. + EXPECT_EQ(pt.x, NSMaxX([[bbfc.get() window] frame]) - 1); } TEST_F(BookmarkBarFolderControllerTest, DropDestination) { @@ -366,6 +371,573 @@ TEST_F(BookmarkBarFolderControllerTest, SimpleScroll) { } } +class BookmarkBarFolderControllerDragDropTest : public CocoaTest { + public: + BrowserTestHelper helper_; + scoped_nsobject<NSView> parent_view_; + scoped_nsobject<ViewResizerPong> resizeDelegate_; + scoped_nsobject<BookmarkBarController> bar_; + + BookmarkBarFolderControllerDragDropTest() { + resizeDelegate_.reset([[ViewResizerPong alloc] init]); + NSRect parent_frame = NSMakeRect(0, 0, 800, 50); + parent_view_.reset([[NSView alloc] initWithFrame:parent_frame]); + [parent_view_ setHidden:YES]; + bar_.reset([[BookmarkBarController alloc] + initWithBrowser:helper_.browser() + initialWidth:NSWidth(parent_frame) + delegate:nil + resizeDelegate:resizeDelegate_.get()]); + InstallAndToggleBar(bar_.get()); + } + + void InstallAndToggleBar(BookmarkBarController* bar) { + // Force loading of the nib. + [bar view]; + // Awkwardness to look like we've been installed. + [parent_view_ addSubview:[bar view]]; + NSRect frame = [[[bar view] superview] frame]; + frame.origin.y = 100; + [[[bar view] superview] setFrame:frame]; + + // Make sure it's on in a window so viewDidMoveToWindow is called + [[test_window() contentView] addSubview:parent_view_]; + + // Make sure it's open so certain things aren't no-ops. + [bar updateAndShowNormalBar:YES + showDetachedBar:NO + withAnimation:NO]; + } +}; + +#if CGFLOAT_IS_DOUBLE +#define CGFLOAT_EPSILON DBL_EPSILON +#else +#define CGFLOAT_EPSILON FLT_EPSILON +#endif + +#define CGFLOAT_EQ(expected, actual) \ + (actual >= (expected - CGFLOAT_EPSILON) && \ + actual <= (expected + CGFLOAT_EPSILON)) + +#define EXPECT_CGFLOAT_EQ(expected, actual) \ + EXPECT_TRUE(CGFLOAT_EQ(actual, expected) + +#define EXPECT_NSRECT_EQ(expected, actual) \ + EXPECT_TRUE(CGFLOAT_EQ(expected.origin.x, actual.origin.x) && \ + CGFLOAT_EQ(expected.origin.y, actual.origin.y) && \ + CGFLOAT_EQ(expected.size.width, actual.size.width) && \ + CGFLOAT_EQ(expected.size.height, actual.size.height)) << \ + "Rects do not match: " << \ + [NSStringFromRect(expected) UTF8String] << \ + " != " << [NSStringFromRect(actual) UTF8String]; + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragMoveBarBookmarkToFolder) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu and drag in a button from the bar. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"2f"]; + NSRect oldToFolderFrame = [toFolder frame]; + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + NSRect oldToWindowFrame = [toWindow frame]; + // Drag a bar button onto a bookmark (i.e. not a folder) in a folder + // so it should end up below the target bookmark. + BookmarkButton* draggedButton = [bar_ buttonWithTitleEqualTo:@"1b"]; + ASSERT_TRUE(draggedButton); + CGFloat horizontalShift = + NSWidth([draggedButton frame]) + bookmarks::kBookmarkHorizontalPadding; + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"2f1b"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton center] + copy:NO]; + // The button should have landed just after "2f1b". + const std::wstring expected_string(L"2f:[ 2f1b 1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ " + "4f2f1b 4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + EXPECT_EQ(expected_string, model_test_utils::ModelStringFromNode(root)); + + // Verify the window still appears by looking for its controller. + EXPECT_TRUE([bar_ folderController]); + + // Gather the new frames. + NSRect newToFolderFrame = [toFolder frame]; + NSRect newToWindowFrame = [toWindow frame]; + // The toFolder should have shifted left horizontally but not vertically. + NSRect expectedToFolderFrame = + NSOffsetRect(oldToFolderFrame, -horizontalShift, 0); + EXPECT_NSRECT_EQ(expectedToFolderFrame, newToFolderFrame); + // The toWindow should have shifted left horizontally, down vertically, + // and grown vertically. + NSRect expectedToWindowFrame = oldToWindowFrame; + expectedToWindowFrame.origin.x -= horizontalShift; + expectedToWindowFrame.origin.y -= bookmarks::kBookmarkBarHeight; + expectedToWindowFrame.size.height += bookmarks::kBookmarkBarHeight; + EXPECT_NSRECT_EQ(expectedToWindowFrame, newToWindowFrame); + + // Move the button back to the bar at the beginning. + draggedButton = [folderController buttonWithTitleEqualTo:@"1b"]; + ASSERT_TRUE(draggedButton); + targetButton = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(targetButton); + [bar_ dragButton:draggedButton + to:[targetButton left] + copy:NO]; + EXPECT_EQ(model_string, model_test_utils::ModelStringFromNode(root)); + // Don't check the folder window since it's not supposed to be showing. +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragCopyBarBookmarkToFolder) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu and copy in a button from the bar. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(toFolder); + NSRect oldToFolderFrame = [toFolder frame]; + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + NSRect oldToWindowFrame = [toWindow frame]; + // Drag a bar button onto a bookmark (i.e. not a folder) in a folder + // so it should end up below the target bookmark. + BookmarkButton* draggedButton = [bar_ buttonWithTitleEqualTo:@"1b"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"2f1b"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton center] + copy:YES]; + // The button should have landed just after "2f1b". + const std::wstring expected_1(L"1b 2f:[ 2f1b 1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ " + "4f2f1b 4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + EXPECT_EQ(expected_1, model_test_utils::ModelStringFromNode(root)); + + // Gather the new frames. + NSRect newToFolderFrame = [toFolder frame]; + NSRect newToWindowFrame = [toWindow frame]; + // The toFolder should have shifted. + EXPECT_NSRECT_EQ(oldToFolderFrame, newToFolderFrame); + // The toWindow should have shifted down vertically and grown vertically. + NSRect expectedToWindowFrame = oldToWindowFrame; + expectedToWindowFrame.origin.y -= bookmarks::kBookmarkBarHeight; + expectedToWindowFrame.size.height += bookmarks::kBookmarkBarHeight; + EXPECT_NSRECT_EQ(expectedToWindowFrame, newToWindowFrame); + + // Copy the button back to the bar after "3b". + draggedButton = [folderController buttonWithTitleEqualTo:@"1b"]; + ASSERT_TRUE(draggedButton); + targetButton = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(targetButton); + [bar_ dragButton:draggedButton + to:[targetButton left] + copy:YES]; + const std::wstring expected_2(L"1b 2f:[ 2f1b 1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 1b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ " + "4f2f1b 4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + EXPECT_EQ(expected_2, model_test_utils::ModelStringFromNode(root)); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, + DragMoveBarBookmarkToSubfolder) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu and a subfolder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + NSRect oldToWindowFrame = [toWindow frame]; + BookmarkButton* toSubfolder = [folderController buttonWithTitleEqualTo:@"4f2f"]; + ASSERT_TRUE(toSubfolder); + NSRect oldToSubfolderFrame = [toSubfolder frame]; + [[toSubfolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toSubfolder]; + BookmarkBarFolderController* subfolderController = + [folderController folderController]; + EXPECT_TRUE(subfolderController); + NSWindow* toSubwindow = [subfolderController window]; + EXPECT_TRUE(toSubwindow); + NSRect oldToSubwindowFrame = [toSubwindow frame]; + // Drag a bar button onto a bookmark (i.e. not a folder) in a folder + // so it should end up below the target bookmark. + BookmarkButton* draggedButton = [bar_ buttonWithTitleEqualTo:@"5b"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = + [subfolderController buttonWithTitleEqualTo:@"4f2f3b"]; + ASSERT_TRUE(targetButton); + [subfolderController dragButton:draggedButton + to:[targetButton center] + copy:NO]; + // The button should have landed just after "2f". + const std::wstring expected_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ " + "4f2f1b 4f2f2b 4f2f3b 5b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] "); + EXPECT_EQ(expected_string, model_test_utils::ModelStringFromNode(root)); + + // Check the window layouts. The folder window should not have changed, + // but the subfolder window should have shifted vertically and grown. + NSRect newToWindowFrame = [toWindow frame]; + EXPECT_NSRECT_EQ(oldToWindowFrame, newToWindowFrame); + NSRect newToSubwindowFrame = [toSubwindow frame]; + NSRect expectedToSubwindowFrame = oldToSubwindowFrame; + expectedToSubwindowFrame.origin.y -= bookmarks::kBookmarkBarHeight; + expectedToSubwindowFrame.size.height += bookmarks::kBookmarkBarHeight; + EXPECT_NSRECT_EQ(expectedToSubwindowFrame, newToSubwindowFrame); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragMoveWithinFolder) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + NSRect oldToWindowFrame = [toWindow frame]; + // Drag a folder button to the top within the same parent. + BookmarkButton* draggedButton = [folderController buttonWithTitleEqualTo:@"4f2f"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"4f1f"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton top] + copy:NO]; + // The button should have landed above "4f1f". + const std::wstring expected_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b " + "2f2f2b 2f2f3b ] 2f3b ] 3b 4f:[ 4f2f:[ 4f2f1b 4f2f2b 4f2f3b ] " + "4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + EXPECT_EQ(expected_string, model_test_utils::ModelStringFromNode(root)); + + // The window should not have gone away. + EXPECT_TRUE([bar_ folderController]); + + // The folder window should not have changed. + NSRect newToWindowFrame = [toWindow frame]; + EXPECT_NSRECT_EQ(oldToWindowFrame, newToWindowFrame); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragParentOntoChild) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + // Drag a folder button to one of its children. + BookmarkButton* draggedButton = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"4f3f"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton top] + copy:NO]; + // The model should not have changed. + EXPECT_EQ(model_string, model_test_utils::ModelStringFromNode(root)); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragMoveChildToParent) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f:[ 4f2f1b " + "4f2f2b 4f2f3b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu and a subfolder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"4f"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + BookmarkButton* toSubfolder = [folderController buttonWithTitleEqualTo:@"4f2f"]; + ASSERT_TRUE(toSubfolder); + [[toSubfolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toSubfolder]; + BookmarkBarFolderController* subfolderController = + [folderController folderController]; + EXPECT_TRUE(subfolderController); + + // Drag a subfolder bookmark to the parent folder. + BookmarkButton* draggedButton = + [subfolderController buttonWithTitleEqualTo:@"4f2f3b"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"4f2f"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton top] + copy:NO]; + // The button should have landed above "4f2f". + const std::wstring expected_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b " + "2f2f3b ] 2f3b ] 3b 4f:[ 4f1f:[ 4f1f1b 4f1f2b 4f1f3b ] 4f2f3b 4f2f:[ " + "4f2f1b 4f2f2b ] 4f3f:[ 4f3f1b 4f3f2b 4f3f3b ] ] 5b "); + EXPECT_EQ(expected_string, model_test_utils::ModelStringFromNode(root)); + + // The window should not have gone away. + EXPECT_TRUE([bar_ folderController]); + // The subfolder should have gone away. + EXPECT_FALSE([folderController folderController]); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, DragWindowResizing) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring + model_string(L"a b:[ b1 b2 b3 ] reallyReallyLongBookmarkName c "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"b"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folderController = [bar_ folderController]; + EXPECT_TRUE(folderController); + NSWindow* toWindow = [folderController window]; + EXPECT_TRUE(toWindow); + CGFloat oldWidth = NSWidth([toWindow frame]); + // Drag the bookmark with a long name to the folder. + BookmarkButton* draggedButton = + [bar_ buttonWithTitleEqualTo:@"reallyReallyLongBookmarkName"]; + ASSERT_TRUE(draggedButton); + BookmarkButton* targetButton = [folderController buttonWithTitleEqualTo:@"b1"]; + ASSERT_TRUE(targetButton); + [folderController dragButton:draggedButton + to:[targetButton center] + copy:NO]; + // Verify the model change. + const std::wstring + expected_string(L"a b:[ b1 reallyReallyLongBookmarkName b2 b3 ] c "); + EXPECT_EQ(expected_string, model_test_utils::ModelStringFromNode(root)); + // Verify the window grew. Just test a reasonable width gain. + CGFloat newWidth = NSWidth([toWindow frame]); + EXPECT_LT(oldWidth + 30.0, newWidth); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, MoveRemoveAddButtons) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b 2f3b ] 3b 4b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Pop up a folder menu. + BookmarkButton* toFolder = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(toFolder); + [[toFolder target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:toFolder]; + BookmarkBarFolderController* folder = [bar_ folderController]; + EXPECT_TRUE(folder); + + // Remember how many buttons are showing. + NSArray* buttons = [folder buttons]; + NSUInteger oldDisplayedButtons = [buttons count]; + + // Move a button around a bit. + [folder moveButtonFromIndex:0 toIndex:2]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f2b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f1b"]); + EXPECT_EQ(oldDisplayedButtons, [buttons count]); + [folder moveButtonFromIndex:2 toIndex:0]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f2b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f3b"]); + EXPECT_EQ(oldDisplayedButtons, [buttons count]); + + // Add a couple of buttons. + const BookmarkNode* node = root->GetChild(2); // Purloin an existing node. + [folder addButtonForNode:node atIndex:0]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f2b"]); + EXPECT_TRUE([[[buttons objectAtIndex:3] title] isEqualToString:@"2f3b"]); + EXPECT_EQ(oldDisplayedButtons + 1, [buttons count]); + node = root->GetChild(3); + [folder addButtonForNode:node atIndex:-1]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f1b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f2b"]); + EXPECT_TRUE([[[buttons objectAtIndex:3] title] isEqualToString:@"2f3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:4] title] isEqualToString:@"4b"]); + EXPECT_EQ(oldDisplayedButtons + 2, [buttons count]); + + // Remove a couple of buttons. + [folder removeButton:4 animate:NO]; + [folder removeButton:1 animate:NO]; + EXPECT_TRUE([[[buttons objectAtIndex:0] title] isEqualToString:@"3b"]); + EXPECT_TRUE([[[buttons objectAtIndex:1] title] isEqualToString:@"2f2b"]); + EXPECT_TRUE([[[buttons objectAtIndex:2] title] isEqualToString:@"2f3b"]); + EXPECT_EQ(oldDisplayedButtons, [buttons count]); +} + +TEST_F(BookmarkBarFolderControllerDragDropTest, ControllerForNode) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b ] 3b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Find the main bar controller. + const void* expectedController = bar_; + const void* actualController = [bar_ controllerForNode:root]; + EXPECT_EQ(expectedController, actualController); + + // Pop up the folder menu. + BookmarkButton* targetFolder = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(targetFolder); + [[targetFolder target] + performSelector:@selector(openBookmarkFolderFromButton:) + withObject:targetFolder]; + BookmarkBarFolderController* folder = [bar_ folderController]; + EXPECT_TRUE(folder); + + // Find the folder controller using the folder controller. + const BookmarkNode* targetNode = root->GetChild(1); + expectedController = folder; + actualController = [bar_ controllerForNode:targetNode]; + EXPECT_EQ(expectedController, actualController); + + // Find the folder controller from the bar. + actualController = [folder controllerForNode:targetNode]; + EXPECT_EQ(expectedController, actualController); +} + +@interface BookmarkBarControllerNoDelete : BookmarkBarController +- (IBAction)deleteBookmark:(id)sender; +@end + +@implementation BookmarkBarControllerNoDelete +- (IBAction)deleteBookmark:(id)sender { + // NOP +} +@end + +class BookmarkBarFolderControllerClosingTest : public + BookmarkBarFolderControllerDragDropTest { + public: + BookmarkBarFolderControllerClosingTest() { + bar_.reset([[BookmarkBarControllerNoDelete alloc] + initWithBrowser:helper_.browser() + initialWidth:NSWidth([parent_view_ frame]) + delegate:nil + resizeDelegate:resizeDelegate_.get()]); + InstallAndToggleBar(bar_.get()); + } +}; + +TEST_F(BookmarkBarFolderControllerClosingTest, DeleteClosesFolder) { + BookmarkModel& model(*helper_.profile()->GetBookmarkModel()); + const BookmarkNode* root = model.GetBookmarkBarNode(); + const std::wstring model_string(L"1b 2f:[ 2f1b 2f2f:[ 2f2f1b 2f2f2b ] " + "2f3b ] 3b "); + model_test_utils::AddNodesFromModelString(model, root, model_string); + + // Validate initial model. + std::wstring actualModelString = model_test_utils::ModelStringFromNode(root); + EXPECT_EQ(model_string, actualModelString); + + // Open the folder menu and submenu. + BookmarkButton* target = [bar_ buttonWithTitleEqualTo:@"2f"]; + ASSERT_TRUE(target); + [[target target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:target]; + BookmarkBarFolderController* folder = [bar_ folderController]; + EXPECT_TRUE(folder); + BookmarkButton* subTarget = [folder buttonWithTitleEqualTo:@"2f2f"]; + ASSERT_TRUE(subTarget); + [[subTarget target] performSelector:@selector(openBookmarkFolderFromButton:) + withObject:subTarget]; + BookmarkBarFolderController* subFolder = [folder folderController]; + EXPECT_TRUE(subFolder); + + // Delete the folder node and verify the window closed down by looking + // for its controller again. + [folder deleteBookmark:folder]; + EXPECT_FALSE([folder folderController]); +} + // TODO(jrg): draggingEntered: and draggingExited: trigger timers so // they are hard to test. Factor out "fire timers" into routines // which can be overridden to fire immediately to make behavior diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view.mm b/chrome/browser/cocoa/bookmark_bar_folder_view.mm index d712944..15a4db6 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_view.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_view.mm @@ -22,7 +22,6 @@ } - (void)drawRect:(NSRect)rect { - // TODO(jrg): copied from bookmark_bar_view but orientation changed. // Code dup sucks but I'm not sure I can take 16 lines and make it // generic for horiz vs vertical while keeping things simple. @@ -105,10 +104,6 @@ // Awkwardness since views open and close out from under us. if (inDrag_) { inDrag_ = NO; - - // This line makes sure menus get closed when a drag isn't - // completed. - [[self controller] closeAllBookmarkFolders]; } [self draggingExited:info]; diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm index 96189f5..83472af 100644 --- a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm +++ b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm @@ -122,6 +122,21 @@ disposition:(WindowOpenDisposition)disposition { } +- (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)animate { +} + +- (id<BookmarkButtonControllerProtocol>)controllerForNode: + (const BookmarkNode*)node { + return nil; +} + +- (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex { +} + +- (void)addButtonForNode:(const BookmarkNode*)node + atIndex:(NSInteger)buttonIndex { +} + @end namespace { diff --git a/chrome/browser/cocoa/bookmark_bar_unittest_helper.h b/chrome/browser/cocoa/bookmark_bar_unittest_helper.h new file mode 100644 index 0000000..402f8a1 --- /dev/null +++ b/chrome/browser/cocoa/bookmark_bar_unittest_helper.h @@ -0,0 +1,48 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_BROWSER_COCOA_AUTOCOMPLETE_BOOKMARK_BAR_UNITTEST_HELPER_H_ +#define CHROME_BROWSER_COCOA_AUTOCOMPLETE_BOOKMARK_BAR_UNITTEST_HELPER_H_ + +#import <Foundation/Foundation.h> + +#import "chrome/browser/cocoa/bookmark_bar_controller.h" +#import "chrome/browser/cocoa/bookmark_bar_folder_controller.h" +#import "chrome/browser/cocoa/bookmark_button.h" + +@interface BookmarkBarController (BookmarkBarUnitTestHelper) + +// Return the bookmark button from this bar controller with the given +// |title|, otherwise nil. This does not recurse into folders. +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title; + +@end + + +@interface BookmarkBarFolderController (BookmarkBarUnitTestHelper) + +// Return the bookmark button from this folder controller with the given +// |title|, otherwise nil. This does not recurse into subfolders. +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title; + +@end + + +@interface BookmarkButton (BookmarkBarUnitTestHelper) + +// Return the center of the button in the base coordinate system of the +// containing window. Useful for simulating mouse clicks or drags. +- (NSPoint)center; + +// Return the top of the button in the base coordinate system of the +// containing window. +- (NSPoint)top; + +// Return the center-left point of the button in the base coordinate system +// of the containing window. +- (NSPoint)left; + +@end + +#endif // CHROME_BROWSER_COCOA_AUTOCOMPLETE_TEXT_FIELD_UNITTEST_HELPER_H_ diff --git a/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm b/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm new file mode 100644 index 0000000..114197e --- /dev/null +++ b/chrome/browser/cocoa/bookmark_bar_unittest_helper.mm @@ -0,0 +1,67 @@ +// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#import "chrome/browser/cocoa/bookmark_bar_unittest_helper.h" + +@interface NSArray (BookmarkBarUnitTestHelper) + +// A helper function for scanning an array of buttons looking for the +// button with the given |title|. +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title; + +@end + + +@implementation NSArray (BookmarkBarUnitTestHelper) + +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title { + for (BookmarkButton* button in self) { + if ([[button title] isEqualToString:title]) + return button; + } + return nil; +} + +@end + +@implementation BookmarkBarController (BookmarkBarUnitTestHelper) + +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title { + return [[self buttons] buttonWithTitleEqualTo:title]; +} + +@end + +@implementation BookmarkBarFolderController(BookmarkBarUnitTestHelper) + +- (BookmarkButton*)buttonWithTitleEqualTo:(NSString*)title { + return [[self buttons] buttonWithTitleEqualTo:title]; +} + +@end + +@implementation BookmarkButton(BookmarkBarUnitTestHelper) + +- (NSPoint)center { + NSRect frame = [self frame]; + NSPoint center = NSMakePoint(NSMidX(frame), NSMidY(frame)); + center = [[self superview] convertPoint:center toView:nil]; + return center; +} + +- (NSPoint)top { + NSRect frame = [self frame]; + NSPoint top = NSMakePoint(NSMidX(frame), NSMaxY(frame)); + top = [[self superview] convertPoint:top toView:nil]; + return top; +} + +- (NSPoint)left { + NSRect frame = [self frame]; + NSPoint left = NSMakePoint(NSMinX(frame), NSMidY(frame)); + left = [[self superview] convertPoint:left toView:nil]; + return left; +} + +@end diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h index cb62053..62acadc 100644 --- a/chrome/browser/cocoa/bookmark_button.h +++ b/chrome/browser/cocoa/bookmark_button.h @@ -46,6 +46,7 @@ class ThemeProvider; // or NSWindowController. The BookmarkButton doesn't use this // protocol directly; it is used when BookmarkButton controllers talk // to each other. +// // Other than the top level owner (the bookmark bar), all bookmark // button controllers have a parent controller. @protocol BookmarkButtonControllerProtocol @@ -116,6 +117,35 @@ class ThemeProvider; - (void)openAll:(const BookmarkNode*)node disposition:(WindowOpenDisposition)disposition; +// There are several operations which may affect the contents of a bookmark +// button controller after it has been created, primary of which are +// cut/paste/delete and drag/drop. Such changes may involve coordinating +// the bookmark button contents of two controllers (such as when a bookmark is +// dragged from one folder to another). The bookmark bar controller +// coordinates in response to notifications propogated by the bookmark model +// through BookmarkBarBridge calls. The following three functions are +// implemented by the controllers and are dispatched by the bookmark bar +// controller in response to notifications coming in from the BookmarkBarBridge. + +// Add a button for the given node to the bar or folder menu. This is safe +// to call when a folder menu window is open as that window will be updated. +// And index of -1 means to append to the end (bottom). +- (void)addButtonForNode:(const BookmarkNode*)node + atIndex:(NSInteger)buttonIndex; + +// Move a button from one place in the menu to another. This is safe +// to call when a folder menu window is open as that window will be updated. +- (void)moveButtonFromIndex:(NSInteger)fromIndex toIndex:(NSInteger)toIndex; + +// Remove the bookmark button at the given index. Show the poof animation +// if |animate:| is YES. It may be obvious, but this is safe +// to call when a folder menu window is open as that window will be updated. +- (void)removeButton:(NSInteger)buttonIndex animate:(BOOL)poof; + +// Determine the controller containing the button representing |node|, if any. +- (id<BookmarkButtonControllerProtocol>)controllerForNode: + (const BookmarkNode*)node; + @end // @protocol BookmarkButtonControllerProtocol diff --git a/chrome/browser/cocoa/bookmark_button.mm b/chrome/browser/cocoa/bookmark_button.mm index fea9d25..00a6b77 100644 --- a/chrome/browser/cocoa/bookmark_button.mm +++ b/chrome/browser/cocoa/bookmark_button.mm @@ -53,7 +53,6 @@ static const CGFloat kDragImageOpacity = 0.7; if ([self delegate]) { // Ask our delegate to fill the pasteboard for us. NSPasteboard* pboard = [NSPasteboard pasteboardWithName:NSDragPboard]; - [[self delegate] fillPasteboard:pboard forDragOfButton:self]; // At the moment, moving bookmarks causes their buttons (like me!) diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 1087624..3b4cc66 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -128,6 +128,8 @@ 'test/chrome_process_util.cc', 'test/chrome_process_util.h', 'test/chrome_process_util_mac.cc', + 'test/model_test_utils.cc', + 'test/model_test_utils.h', 'test/profile_mock.h', 'test/test_browser_window.h', 'test/test_location_bar.h', @@ -637,6 +639,8 @@ 'browser/cocoa/bookmark_bar_folder_view_unittest.mm', 'browser/cocoa/bookmark_bar_folder_window_unittest.mm', 'browser/cocoa/bookmark_bar_toolbar_view_unittest.mm', + 'browser/cocoa/bookmark_bar_unittest_helper.h', + 'browser/cocoa/bookmark_bar_unittest_helper.mm', 'browser/cocoa/bookmark_bar_view_unittest.mm', 'browser/cocoa/bookmark_bubble_controller_unittest.mm', 'browser/cocoa/bookmark_button_unittest.mm', diff --git a/chrome/test/model_test_utils.cc b/chrome/test/model_test_utils.cc new file mode 100644 index 0000000..218cfd8 --- /dev/null +++ b/chrome/test/model_test_utils.cc @@ -0,0 +1,82 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "model_test_utils.h" + +#include "chrome/browser/bookmarks/bookmark_model.h" +#include "googleurl/src/gurl.h" + +namespace model_test_utils { + +std::wstring ModelStringFromNode(const BookmarkNode* node) { + // Since the children of the node are not available as a vector, + // we'll just have to do it the hard way. + int child_count = node->GetChildCount(); + std::wstring child_string; + for (int i = 0; i < child_count; ++i) { + const BookmarkNode* child = node->GetChild(i); + if (child->is_folder()) { + child_string += child->GetTitle() + L":[ " + ModelStringFromNode(child) + + L"] "; + } else { + child_string += child->GetTitle() + L" "; + } + } + return child_string; +} + +// Helper function which does the actual work of creating the nodes for +// a particular level in the hierarchy. +std::wstring::size_type AddNodesFromString(BookmarkModel& model, + const BookmarkNode* node, + const std::wstring& model_string, + std::wstring::size_type start_pos) { + DCHECK(node); + int index = node->GetChildCount(); + static const std::wstring folder_tell(L":["); + std::wstring::size_type end_pos = model_string.find(' ', start_pos); + while (end_pos != std::wstring::npos) { + std::wstring::size_type part_length = end_pos - start_pos; + std::wstring node_name = model_string.substr(start_pos, part_length); + // Are we at the end of a folder group? + if (node_name != L"]") { + // No, is it a folder? + std::wstring tell; + if (part_length > 2) + tell = node_name.substr(part_length - 2, 2); + if (tell == folder_tell) { + node_name = node_name.substr(0, part_length - 2); + const BookmarkNode* new_node = model.AddGroup(node, index, node_name); + end_pos = AddNodesFromString(model, new_node, model_string, + end_pos + 1); + } else { + std::string url_string("http://"); + url_string += std::string(node_name.begin(), node_name.end()); + url_string += ".com"; + model.AddURL(node, index, node_name, GURL(url_string)); + ++end_pos; + } + ++index; + start_pos = end_pos; + end_pos = model_string.find(' ', start_pos); + } else { + ++end_pos; + break; + } + } + return end_pos; +} + +void AddNodesFromModelString(BookmarkModel& model, + const BookmarkNode* node, + const std::wstring& model_string) { + DCHECK(node); + const std::wstring folder_tell(L":["); + std::wstring::size_type start_pos = 0; + std::wstring::size_type end_pos = + AddNodesFromString(model, node, model_string, start_pos); + DCHECK(end_pos == std::wstring::npos); +} + +} // namespace model_test_utils diff --git a/chrome/test/model_test_utils.h b/chrome/test/model_test_utils.h new file mode 100644 index 0000000..1d5c3c6 --- /dev/null +++ b/chrome/test/model_test_utils.h @@ -0,0 +1,40 @@ +// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_TEST_MODEL_TEST_UTILS_H_ +#define CHROME_TEST_MODEL_TEST_UTILS_H_ + +#include <string> + +class BookmarkModel; +class BookmarkNode; + +namespace model_test_utils { + +// Return the descendants of |node| as a string useful for verifying node +// modifications. The format of the resulting string is: +// +// result = node " " , { node " " } +// node = bookmark title | folder +// folder = folder title ":[ " { node " " } "]" +// bookmark title = (* string with no spaces *) +// folder title = (* string with no spaces *) +// +// Example: "a f1:[ b d c ] d f2:[ e f g ] h " +std::wstring ModelStringFromNode(const BookmarkNode* node); + +// Create and add the node hierarchy specified by |nodeString| to the +// bookmark node given by |node|. The string has the same format as +// specified for ModelStringFromNode. The new nodes added to |node| +// are appended to the end of node's existing subnodes, if any. +// |model| must be the model of which |node| is a member. +// NOTE: The string format is very rigid and easily broken if not followed +// exactly (since we're using a very simple parser). +void AddNodesFromModelString(BookmarkModel& model, + const BookmarkNode* node, + const std::wstring& model_string); + +} // namespace model_test_utils + +#endif // CHROME_TEST_MODEL_TEST_UTILS_H_ |