fix: Menu accelerators not working (#15094)

This change fixes the regression in the menu accelerators working in linux, on some environments.
This commit is contained in:
Nitish Sakhawalkar 2018-11-09 13:54:16 -08:00 committed by GitHub
parent 9e2b7dbea5
commit 5994bf6745
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 63 additions and 11 deletions

View file

@ -900,14 +900,18 @@ void NativeWindowViews::SetFocusable(bool focusable) {
void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) { void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) {
#if defined(USE_X11) #if defined(USE_X11)
if (menu_model == nullptr) if (menu_model == nullptr) {
global_menu_bar_.reset(); global_menu_bar_.reset();
root_view_->UnregisterAcceleratorsWithFocusManager();
return;
}
if (!global_menu_bar_ && ShouldUseGlobalMenuBar()) if (!global_menu_bar_ && ShouldUseGlobalMenuBar())
global_menu_bar_.reset(new GlobalMenuBarX11(this)); global_menu_bar_.reset(new GlobalMenuBarX11(this));
// Use global application menu bar when possible. // Use global application menu bar when possible.
if (global_menu_bar_ && global_menu_bar_->IsServerStarted()) { if (global_menu_bar_ && global_menu_bar_->IsServerStarted()) {
root_view_->RegisterAcceleratorsWithFocusManager(menu_model);
global_menu_bar_->SetMenu(menu_model); global_menu_bar_->SetMenu(menu_model);
return; return;
} }

View file

@ -46,15 +46,14 @@ RootView::~RootView() {}
void RootView::SetMenu(AtomMenuModel* menu_model) { void RootView::SetMenu(AtomMenuModel* menu_model) {
if (menu_model == nullptr) { if (menu_model == nullptr) {
// Remove accelerators // Remove accelerators
accelerator_table_.clear(); UnregisterAcceleratorsWithFocusManager();
GetFocusManager()->UnregisterAccelerators(this);
// and menu bar. // and menu bar.
SetMenuBarVisibility(false); SetMenuBarVisibility(false);
menu_bar_.reset(); menu_bar_.reset();
return; return;
} }
RegisterAccelerators(menu_model); RegisterAcceleratorsWithFocusManager(menu_model);
// Do not show menu bar in frameless window. // Do not show menu bar in frameless window.
if (!window_->has_frame()) if (!window_->has_frame())
@ -197,12 +196,13 @@ bool RootView::AcceleratorPressed(const ui::Accelerator& accelerator) {
accelerator); accelerator);
} }
void RootView::RegisterAccelerators(AtomMenuModel* menu_model) { void RootView::RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model) {
if (!menu_model)
return;
// Clear previous accelerators. // Clear previous accelerators.
views::FocusManager* focus_manager = GetFocusManager(); UnregisterAcceleratorsWithFocusManager();
accelerator_table_.clear();
focus_manager->UnregisterAccelerators(this);
views::FocusManager* focus_manager = GetFocusManager();
// Register accelerators with focus manager. // Register accelerators with focus manager.
accelerator_util::GenerateAcceleratorTable(&accelerator_table_, menu_model); accelerator_util::GenerateAcceleratorTable(&accelerator_table_, menu_model);
for (const auto& iter : accelerator_table_) { for (const auto& iter : accelerator_table_) {
@ -211,4 +211,10 @@ void RootView::RegisterAccelerators(AtomMenuModel* menu_model) {
} }
} }
void RootView::UnregisterAcceleratorsWithFocusManager() {
views::FocusManager* focus_manager = GetFocusManager();
accelerator_table_.clear();
focus_manager->UnregisterAccelerators(this);
}
} // namespace atom } // namespace atom

View file

@ -36,6 +36,9 @@ class RootView : public views::View {
void HandleKeyEvent(const content::NativeWebKeyboardEvent& event); void HandleKeyEvent(const content::NativeWebKeyboardEvent& event);
void ResetAltState(); void ResetAltState();
void RestoreFocus(); void RestoreFocus();
// Register/Unregister accelerators supported by the menu model.
void RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model);
void UnregisterAcceleratorsWithFocusManager();
// views::View: // views::View:
void Layout() override; void Layout() override;
@ -44,9 +47,6 @@ class RootView : public views::View {
bool AcceleratorPressed(const ui::Accelerator& accelerator) override; bool AcceleratorPressed(const ui::Accelerator& accelerator) override;
private: private:
// Register accelerators supported by the menu model.
void RegisterAccelerators(AtomMenuModel* menu_model);
// Parent window, weak ref. // Parent window, weak ref.
NativeWindow* window_; NativeWindow* window_;

View file

@ -742,4 +742,46 @@ describe('Menu module', () => {
expect(Menu.getApplicationMenu()).to.be.null() 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')
})
})
}) })