Mousing over the bookmark bar now behaves properly when traversing a non-folder item; this would previously cause folder menus to discontinue appearing. Now once the bar folder menus are showing they will continue showing even when a non-folder bookmark bar item is traversed. Folder menus will discontinue showing when 1) a bar folder button is clicked, 2) a bookmark choice is made, 3) a click is made somewhere else in the browser window, 4) when the window loses focus, etc.

BUG=None
TEST=1) Mouse over the bar without clicking and verify that no folder menus appear. 2) Click on a folder button in the bar and verify that its folder menu appears. 3) Mouse over other folders in the bar and verify that the old menu closes and a new one opens. 4) Mouse over a non-folder button in the bar and verify that the old menu closes and another does not appear. 5) Mouse over a folder button and verify that the folder's menu appears. 6) Click on a folder button in the bar and verify that the menu closes. 7) Mouse over other folder buttons and verify no menu appears.

Review URL: http://codereview.chromium.org/1912008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@46626 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 76 insertions, 40 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index 29952b8..a0ccd31 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -213,6 +213,13 @@ willAnimateFromState:(bookmarks::VisualState)oldState
   // that all bookmark buttons we create will be visible.  Thus,
   // [buttons_ count] isn't a definitive check.
   int displayedButtonCount_;
+
+  // A state flag which tracks when the bar's folder menus should be shown.
+  // An initial click in any of the folder buttons turns this on and
+  // one of the following will turn it off: another click in the button,
+  // the window losing focus, a click somewhere other than in the bar
+  // or a folder menu.
+  BOOL showFolderMenus_;
 }
 
 @property(readonly, nonatomic) bookmarks::VisualState visualState;
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index f862d6d..596b651 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -428,6 +428,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 
 // NSNotificationCenter callback.
 - (void)parentWindowDidResignKey:(NSNotification*)notification {
+  showFolderMenus_ = NO;
   [self closeAllBookmarkFolders];
 }
 
