Can't navigate overflow menu with keyboard arrow keys |
|||||||||
Issue descriptionChrome Version: 70.0.3501.0 (Official Build) canary (64-bit) OS: Mac What steps will reproduce the problem? (1) open cnn.com (2) select the overflow (3-dot) menu so it opens with the mouse (3) hit the arrow keys What is the expected result? selection in the overflow menu moves as arrow keys are used. What happens instead? arrow keys act on the content area.
,
Jul 24
#2 step -- use mouse to trigger the overflow.
,
Jul 24
,
Jul 24
First pass analysis: This is only an issue with Material style menus. Material menus will likely need to pick up key focus when they appear. Sending this to ellyjones@.
,
Jul 25
Interesting - thanks for the report. They do pick up key focus when opened with the keyboard, but not when opened with the mouse. I am puzzled by that. Taking a look.
,
Jul 25
In the bad case, MenuController::OnWillDispatchKeyEvent() is only called for ET_KEY_RELEASED. In the "good" case, it receives both those and ET_KEY_PRESSED. In neither case does the menu actually have focus (!), which suggests that some funny business is happening with the responder chain...
,
Jul 25
Stealing the keyboard focus in MenuButton::Activate() fixes this specific issue but introduces a different one, which is that clicking the MenuButton steals keyboard focus and doesn't give it back afterwards. It's obviously possible to store the old focused view and try to restore it, but I worry about the menu causing that view to disappear. In general, I think the actual fix is going to be to have the MenuHost first in the responder chain - however that works right now.
,
Jul 25
FYI, this is not release blocking, as per laura.
,
Jul 31
,
Jul 31
Since it's not release blocking, moving to P2.
,
Aug 1
DispatchEventToMenu in bridged_content_view.mm will only be invoked if the keystroke isn't swallowed before BridgedContentView is reached in the responder chain. You'll notice that arrow keys work when focusing the omnibox _then_ clicking the app menu. But not if WebContents was clicked beforehand, since that puts it before BridgedContentView in the responder chain. I think the trick may be to invoke Widget::ClearNativeFocus() which is done from browser ui when calling FocusManager::ClearFocus. but we should also restore focus when the menu is dismissed (see BrowserView::OnActiveTabChanged() for ideas..).
,
Aug 2
MacViews triage: I'll try this in M70.
,
Aug 2
,
Aug 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e737baf4519b48a0ff8bd239a45686412336e40 commit 8e737baf4519b48a0ff8bd239a45686412336e40 Author: Elly Fong-Jones <ellyjones@chromium.org> Date: Wed Aug 15 15:30:12 2018 views: fix Mac menu key handling This change: 1) Adds logic to EventMonitorMac so that if an event is marked handled by the client of EventMonitorMac, the event is not handled further - i.e., EventMonitorMac can now eat events; 2) Renames the existing MenuPreTargetHandler to MenuPreTargetHandlerAura; 3) Adds MenuPreTargetHandlerMac which uses EventMonitorMac; 4) Adds MenuPreTargetHandler::Create to manufacture the platform-appropriate MenuPreTargetHandler instance. Bug: 867127 Change-Id: I33f7c750598b0496fa291434098f8019e4b99c47 Reviewed-on: https://chromium-review.googlesource.com/1174568 Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Cr-Commit-Position: refs/heads/master@{#583254} [modify] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/BUILD.gn [modify] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_controller.cc [modify] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_controller.h [modify] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_pre_target_handler.h [rename] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_pre_target_handler_aura.cc [add] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_pre_target_handler_aura.h [add] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_pre_target_handler_mac.h [add] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/controls/menu/menu_pre_target_handler_mac.mm [modify] https://crrev.com/8e737baf4519b48a0ff8bd239a45686412336e40/ui/views/event_monitor_mac.mm
,
Aug 15
,
Aug 16
Tried testing the issue on build without fix #70.0.3501.0 and latest canary version #70.0.3524.0 using Mac OS 10.13.1 ,by following the below steps. Steps: ===== 1.Launched chrome. 2.Navigated to cnn.com. 3.Clicked on the three dot overflow menu. 4.By hitting the arrow keys, observed that overlay menu does not get selected but the page behind the overlay menu scrolls accordingly. Attached screencasts for reference. ellyjones@: Could you please review the attached screencasts and help us in verifying the fix. Thanks.!
,
Aug 20
#16: I don't understand this video. This bug is about the three dots menu *in the browser*, in the top right of the window. It has nothing to do with CNN's website's menus. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by markchang@chromium.org
, Jul 24