diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 8ee9099177459..a5c1df8fc27ec 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -900,14 +900,18 @@ void NativeWindowViews::SetFocusable(bool focusable) { void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) { #if defined(USE_X11) - if (menu_model == nullptr) + if (menu_model == nullptr) { global_menu_bar_.reset(); + root_view_->UnregisterAcceleratorsWithFocusManager(); + return; + } if (!global_menu_bar_ && ShouldUseGlobalMenuBar()) global_menu_bar_.reset(new GlobalMenuBarX11(this)); // Use global application menu bar when possible. if (global_menu_bar_ && global_menu_bar_->IsServerStarted()) { + root_view_->RegisterAcceleratorsWithFocusManager(menu_model); global_menu_bar_->SetMenu(menu_model); return; } diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc index 716563ff0141c..304eb5dd87474 100644 --- a/atom/browser/ui/views/root_view.cc +++ b/atom/browser/ui/views/root_view.cc @@ -44,15 +44,14 @@ RootView::~RootView() {} void RootView::SetMenu(AtomMenuModel* menu_model) { if (menu_model == nullptr) { // Remove accelerators - accelerator_table_.clear(); - GetFocusManager()->UnregisterAccelerators(this); + UnregisterAcceleratorsWithFocusManager(); // and menu bar. SetMenuBarVisibility(false); menu_bar_.reset(); return; } - RegisterAccelerators(menu_model); + RegisterAcceleratorsWithFocusManager(menu_model); // Do not show menu bar in frameless window. if (!window_->has_frame()) @@ -178,12 +177,13 @@ bool RootView::AcceleratorPressed(const ui::Accelerator& accelerator) { accelerator); } -void RootView::RegisterAccelerators(AtomMenuModel* menu_model) { +void RootView::RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model) { + if (!menu_model) + return; // Clear previous accelerators. - views::FocusManager* focus_manager = GetFocusManager(); - accelerator_table_.clear(); - focus_manager->UnregisterAccelerators(this); + UnregisterAcceleratorsWithFocusManager(); + views::FocusManager* focus_manager = GetFocusManager(); // Register accelerators with focus manager. accelerator_util::GenerateAcceleratorTable(&accelerator_table_, menu_model); for (const auto& iter : accelerator_table_) { @@ -192,4 +192,10 @@ void RootView::RegisterAccelerators(AtomMenuModel* menu_model) { } } +void RootView::UnregisterAcceleratorsWithFocusManager() { + views::FocusManager* focus_manager = GetFocusManager(); + accelerator_table_.clear(); + focus_manager->UnregisterAccelerators(this); +} + } // namespace atom diff --git a/atom/browser/ui/views/root_view.h b/atom/browser/ui/views/root_view.h index 66c6d4e8a5571..300073655bc2f 100644 --- a/atom/browser/ui/views/root_view.h +++ b/atom/browser/ui/views/root_view.h @@ -34,6 +34,10 @@ class RootView : public views::View { bool IsMenuBarVisible() const; void HandleKeyEvent(const content::NativeWebKeyboardEvent& event); void ResetAltState(); + void RestoreFocus(); + // Register/Unregister accelerators supported by the menu model. + void RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model); + void UnregisterAcceleratorsWithFocusManager(); // views::View: void Layout() override; @@ -42,9 +46,6 @@ class RootView : public views::View { bool AcceleratorPressed(const ui::Accelerator& accelerator) override; private: - // Register accelerators supported by the menu model. - void RegisterAccelerators(AtomMenuModel* menu_model); - // Parent window, weak ref. NativeWindow* window_; diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 8693290523a28..7b9a8a410b86d 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -742,4 +742,46 @@ describe('Menu module', () => { expect(Menu.getApplicationMenu()).to.be.null() }) }) + + describe('menu accelerators', () => { + let testFn = it + try { + // We have other tests that check if native modules work, if we fail to require + // robotjs let's skip this test to avoid false negatives + require('robotjs') + } catch (err) { + testFn = it.skip + } + const sendRobotjsKey = (key, modifiers = [], delay = 500) => { + return new Promise((resolve, reject) => { + require('robotjs').keyTap(key, modifiers) + setTimeout(() => { + resolve() + }, delay) + }) + } + + testFn('menu accelerators perform the specified action', async () => { + const menu = Menu.buildFromTemplate([ + { + label: 'Test', + submenu: [ + { + label: 'Test Item', + accelerator: 'Ctrl+T', + click: () => { + // Test will succeed, only when the menu accelerator action + // is triggered + Promise.resolve() + }, + id: 'test' + } + ] + } + ]) + Menu.setApplicationMenu(menu) + expect(Menu.getApplicationMenu()).to.not.be.null() + await sendRobotjsKey('t', 'control') + }) + }) })