@@ -437,22 +438,25 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 // real menus).  We detect and act here.
 - (void)mouseEnteredButton:(id)sender event:(NSEvent*)event {
   DCHECK([sender isKindOfClass:[BookmarkButton class]]);
-  // If nothing is open, do nothing.  Different from
-  // BookmarkBarFolderController since we default to NOT being enabled.
-  if (!folderController_)
+
+  // If folder menus are not being shown, do nothing.  This is different from
+  // BookmarkBarFolderController's implementation because the bar should NOT
+  // automatically open folder menus when the mouse passes over a folder
+  // button while the BookmarkBarFolderController DOES automically open
+  // a subfolder menu.
+  if (!showFolderMenus_)
     return;
 
   // From here down: same logic as BookmarkBarFolderController.
   // TODO(jrg): find a way to share these 4 non-comment lines?
   // http://crbug.com/35966
-
   // If already opened, then we exited but re-entered the button, so do nothing.
   if ([folderController_ parentButton] == sender)
     return;
   // Else open a new one if it makes sense to do so.
-  if ([sender bookmarkNode]->is_folder())
+  if ([sender bookmarkNode]->is_folder()) {
     [folderTarget_ openBookmarkFolderFromButton:sender];
-  else {
+  } else {
     // We're over a non-folder bookmark so close any old folders.
     [folderController_ close];
     folderController_ = nil;
@@ -489,6 +493,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 // NOT an override of a standard Cocoa call made to NSViewControllers.
 - (void)hookForEvent:(NSEvent*)theEvent {
   if ([self isEventAnExitEvent:theEvent]) {
+    showFolderMenus_ = NO;
     [self watchForExitEvent:NO];
     [self closeAllBookmarkFolders];
   }
@@ -1140,6 +1145,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
 
 // Redirect to our logic shared with BookmarkBarFolderController.
 - (IBAction)openBookmarkFolderFromButton:(id)sender {
+  // Toggle presentation of bar folder menus.
+  showFolderMenus_ = !showFolderMenus_;
   [folderTarget_ openBookmarkFolderFromButton:sender];
 }
 
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 996e677..77b2397 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -26,18 +26,6 @@
 #include "testing/platform_test.h"
 #import "third_party/ocmock/OCMock/OCMock.h"
 
-// Add a redirect to make testing easier.
-@interface BookmarkBarController(MakeTestingEasier)
-- (IBAction)openBookmarkFolderFromButton:(id)sender;
-@end
-
-@implementation BookmarkBarController(MakeTestingEasier)
-- (IBAction)openBookmarkFolderFromButton:(id)sender {
-  [[self folderTarget] openBookmarkFolderFromButton:sender];
-}
-@end
-
-
 // Just like a BookmarkBarController but openURL: is stubbed out.
 @interface BookmarkBarControllerNoOpen : BookmarkBarController {
  @public
@@ -1092,27 +1080,6 @@ TEST_F(BookmarkBarControllerTest, TestFolders) {
   folder = model->AddGroup(parent, parent->GetChildCount(), L"empty");
 
   EXPECT_EQ([[bar_ buttons] count], 2U);
-  BookmarkButton* button = [[bar_ buttons] objectAtIndex:0];  // full one
-
-  EXPECT_FALSE([bar_ folderController]);
-  [bar_ openBookmarkFolderFromButton:button];
-  BookmarkBarFolderController* bbfc = [bar_ folderController];
-  EXPECT_TRUE(bbfc);
-
-  // Make sure a 2nd open on the same button closes things.
-  [bar_ openBookmarkFolderFromButton:button];
-  EXPECT_FALSE([bar_ folderController]);
-
-  // Next open is a different button.
-  [bar_ openBookmarkFolderFromButton:[[bar_ buttons] objectAtIndex:1]];
-  EXPECT_TRUE([bar_ folderController]);
-  EXPECT_NE(bbfc, [bar_ folderController]);
-
-  // Finally confirm a close removes the folder controller.
-  [bar_ closeBookmarkFolder:nil];
-  EXPECT_FALSE([bar_ folderController]);
-
-  // Next part of the test: similar actions but with mouseEntered/mouseExited.
 
   // First confirm mouseEntered does nothing if "menus" aren't active.
   NSEvent* event = test_event_utils::MakeMouseEvent(NSOtherMouseUp, 0);
@@ -1121,7 +1088,7 @@ TEST_F(BookmarkBarControllerTest, TestFolders) {
 
   // Make one active.  Entering it is now a no-op.
   [bar_ openBookmarkFolderFromButton:[[bar_ buttons] objectAtIndex:0]];
-  bbfc = [bar_ folderController];
+  BookmarkBarFolderController* bbfc = [bar_ folderController];
   EXPECT_TRUE(bbfc);
   [bar_ mouseEnteredButton:[[bar_ buttons] objectAtIndex:0] event:event];
   EXPECT_EQ(bbfc, [bar_ folderController]);
@@ -1138,6 +1105,61 @@ TEST_F(BookmarkBarControllerTest, TestFolders) {
   [bar_ closeBookmarkFolder:nil];
 }
 
+// Verify that the folder menu presentation properly tracks mouse movements
+// over the bar. Until there is a click no folder menus should show. After a
+// click on a folder folder menus should show until another click on a folder
+// button, and a click outside the bar and its folder menus.
+TEST_F(BookmarkBarControllerTest, TestFolderButtons) {
+  BookmarkModel& model(*helper_.profile()->GetBookmarkModel());
+  const BookmarkNode* root = model.GetBookmarkBarNode();
+  const std::wstring model_string(L"1b 2f:[ 2f1b 2f2b ] 3b 4f:[ 4f1b 4f2b ] ");
+  model_test_utils::AddNodesFromModelString(model, root, model_string);
+
+  // Validate initial model and that we do not have a folder controller.
+  std::wstring actualModelString = model_test_utils::ModelStringFromNode(root);
+  EXPECT_EQ(model_string, actualModelString);
+  EXPECT_FALSE([bar_ folderController]);
+
+  // Click on a folder button.
+  BookmarkButton* button = [bar_ buttonWithTitleEqualTo:@"4f"];
+  [bar_ openBookmarkFolderFromButton:button];
+  BookmarkBarFolderController* bbfc = [bar_ folderController];
+  EXPECT_TRUE(bbfc);
+
+  // Make sure a 2nd click on the same button closes things.
+  [bar_ openBookmarkFolderFromButton:button];
+  EXPECT_FALSE([bar_ folderController]);
+
+  // Next open is a different button.
+  button = [bar_ buttonWithTitleEqualTo:@"2f"];
+  [bar_ openBookmarkFolderFromButton:button];
+  EXPECT_TRUE([bar_ folderController]);
+
+  // Mouse over a non-folder button and confirm controller has gone away.
+  button = [bar_ buttonWithTitleEqualTo:@"1b"];
+  NSEvent* event = test_event_utils::MouseEventAtPoint([button center],
+                                                       NSMouseMoved, 0);
+  [bar_ mouseEnteredButton:button event:event];
+  EXPECT_FALSE([bar_ folderController]);
+
+  // Mouse over the original folder and confirm a new controller.
+  button = [bar_ buttonWithTitleEqualTo:@"2f"];
+  [bar_ mouseEnteredButton:button event:event];
+  BookmarkBarFolderController* oldBBFC = [bar_ folderController];
+  EXPECT_TRUE(oldBBFC);
+
+  // 'Jump' over to a different folder and confirm a new controller.
+  button = [bar_ buttonWithTitleEqualTo:@"4f"];
+  [bar_ mouseEnteredButton:button event:event];
+  BookmarkBarFolderController* newBBFC = [bar_ folderController];
+  EXPECT_TRUE(newBBFC);
+  EXPECT_NE(oldBBFC, newBBFC);
+
+  // Another click should close the folder menus.
+  [bar_ openBookmarkFolderFromButton:button];
+  EXPECT_FALSE([bar_ folderController]);
+}
+
 // Make sure the "off the side" folder looks like a bookmark folder
 // but only contains "off the side" items.
 TEST_F(BookmarkBarControllerTest, OffTheSideFolder) {