summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorshahriar.rostami <shahriar.rostami@gmail.com>2015-08-12 16:22:34 -0700
committerCommit bot <commit-bot@chromium.org>2015-08-12 23:41:10 +0000
commit7c613d863bb31a658561e9d52d68a65151217316 (patch)
treef9cb4795e251857386fd5f926b6cc11f217a4a18
parent9f603722c02171d1448754aaea96d2076f28773d (diff)
downloadchromium_src-7c613d863bb31a658561e9d52d68a65151217316.zip
chromium_src-7c613d863bb31a658561e9d52d68a65151217316.tar.gz
chromium_src-7c613d863bb31a658561e9d52d68a65151217316.tar.bz2
[Mac] Prevent unintended bookmark button hovering changes in scrolling with keyboard.
While we were scrolling bookmark bar menu with up/down keys (obviously with a large number of bookmarks in a given folder) mouseEnteredButton was fired due to the scrolling/resizing of the frame. Now, before scrolling we set a flag to distinguish incorrect false mouse events from real ones. The flag resets whenever a real mouse movement happens. BUG=504199 TEST=Follow these steps (taken from bug report): 1. Create a large number of bookmarks in the bookmarks bar. 2. While viewing the expanded menu, press the down arrow key repeatedly. Each time scroll occurs, the selection moves back to the mouse position. Review URL: https://codereview.chromium.org/1261173003 Cr-Commit-Position: refs/heads/master@{#343123}
-rw-r--r--chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h2
-rw-r--r--chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm14
-rw-r--r--chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm35
3 files changed, 51 insertions, 0 deletions
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
index 79057fb..7a68839 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
@@ -140,6 +140,7 @@ class Profile;
NSString* typedPrefix_;
Profile* profile_;
+ BOOL isScrolling_;
}
// Designated initializer.
@@ -200,6 +201,7 @@ class Profile;
// Return YES if the scroll-up or scroll-down arrows are showing.
- (BOOL)canScrollUp;
- (BOOL)canScrollDown;
+- (BOOL)isScrolling;
- (CGFloat)verticalScrollArrowHeight;
- (NSView*)visibleView;
- (NSScrollView*)scrollView;
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
index 6b49f00..d71ce4a 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
@@ -934,6 +934,7 @@ NSRect GetFirstButtonFrameForHeight(CGFloat height) {
index == selectedIndex_);
NSRect windowFrame = [[self window] frame];
NSSize newSize = NSMakeSize(NSWidth(windowFrame), 0.0);
+ isScrolling_ = YES;
[self adjustWindowLeft:windowFrame.origin.x
size:newSize
scrollingBy:finalDelta];
@@ -1340,6 +1341,11 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
// Called from BookmarkButton.
// Unlike bookmark_bar_controller's version, we DO default to being enabled.
- (void)mouseEnteredButton:(id)sender event:(NSEvent*)event {
+ // Prevent unnecessary button selection change while scrolling due to the
+ // size changing that happens in -performOneScroll:.
+ if (isScrolling_)
+ return;
+
[[NSCursor arrowCursor] set];
buttonThatMouseIsIn_ = sender;
@@ -1370,6 +1376,10 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
buttonThatMouseIsIn_ = nil;
[self setSelectedButtonByIndex:-1];
+ // During scrolling -mouseExitedButton: stops scrolling, so update the
+ // corresponding status field to reflect is has stopped.
+ isScrolling_ = NO;
+
// Stop any timer about opening a new hover-open folder.
// Since a performSelector:withDelay: on self retains self, it is
@@ -2011,6 +2021,10 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
return ![scrollDownArrowView_ isHidden];
}
+- (BOOL)isScrolling {
+ return isScrolling_;
+}
+
- (CGFloat)verticalScrollArrowHeight {
return verticalScrollArrowHeight_;
}
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
index 36d8bd6..203b6f8 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller_unittest.mm
@@ -484,6 +484,41 @@ TEST_F(BookmarkBarFolderControllerTest, ChildFolderWidth) {
EXPECT_GT(wideWidth, thinWidth);
}
+// Scrolling (in this case using keyboard up/down buttons) should
+// not be a cause of item hovering change where the mouse is pointing to.
+// Here we are simulating scrolling by calling -performOneScroll: which
+// indirectly is called by keyboard up/down buttons.
+TEST_F(BookmarkBarFolderControllerTest, ScrollingBehaviorAndMouseMovement){
+ base::scoped_nsobject<BookmarkBarFolderController> bbfc;
+ AddLotsOfNodes();
+ bbfc.reset(SimpleBookmarkBarFolderController());
+ [bbfc showWindow:bbfc.get()];
+ // We should be able to scroll-up otherwise the rest of the test is pointless.
+ ASSERT_TRUE([bbfc canScrollUp]);
+ NSArray* buttons = [bbfc buttons];
+ BookmarkButton* currentButton = [bbfc buttonThatMouseIsIn];
+ // Mouse cursor is not pointing to any button.
+ EXPECT_FALSE(currentButton);
+ BookmarkButton* firstButton = [buttons objectAtIndex:0];
+ [bbfc mouseEnteredButton:firstButton event:nil];
+ // Mouse cursor should be over the first button.
+ EXPECT_EQ(firstButton, [bbfc buttonThatMouseIsIn]);
+ while ([bbfc canScrollUp]) {
+ [bbfc performOneScroll:200 updateMouseSelection:NO];
+ [bbfc mouseEnteredButton:[buttons objectAtIndex:2] event:nil];
+ // -buttonThatMouseIsIn: must return firstButton because we
+ // are still scrolling. i.e. should not be changed when
+ // -mouseEnteredButton: is called.
+ EXPECT_EQ(firstButton,[bbfc buttonThatMouseIsIn]);
+ // We are scrolling unless mouse movement happens.
+ EXPECT_TRUE([bbfc isScrolling]);
+ }
+ [bbfc mouseExitedButton:firstButton event:nil];
+ // If mouse exit from a button scrolling should be stopped.
+ EXPECT_FALSE([bbfc isScrolling]);
+ [bbfc mouseEnteredButton:nil event:nil];
+}
+
// Simple scrolling tests.
// Currently flaky due to a changed definition of the correct menu boundaries.
TEST_F(BookmarkBarFolderControllerTest, DISABLED_SimpleScroll) {