Commit 7575340a authored by Timothy Loh's avatar Timothy Loh Committed by Commit Bot

Revert "Mac: Don't trigger mouse down events on bookmark buttons if command key is down"

This reverts commit 4a7db45d.

Reason for revert: DraggableButtonTest.ActsOnMouseDownIgnoredForCommandClick fails on Mac10.12 bot, e.g.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/12756 (test times out)

Original change's description:
> Mac: Don't trigger mouse down events on bookmark buttons if command key is down
> 
> Currently, all clicks on a bookmark folder button trigger their action on
> mouse down, so that the folder menu can open on mouse down as expected.
> 
> This also includes command click, which, rather than opening the folder menu,
> opens all the bookmarks in the folder as new tabs. This isn't desirable since:
> - The expected behavior is that button clicks trigger on mouse up.
> - Triggering this on mouse down interacts poorly with the nested run loop
> created by the confirmation dialog we display when the folder has many links.
> 
> This change causes the action to no longer fire on mouse down if the user
> is command-clicking the folder button.
> 
> Bug: 840387
> Change-Id: Ic1364160cf26f2f3ea6e67d7dd0d6cf5079082eb
> Reviewed-on: https://chromium-review.googlesource.com/1053947
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Commit-Queue: Leonard Grey <lgrey@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#557676}

TBR=rsesek@chromium.org,lgrey@chromium.org

