Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mac: Keyboard shortcuts and menu bar item for Find don't work #3462

Closed
magreenblatt opened this issue Mar 9, 2023 · 10 comments
Closed

mac: Keyboard shortcuts and menu bar item for Find don't work #3462

magreenblatt opened this issue Mar 9, 2023 · 10 comments
Labels
bug Bug report macos MacOS platform SampleApps Related to sample apps

Comments

@magreenblatt
Copy link
Collaborator

Original report by me.


What steps will reproduce the problem?

  1. Run cefclient with the Chrome runtime enabled
  2. Execute a keyboard shortcut like Cmd-F

What is the expected output? What do you see instead?

For Cmd-F the “Find” widget should display. Instead, nothing happens.

What version of the product are you using? On what operating system?

M112 on macOS 13.2.1 (22D68).

Additional related notes:

  • Selecting “Find..” from the 3-dot menu works as expected.
  • Find-related options on the top menu are disabled (also disabled with the Alloy runtime).

@magreenblatt
Copy link
Collaborator Author

Related commentary from chrome/browser/global_keyboard_shortcuts_mac.h CommandForKeyEvent:

// macOS applications are supposed to put all keyEquivalents [hotkeys] in the
// menu bar. For legacy reasons, Chrome does not. There are around 30 hotkeys
// that are explicitly coded to virtual keycodes. This has the following
// downsides:
// * There is no way for the user to configure or disable these keyEquivalents.
// * This can cause keyEquivalent conflicts for non-US keyboard layouts with
// different default keyEquivalents, see https://crbug.com/841299.
//
// This function first searches the menu bar for a matching keyEquivalent. If
// nothing is found, then it searches through the explicitly coded virtual
// keycodes not present in the NSMenu.
//
// Note: AppKit exposes symbolic hotkeys [e.g. cmd + `] not present in the
// NSMenu as well. The user can remap these to conflict with Chrome hotkeys.
// This function will return the Chrome hotkey, regardless of whether there's a
// conflicting symbolic hotkey.

So, this issue is likely related to the menu bar items being disabled.

@magreenblatt magreenblatt changed the title mac: cefclient: Keyboard shortcuts don't work mac: cefclient: Keyboard shortcuts for Find don't work Mar 15, 2023
@magreenblatt
Copy link
Collaborator Author

magreenblatt commented Mar 15, 2023

Chrome builds the menu bar in chrome/browser/ui/cocoa/main_menu_builder.mm [1] whereas CEF sample apps use MainMenu.xib.

Some commands (like cut/copy/paste here) are handled by Chromium's render_widget_host_view_cocoa.h, which is the view that lives in the Cocoa view hierarchy.

Other commands (like find) might need to be enabled/handled in CEF's ClientAppDelegate, in which case we would be blocked by #3282.

Looking at the find menu created by Chrome's main_menu_builder.mm, some items simply map to a command ID (like IDC_FIND) which is then passed to commandDispatch in Chrome's app_controller_mac.mm [2], while others are directly forwarded to selectors implemented by Chromium's render_widget_host_view_cocoa.mm (like copyToFindPboard). Enabled/disabled status for these items is handled via validateUserInterfaceItem (in render_widget_host_view_cocoa.mm if there's a browser, app_controller_mac.mm otherwise).

[1] CEF disables the call to chrome::BuildMainMenu in ChromeBrowserMainPartsMac::PreCreateMainMessageLoop via chrome_runtime.patch.

[2] There's also some fancy logic to route handling of items using commandDispatch.

@magreenblatt magreenblatt changed the title mac: cefclient: Keyboard shortcuts for Find don't work mac: Keyboard shortcuts for Find don't work Mar 15, 2023
@magreenblatt magreenblatt removed the cefclient Related to the cefclient sample app label Mar 15, 2023
@magreenblatt
Copy link
Collaborator Author

This is also an issue with cefsimple

@magreenblatt magreenblatt changed the title mac: Keyboard shortcuts for Find don't work mac: Keyboard shortcuts and menu bar item for Find don't work Mar 15, 2023
@magreenblatt
Copy link
Collaborator Author

@magreenblatt
Copy link
Collaborator Author

Testing with ⌘C

Results in a call to validateUserInterfaceItem.

When browser view is focused:

command_dispatcher.mm(95) performKeyEquivalent:
native_widget_mac_nswindow.mm(613) defaultPerformKeyEquivalent:
render_widget_host_view_cocoa.mm(888) performKeyEquivalent:
render_widget_host_view_cocoa.mm(904) performKeyEquivalent: YES
command_dispatcher.mm(197) redispatchKeyEvent:
command_dispatcher.mm(95) performKeyEquivalent:
render_widget_host_view_cocoa.mm(1628) validateUserInterfaceItem:
chrome_render_widget_host_view_mac_delegate.mm(153) validateUserInterfaceItem:
command_dispatcher.mm(197) redispatchKeyEvent:

When URL bar text area is focused:

command_dispatcher.mm(95) performKeyEquivalent:
native_widget_mac_nswindow.mm(613) defaultPerformKeyEquivalent:
render_widget_host_view_cocoa.mm(888) performKeyEquivalent:
render_widget_host_view_cocoa.mm(891) performKeyEquivalent: NO (NotKeyWindow)
bridged_content_view.mm(1572) validateUserInterfaceItem:

Call stack to chrome_render_widget_host_view_mac_delegate.mm validateUserInterfaceItem:

6   Chromium Embedded Framework         0x00000001bbf809d0 -[ChromeRenderWidgetHostViewMacDelegate validateUserInterfaceItem:isValidItem:] + 240
7   Chromium Embedded Framework         0x00000001affdc224 -[RenderWidgetHostViewCocoa validateUserInterfaceItem:] + 1124
8   AppKit                              0x00007ff8133a8691 -[NSMenu _enableItem:] + 933
9   AppKit                              0x00007ff8134dfdbf -[NSCarbonMenuImpl _carbonUpdateStatusEvent:handlerCallRef:] + 384
10  AppKit                              0x00007ff8134c8054 NSSLMMenuEventHandler + 990
11  HIToolbox                           0x00007ff819bb1e3f DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1381
12  HIToolbox                           0x00007ff819bb1288 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 333
13  HIToolbox                           0x00007ff819bc5d85 SendEventToEventTarget + 39
14  HIToolbox                           0x00007ff819c218cb SendHICommandEvent(unsigned int, HICommand const*, unsigned int, unsigned int, unsigned char, void const*, OpaqueEventTargetRef*, OpaqueEventTargetRef*, OpaqueEventRef**) + 361
15  HIToolbox                           0x00007ff819c21753 UpdateHICommandStatus + 47
16  HIToolbox                           0x00007ff819c216dc EnableMenu(MenuData*, OpaqueEventTargetRef*, unsigned short, unsigned char, CheckMenuData*, unsigned int, double) + 84
17  HIToolbox                           0x00007ff819bfe8e5 Check1MenuForKeyEvent(MenuData*, CheckMenuData*) + 783
18  HIToolbox                           0x00007ff819bfe1b5 CheckMenusForKeyEvent(MenuData*, CheckMenuData*) + 663
19  HIToolbox                           0x00007ff819d62430 IsMatchingMenuKeyEvent(MenuData*, OpaqueEventRef*, unsigned int, MenuData**, unsigned short*) + 815
20  HIToolbox                           0x00007ff819bfdd78 _IsMenuKeyEvent(MenuData*, OpaqueEventRef*, unsigned int, MenuData**, unsigned short*) + 37
21  HIToolbox                           0x00007ff819bfdd28 IsMenuKeyEvent + 112
22  AppKit                              0x00007ff8136a05fd +[NSCarbonMenuImpl _menuItemWithKeyEquivalentMatchingEventRef:inMenu:includingDisabledItems:] + 214
23  AppKit                              0x00007ff8134c7af9 _NSFindMenuItemMatchingCommandKeyEvent + 206
24  AppKit                              0x00007ff8134e25d7 -[NSMenu performKeyEquivalent:] + 317
25  AppKit                              0x00007ff8139cc8dc routeKeyEquivalent + 656
26  AppKit                              0x00007ff8133634e4 -[NSApplication(NSEvent) sendEvent:] + 804
27  cefclient                           0x00000001083c2a4d -[ClientApplication sendEvent:] + 93
28  Chromium Embedded Framework         0x00000001b98e6673 -[CommandDispatcher redispatchKeyEvent:] + 1443
29  Chromium Embedded Framework         0x00000001bbb35271 remote_cocoa::NativeWidgetNSWindowBridge::RedispatchKeyEvent(NSEvent*) + 65
30  Chromium Embedded Framework         0x00000001c0f8a7eb views::NativeWidgetMacNSWindowHost::RedispatchKeyEvent(NSEvent*) + 91
31  Chromium Embedded Framework         0x00000001ca4114af views::UnhandledKeyboardEventHandler::HandleNativeKeyboardEvent(content::NativeWebKeyboardEvent const&, views::FocusManager*) + 159
32  Chromium Embedded Framework         0x00000001ca4078f8 views::UnhandledKeyboardEventHandler::HandleKeyboardEvent(content::NativeWebKeyboardEvent const&, views::FocusManager*) + 376
33  Chromium Embedded Framework         0x00000001c9c4352b BrowserView::HandleKeyboardEvent(content::NativeWebKeyboardEvent const&) + 123
34  Chromium Embedded Framework         0x00000001c92809f5 Browser::HandleKeyboardEvent(content::WebContents*, content::NativeWebKeyboardEvent const&) + 213
35  Chromium Embedded Framework         0x00000001afdd5caa content::WebContentsImpl::HandleKeyboardEvent(content::NativeWebKeyboardEvent const&) + 410
36  Chromium Embedded Framework         0x00000001af925834 content::RenderWidgetHostImpl::OnKeyboardEventAck(content::EventWithLatencyInfo<content::NativeWebKeyboardEvent> const&, blink::mojom::InputEventResultSource, blink::mojom::InputEventResultState) + 548

Testing with ⌘F

Does not result in a call to validateUserInterfaceItem.

When browser view is focused:

command_dispatcher.mm(95) performKeyEquivalent:
native_widget_mac_nswindow.mm(613) defaultPerformKeyEquivalent:
render_widget_host_view_cocoa.mm(888) performKeyEquivalent:
render_widget_host_view_cocoa.mm(904) performKeyEquivalent: YES
command_dispatcher.mm(197) redispatchKeyEvent:
command_dispatcher.mm(95) performKeyEquivalent:
command_dispatcher.mm(197) redispatchKeyEvent:

When URL bar text area is focused:

command_dispatcher.mm(95) performKeyEquivalent:
native_widget_mac_nswindow.mm(613) defaultPerformKeyEquivalent:
render_widget_host_view_cocoa.mm(888) performKeyEquivalent:
render_widget_host_view_cocoa.mm(891) performKeyEquivalent: NO (NotKeyWindow)

@magreenblatt
Copy link
Collaborator Author

Testing with ⌘F; Does not result in a call to validateUserInterfaceItem.

This might be because the Find command does not have a valid target in MainMenu.xib (e.g. performFindPanelAction does not exist) (details).

@magreenblatt
Copy link
Collaborator Author

Other commands (like find) might need to be enabled/handled in CEF's ClientAppDelegate, in which case we would be blocked by #3282.

This looks like the way forward. We can then configure the correct target in MainMenu.xib and forward the commands to Chrome.

@magreenblatt
Copy link
Collaborator Author

Changing the "Find" portion of MainMenu.xib for cefclient to:

<menuItem title="Find" id="218">
    <menu key="submenu" title="Find" id="220">
        <items>
            <menuItem title="Find…" tag="37000" keyEquivalent="f" id="209">
                <connections>
                    <action selector="commandDispatch:" target="-1" id="241"/>
                </connections>
            </menuItem>
            <menuItem title="Find Next" tag="37001" keyEquivalent="g" id="208">
                <connections>
                    <action selector="commandDispatch:" target="-1" id="242"/>
                </connections>
            </menuItem>
            <menuItem title="Find Previous" tag="37002" keyEquivalent="G" id="213">
                <modifierMask key="keyEquivalentModifierMask" shift="YES" command="YES"/>
                <connections>
                    <action selector="commandDispatch:" target="-1" id="243"/>
                </connections>
            </menuItem>
            <menuItem title="Use Selection for Find" keyEquivalent="e" id="221">
                <connections>
                    <action selector="copyToFindPboard:" target="-1" id="244"/>
                </connections>
            </menuItem>
            <menuItem title="Jump to Selection" keyEquivalent="j" id="210">
                <connections>
                    <action selector="centerSelectionInVisibleArea:" target="-1" id="245"/>
                </connections>
            </menuItem>
        </items>
    </menu>
</menuItem>

The copyToFindPboard: and centerSelectionInVisibleArea: items are checked via:

  • respondsToSelector: in render_widget_host_view_cocoa.mm (returns YES)
  • validateUserInterfaceItem: in render_widget_host_view_cocoa.mm and chrome_render_widget_host_view_mac_delegate.mm

The commandDispatch: items are checked via:

  • respondsToSelector: in render_widget_host_view_cocoa.mm and native_widget_mac_nswindow.mm (returns NO here).

It looks like BrowserFrameMac::OnWindowInitialized is not being called, so _commandHandler is nil in native_widget_mac_nswindow.mm.

For cefsimple --enable-chrome-runtime it is initialized via:

6   Chromium Embedded Framework         0x00000001ca7a61d9 BrowserFrameMac::OnWindowInitialized() + 233
7   Chromium Embedded Framework         0x00000001c26fd0a6 views::NativeWidgetMac::InitNativeWidget(views::Widget::InitParams) + 1494
8   Chromium Embedded Framework         0x00000001c266e0d0 views::Widget::Init(views::Widget::InitParams) + 2576
9   Chromium Embedded Framework         0x00000001cb25aa31 BrowserFrame::InitBrowserFrame() + 785
10  Chromium Embedded Framework         0x00000001cb3a54dc BrowserWindow::CreateBrowserWindow(std::Cr::unique_ptr<Browser, std::Cr::default_delete<Browser>>, bool, bool) + 204
11  Chromium Embedded Framework         0x00000001ca9b34f1 (anonymous namespace)::CreateBrowserWindow(std::Cr::unique_ptr<Browser, std::Cr::default_delete<Browser>>, bool, bool) + 81
12  Chromium Embedded Framework         0x00000001ca9b2687 Browser::Browser(Browser::CreateParams const&) + 3047
13  Chromium Embedded Framework         0x00000001ca9b1a8d Browser::Browser(Browser::CreateParams const&) + 29
14  Chromium Embedded Framework         0x00000001ca9b194d Browser::Create(Browser::CreateParams const&) + 189
15  Chromium Embedded Framework         0x00000001a674eb89 ChromeBrowserHostImpl::CreateBrowser(CefBrowserCreateParams const&) + 953
16  Chromium Embedded Framework         0x00000001a674e57d ChromeBrowserHostImpl::Create(CefBrowserCreateParams const&) + 61
17  Chromium Embedded Framework         0x00000001a6713ef4 CefBrowserHostBase::Create(CefBrowserCreateParams&) + 180
18  Chromium Embedded Framework         0x00000001a6713c8f CefBrowserHost::CreateBrowserSync(CefWindowInfo const&, scoped_refptr<CefClient>, CefStringBase<CefStringTraitsUTF16> const&, CefStructBase<CefBrowserSettingsTraits> const&, scoped_refptr<CefDictionaryValue>, scoped_refptr<CefRequestContext>) + 975
19  Chromium Embedded Framework         0x00000001a6714cfc (anonymous namespace)::CreateBrowserHelper::Run() + 140

@magreenblatt
Copy link
Collaborator Author

It looks like BrowserFrameMac::OnWindowInitialized is not being called

After fixing that, the commandDispatch: items are checked via:

  • native_widget_mac_nswindow.mm validateUserInterfaceItem
  • browser_window_command_handler.mm validateUserInterfaceItem
  • BrowserFrameMac::ValidateUserInterfaceItem

We can implement the various BrowserFrameMac (views::NativeWidgetMac) callbacks in CefNativeWidgetMac.

@magreenblatt
Copy link
Collaborator Author

Testing these changes it looks like the Find Bar can still be triggered (even in Google Chrome) while showing a window-modal dialog. Filed as https://bugs.chromium.org/p/chromium/issues/detail?id=1434768.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report macos MacOS platform SampleApps Related to sample apps
Projects
None yet
Development

No branches or pull requests

1 participant