New issue
Advanced search Search tips

Issue 867127 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Can't navigate overflow menu with keyboard arrow keys

Project Member Reported by markchang@chromium.org, Jul 24

Issue description

Chrome 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.


 
Cc: lpalmaro@chromium.org
#2 step -- use mouse to trigger the overflow.
Description: Show this description
Owner: ellyjo...@chromium.org
Status: Assigned (was: Available)
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@.
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.
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...
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.
FYI, this is not release blocking, as per laura.
Labels: Group-Menus
Labels: -Pri-1 Pri-2
Since it's not release blocking, moving to P2.
Labels: Proj-MacViews
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..).
Labels: -M-69 -Target-69 Target-70 M-70
MacViews triage: I'll try this in M70.
Project Member

Comment 14 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Labels: Needs-Feedback
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.!
867127-(70.0.3501.0).mp4
4.3 MB View Download
861127-(70.0.3524.0).mp4
4.9 MB View Download
#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