Change-Id: Iafb3cc8af01be9ba50a5e944b44750c62ee1df84
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 840387
Reviewed-on: https://chromium-review.googlesource.com/1055027Reviewed-by: default avatarTimothy Loh <timloh@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557760}
parent b9456c16
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include <cmath> #include <cmath>
#include "base/logging.h" #include "base/logging.h"
#include "ui/base/cocoa/cocoa_base_utils.h"
// Code taken from <http://codereview.chromium.org/180036/diff/3001/3004>. // Code taken from <http://codereview.chromium.org/180036/diff/3001/3004>.
// TODO(viettrungluu): Do we want common, standard code for drag hysteresis? // TODO(viettrungluu): Do we want common, standard code for drag hysteresis?
...@@ -87,15 +86,6 @@ const CGFloat kDragExpirationTimeout = 0.45; ...@@ -87,15 +86,6 @@ const CGFloat kDragExpirationTimeout = 0.45;
whenMouseDown_ = [theEvent timestamp]; whenMouseDown_ = [theEvent timestamp];
actionHasFired_ = NO; actionHasFired_ = NO;
// Even if the regular click should trigger on mouse down (for example,
// opening the bookmark menu), command click should behave like a
// button click.
WindowOpenDisposition disposition =
ui::WindowOpenDispositionFromNSEvent(theEvent);
BOOL shouldActOnMouseDown =
actsOnMouseDown_ &&
disposition != WindowOpenDisposition::NEW_BACKGROUND_TAB;
if (draggable_) { if (draggable_) {
NSDate* date = [NSDate dateWithTimeIntervalSinceNow:kDragExpirationTimeout]; NSDate* date = [NSDate dateWithTimeIntervalSinceNow:kDragExpirationTimeout];
if ([self dragShouldBeginFromMouseDown:theEvent if ([self dragShouldBeginFromMouseDown:theEvent
...@@ -103,7 +93,7 @@ const CGFloat kDragExpirationTimeout = 0.45; ...@@ -103,7 +93,7 @@ const CGFloat kDragExpirationTimeout = 0.45;
[button_ beginDrag:theEvent]; [button_ beginDrag:theEvent];
[self endDrag]; [self endDrag];
} else { } else {
if (shouldActOnMouseDown) { if (actsOnMouseDown_) {
[self performMouseDownAction:theEvent]; [self performMouseDownAction:theEvent];
return kDraggableButtonImplDidWork; return kDraggableButtonImplDidWork;
} else { } else {
...@@ -111,7 +101,7 @@ const CGFloat kDragExpirationTimeout = 0.45; ...@@ -111,7 +101,7 @@ const CGFloat kDragExpirationTimeout = 0.45;
} }
} }
} else { } else {
if (shouldActOnMouseDown) { if (actsOnMouseDown_) {
[self performMouseDownAction:theEvent]; [self performMouseDownAction:theEvent];
return kDraggableButtonImplDidWork; return kDraggableButtonImplDidWork;
} else { } else {
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
@interface TestableDraggableButton : DraggableButton { @interface TestableDraggableButton : DraggableButton {
NSUInteger dragCount_; NSUInteger dragCount_;
NSUInteger triggerCount_; BOOL wasTriggered_;
} }
- (void)trigger:(id)sender; - (void)trigger:(id)sender;
- (BOOL)wasTriggered; - (BOOL)wasTriggered;
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
- (id)initWithFrame:(NSRect)frame { - (id)initWithFrame:(NSRect)frame {
if ((self = [super initWithFrame:frame])) { if ((self = [super initWithFrame:frame])) {
dragCount_ = 0; dragCount_ = 0;
triggerCount_ = 0; wasTriggered_ = NO;
} }
return self; return self;
} }
...@@ -31,24 +31,16 @@ ...@@ -31,24 +31,16 @@
} }
- (void)trigger:(id)sender { - (void)trigger:(id)sender {
triggerCount_++; wasTriggered_ = YES;
} }
- (BOOL)wasTriggered { - (BOOL)wasTriggered {
return triggerCount_ > 0; return wasTriggered_;
}
- (NSUInteger)triggerCount {
return triggerCount_;
} }
- (NSUInteger)dragCount { - (NSUInteger)dragCount {
return dragCount_; return dragCount_;
} }
- (void)performBlock:(void (^)(void))block {
block();
}
@end @end
class DraggableButtonTest : public CocoaTest {}; class DraggableButtonTest : public CocoaTest {};
...@@ -132,7 +124,7 @@ TEST_F(DraggableButtonTest, ResetState) { ...@@ -132,7 +124,7 @@ TEST_F(DraggableButtonTest, ResetState) {
cocoa_test_event_utils::MouseEventAtPoint(NSMakePoint(100,100), cocoa_test_event_utils::MouseEventAtPoint(NSMakePoint(100,100),
NSLeftMouseUp, NSLeftMouseUp,
0); 0);
// If the mouse moves > 5 pixels in either direction it should cause a drag. // If the mouse moves > 5 pixels in either direciton it should cause a drag.
[NSApp postEvent:upEvent atStart:YES]; [NSApp postEvent:upEvent atStart:YES];
[NSApp postEvent:moveEvent atStart:YES]; [NSApp postEvent:moveEvent atStart:YES];
[button mouseDown:downEvent]; [button mouseDown:downEvent];
...@@ -148,42 +140,3 @@ TEST_F(DraggableButtonTest, ResetState) { ...@@ -148,42 +140,3 @@ TEST_F(DraggableButtonTest, ResetState) {
EXPECT_EQ(2U, [button dragCount]); EXPECT_EQ(2U, [button dragCount]);
EXPECT_FALSE([[button cell] isHighlighted]); EXPECT_FALSE([[button cell] isHighlighted]);
} }
TEST_F(DraggableButtonTest, ActsOnMouseDown) {
base::scoped_nsobject<TestableDraggableButton> button(
[[TestableDraggableButton alloc]
initWithFrame:NSMakeRect(0, 0, 500, 500)]);
[button setTarget:button];
[button setAction:@selector(trigger:)];
[[button draggableButton] setActsOnMouseDown:YES];
[[test_window() contentView] addSubview:button.get()];
[button performSelector:@selector(performBlock:)
withObject:^{
EXPECT_EQ([button triggerCount], 1U);
}
afterDelay:.5
inModes:@[ NSEventTrackingRunLoopMode ]];
NSEvent* downEvent = cocoa_test_event_utils::MouseEventAtPoint(
NSMakePoint(10, 10), NSLeftMouseDown, 0);
[button mouseDown:downEvent];
}
TEST_F(DraggableButtonTest, ActsOnMouseDownIgnoredForCommandClick) {
base::scoped_nsobject<TestableDraggableButton> button(
[[TestableDraggableButton alloc]
initWithFrame:NSMakeRect(0, 0, 500, 500)]);
[button setTarget:button];
[button setAction:@selector(trigger:)];
[[button draggableButton] setActsOnMouseDown:YES];
[[test_window() contentView] addSubview:button.get()];
NSEvent* downEvent = cocoa_test_event_utils::MouseEventAtPoint(
NSMakePoint(10, 10), NSLeftMouseDown, NSEventModifierFlagCommand);
[button mouseDown:downEvent];
EXPECT_FALSE([button wasTriggered]);
NSEvent* upEvent = cocoa_test_event_utils::MouseEventAtPoint(
NSMakePoint(100, 100), NSLeftMouseUp, 0);
[button mouseUp:upEvent];
EXPECT_EQ([button triggerCount], 1U);
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment