From c789de1b39f8835e110d3d111d38dd4574200137 Mon Sep 17 00:00:00 2001 From: Brian Clifton Date: Fri, 3 Mar 2017 16:19:57 -0800 Subject: [PATCH] Menu on Windows now properly responds to ALT. Menu is now toggled only under these conditions: - the left ALT key is pressed (ignore AltGr) - last key pressed was ALT (typing ALT codes should not toggle menu) - no other key is being pushed simultaneously - since initial keydown, ALT has been the only key pressed NOTE: key state is reset when window blurs (in case you used a shortcut that opened a new window, which prevents the keyup handler from firing on release) Auditors: @jonathansampson, @bbondy Fixes https://github.com/brave/browser-laptop/issues/5775 --- js/components/main.js | 52 ++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/js/components/main.js b/js/components/main.js index f0f8b5eeb11..5dcb0eefe82 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -105,7 +105,7 @@ class Main extends ImmutableComponent { this.onFind = this.onFind.bind(this) this.onFindHide = this.onFindHide.bind(this) this.checkForTitleMode = debounce(this.checkForTitleMode.bind(this), 20) - this.lastKeyPressed = undefined + this.resetAltMenuProcessing() } registerWindowLevelShortcuts () { // For window level shortcuts that don't work as local shortcuts @@ -131,36 +131,47 @@ class Main extends ImmutableComponent { } break } - + this.keydown[e.which] = true this.lastKeyPressed = e.which }) } + resetAltMenuProcessing () { + this.lastKeyPressed = undefined + this.keydown = {} + this.keydownHistory = [] + } + registerCustomTitlebarHandlers () { if (this.customTitlebar.enabled) { document.addEventListener('keyup', (e) => { const customTitlebar = this.customTitlebar switch (e.which) { case keyCodes.ALT: - // Ignore right alt (AltGr) - if (e.location === keyLocations.DOM_KEY_LOCATION_RIGHT) { + /* + Only show/hide the menu if: + - the left ALT key is pressed (ignore AltGr) + - last key pressed was ALT (typing ALT codes should not toggle menu) + - no other key is being pushed simultaneously + - since initial keydown, ALT has been the only key pressed + */ + if (e.location === keyLocations.DOM_KEY_LOCATION_RIGHT || + this.lastKeyPressed !== keyCodes.ALT || + Object.keys(this.keydown).length > 1 || + this.keydownHistory.length > 0) { break } - // Only show/hide the menu if last key pressed was ALT - // (typing ALT codes should not toggle menu) - if (this.lastKeyPressed === keyCodes.ALT) { - e.preventDefault() + e.preventDefault() - if (getSetting(settings.AUTO_HIDE_MENU)) { - windowActions.toggleMenubarVisible(null) + if (getSetting(settings.AUTO_HIDE_MENU)) { + windowActions.toggleMenubarVisible(null) + } else { + if (customTitlebar.menubarSelectedIndex) { + windowActions.setMenuBarSelectedIndex() + windowActions.setContextMenuDetail() } else { - if (customTitlebar.menubarSelectedIndex) { - windowActions.setMenuBarSelectedIndex() - windowActions.setContextMenuDetail() - } else { - windowActions.setMenuBarSelectedIndex(0) - } + windowActions.setMenuBarSelectedIndex(0) } } break @@ -177,6 +188,14 @@ class Main extends ImmutableComponent { } break } + + // For ALT menu processing + if (Object.keys(this.keydown).length > 1) { + this.keydownHistory.push(e.which) + } else { + this.keydownHistory = [] + } + delete this.keydown[e.which] }) document.addEventListener('focus', (e) => { @@ -528,6 +547,7 @@ class Main extends ImmutableComponent { windowActions.onFocusChanged(true) }) currentWindow.on('blur', function () { + self.resetAltMenuProcessing() appActions.windowBlurred(currentWindow.id) windowActions.onFocusChanged(false) })