MacViews - bubbles do not dismiss when rapidly clicking |
||||||
Issue description
Chrome Version : 64.0.3269.3
OS Version: OS X 10.12.6
What steps will reproduce the problem?
1. Mash the profile switcher button with clicks
What is the expected result?
profile switcher should appear and disappear with each pair of clicks
What happens instead of that?
stays open.
Views bubbles are relying on activation changes:
void BubbleDialogDelegateView::OnWidgetActivationChanged(Widget* widget,
bool active) {
if (close_on_deactivate() && widget == GetWidget() && !active)
GetWidget()->Close();
}
But it looks like the mac window server suppresses activation changes when clicking rapidly. For Cocoa bubbles, we install an event tap and call -[NSWindow windowDidResignKey:] directly with a handcrafted NSNotification -- https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/base_bubble_controller.mm?q=registerKeyStateEventTap&sq=package:chromium&l=358
Installing an event tap is easy with views::EventMonitor, but we probably want to wrap up this logic and sahre it since there's some quirky stuff with window levels.
See context at http://crbug.com/784574#c19
,
Nov 29 2017
,
Nov 29 2017
,
Nov 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9cedd9c6854a7fb14a2bb8e1c52293641deea536 commit 9cedd9c6854a7fb14a2bb8e1c52293641deea536 Author: Trent Apted <tapted@chromium.org> Date: Thu Nov 30 01:16:18 2017 MacViews: Share logic with Cocoa bubbles for dismissing on clicks. MacOS suppresses window activation events when clicking rapidly, so to get menu-like behavior on popups we need to monitor clicks explicitly. Currently, clicking rapidly on browser buttons that summon MacViews popups does not give the expected behavior. Share the logic from base_bubble_controller.mm that does this, and use it in views::BubbleDialogDelegate. Bug: 789377 Change-Id: I6f38fe70032ffdff6707651f322500de612d002e Reviewed-on: https://chromium-review.googlesource.com/795611 Commit-Queue: Trent Apted <tapted@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#520367} [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/BUILD.gn [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/base_bubble_controller.h [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/base_bubble_controller.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/base_bubble_controller_unittest.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/menu_button_unittest.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/tabs/tab_controller_unittest.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/chrome/browser/ui/cocoa/toolbar/reload_button_unittest.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/BUILD.gn [add] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/cocoa/bubble_closer.h [add] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/cocoa/bubble_closer.mm [add] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/cocoa/bubble_closer_unittest.mm [rename] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/test/menu_test_observer.h [rename] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/base/test/menu_test_observer.mm [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/views/bubble/bubble_dialog_delegate.cc [modify] https://crrev.com/9cedd9c6854a7fb14a2bb8e1c52293641deea536/ui/views/bubble/bubble_dialog_delegate.h
,
Nov 30 2017
,
Nov 30 2017
[Auto-generated comment by a script] We noticed that this issue is targeted for M-64; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-64 label, otherwise remove Merge-TBD label. Thanks.
,
Dec 2 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by tapted@chromium.org
, Nov 29 2017