From 6b54cbe573d70b77b81f5c79bdcced4786c5b56a Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Wed, 24 May 2023 00:50:17 +0000 Subject: [PATCH] Bug 1821886 - Reserve shortcut keys exiting from the fullscreen mode r=Gijs,edgar,smaug Chrome for Windows does not dispatch `keydown` event for shortcut keys existing from the fullscreen mode. Therefore, we can follow it. For reserving only shortcut keys in fullscreen mode, we need to duplicate XUL `` elements which define the shortcut keys (only one in Windows/Linux, but 3 in macOS). Then, their `disabled` attributes should be managed when toggling the fullscreen mode. Finally, we need to make `XULKeySetGlobalKeyListener` check the `disabled` attribute **of** `` elements because it's check in `DispatchXULKeyCommand` in the final step: https://searchfox.org/mozilla-central/rev/11a4d97a7b5cdfa133f4bda4525649f651703018/dom/events/KeyEventHandler.cpp#315-316 and it stops handling everything with doing nothing. I'm not sure whether this was intentionally implemented or just a inefficient code which we didn't take care the performance. However, I think that ignoring the disabled `` elements is reasonable behavior from `` element users point of view. (I found only one `` which is disabled by default: https://searchfox.org/mozilla-central/rev/11a4d97a7b5cdfa133f4bda4525649f651703018/browser/base/content/browser-sets.inc#225-233) Differential Revision: https://phabricator.services.mozilla.com/D178262 --- .../browser-fullScreenAndPointerLock.js | 20 ++++ browser/base/content/browser-sets.inc | 12 +- .../base/content/test/fullscreen/browser.ini | 1 + .../browser_fullscreen_enterInUrlbar.js | 5 + .../browser_fullscreen_keydown_reservation.js | 112 ++++++++++++++++++ .../fullscreen/browser_fullscreen-api-keys.js | 7 +- dom/events/GlobalKeyListener.cpp | 5 + dom/events/KeyEventHandler.cpp | 9 +- dom/events/KeyEventHandler.h | 9 +- 9 files changed, 173 insertions(+), 7 deletions(-) create mode 100644 browser/base/content/test/fullscreen/browser_fullscreen_keydown_reservation.js diff --git a/browser/base/content/browser-fullScreenAndPointerLock.js b/browser/base/content/browser-fullScreenAndPointerLock.js index 8376ee86a3f1..13af709e61c2 100644 --- a/browser/base/content/browser-fullScreenAndPointerLock.js +++ b/browser/base/content/browser-fullScreenAndPointerLock.js @@ -365,6 +365,7 @@ var FullScreen = { this._isPopupOpen = false; this.cleanup(); } + this._toggleShortcutKeys(); }, exitDomFullScreen() { @@ -556,6 +557,25 @@ var FullScreen = { } }, + _toggleShortcutKeys() { + const kEnterKeyIds = [ + "key_enterFullScreen", + "key_enterFullScreen_old", + "key_enterFullScreen_compat", + ]; + const kExitKeyIds = [ + "key_exitFullScreen", + "key_exitFullScreen_old", + "key_exitFullScreen_compat", + ]; + for (let id of window.fullScreen ? kEnterKeyIds : kExitKeyIds) { + document.getElementById(id)?.setAttribute("disabled", "true"); + } + for (let id of window.fullScreen ? kExitKeyIds : kEnterKeyIds) { + document.getElementById(id)?.removeAttribute("disabled"); + } + }, + /** * Clean up full screen, starting from the request origin's first ancestor * frame that is OOP. diff --git a/browser/base/content/browser-sets.inc b/browser/base/content/browser-sets.inc index 9a44a6a35547..dcf0593f2a78 100644 --- a/browser/base/content/browser-sets.inc +++ b/browser/base/content/browser-sets.inc @@ -215,12 +215,16 @@ #ifndef XP_MACOSX - + + #else - - - + + + + + + #endif { + content.wrappedJSObject.keydown = null; + content.window.addEventListener("keydown", event => { + switch (event.key) { + case "Shift": + case "Meta": + case "Control": + break; + default: + content.wrappedJSObject.keydown = event; + } + }); + }); + + EventUtils.synthesizeKey(shortcutKey.key, shortcutKey.modifiers); + + info( + `Waiting for entering the fullscreen mode with synthesizing ${shortcutDescription( + shortcutKey + )}...` + ); + await fullScreenEntered; + + info("Retrieving the result..."); + Assert.ok( + await SpecialPowers.spawn( + tab.linkedBrowser, + [], + async () => !!content.wrappedJSObject.keydown + ), + `Entering the fullscreen mode with ${shortcutDescription( + shortcutKey + )} should cause "keydown" event` + ); + + const fullScreenExited = BrowserTestUtils.waitForEvent( + window, + "fullscreen" + ); + + await SpecialPowers.spawn(tab.linkedBrowser, [], async () => { + content.wrappedJSObject.keydown = null; + }); + + EventUtils.synthesizeKey(shortcutKey.key, shortcutKey.modifiers); + + info( + `Waiting for exiting from the fullscreen mode with synthesizing ${shortcutDescription( + shortcutKey + )}...` + ); + await fullScreenExited; + + info("Retrieving the result..."); + Assert.ok( + await SpecialPowers.spawn( + tab.linkedBrowser, + [], + async () => !content.wrappedJSObject.keydown + ), + `Exiting from the fullscreen mode with ${shortcutDescription( + shortcutKey + )} should not cause "keydown" event` + ); + + BrowserTestUtils.removeTab(tab); + } +}); diff --git a/dom/base/test/fullscreen/browser_fullscreen-api-keys.js b/dom/base/test/fullscreen/browser_fullscreen-api-keys.js index 784bba767d07..1b1a07975e10 100644 --- a/dom/base/test/fullscreen/browser_fullscreen-api-keys.js +++ b/dom/base/test/fullscreen/browser_fullscreen-api-keys.js @@ -23,6 +23,11 @@ function receiveExpectedKeyEvents(aBrowser, aKeyCode, aTrusted) { let events = trusted ? ["keydown", "keyup"] : ["keydown", "keypress", "keyup"]; + if (trusted && keyCode == content.wrappedJSObject.KeyEvent.DOM_VK_F11) { + // trusted `F11` key shouldn't be fired because of reserved when it's + // a shortcut key for exiting from the full screen mode. + events.shift(); + } function listener(event) { let expected = events.shift(); Assert.equal( @@ -197,7 +202,7 @@ add_task(async function () { "correct number of fullscreen events occurred" ); if (!suppressed) { - expectedKeyEventsCount += keyCode == "VK_F11" ? 2 : 3; + expectedKeyEventsCount += keyCode == "VK_F11" ? 1 : 3; } is( keyEventsCount, diff --git a/dom/events/GlobalKeyListener.cpp b/dom/events/GlobalKeyListener.cpp index 322bdb6df5f5..5711e85d9439 100644 --- a/dom/events/GlobalKeyListener.cpp +++ b/dom/events/GlobalKeyListener.cpp @@ -623,6 +623,11 @@ already_AddRefed XULKeySetGlobalKeyListener::GetHandlerTarget( bool XULKeySetGlobalKeyListener::CanHandle(KeyEventHandler* aHandler, bool aWillExecute) const { + // If the element itself is disabled, ignore it. + if (aHandler->KeyElementIsDisabled()) { + return false; + } + nsCOMPtr commandElement; if (!GetElementForHandler(aHandler, getter_AddRefs(commandElement))) { return false; diff --git a/dom/events/KeyEventHandler.cpp b/dom/events/KeyEventHandler.cpp index 9f544ed318d3..4fdf3f71071f 100644 --- a/dom/events/KeyEventHandler.cpp +++ b/dom/events/KeyEventHandler.cpp @@ -166,7 +166,14 @@ bool KeyEventHandler::TryConvertToKeyboardShortcut( return true; } -already_AddRefed KeyEventHandler::GetHandlerElement() { +bool KeyEventHandler::KeyElementIsDisabled() const { + RefPtr keyElement = GetHandlerElement(); + return keyElement && + keyElement->AttrValueIs(kNameSpaceID_None, nsGkAtoms::disabled, + nsGkAtoms::_true, eCaseMatters); +} + +already_AddRefed KeyEventHandler::GetHandlerElement() const { if (mIsXULKey) { nsCOMPtr element = do_QueryReferent(mHandlerElement); return element.forget(); diff --git a/dom/events/KeyEventHandler.h b/dom/events/KeyEventHandler.h index 0e9b2fea4575..5d444cf32270 100644 --- a/dom/events/KeyEventHandler.h +++ b/dom/events/KeyEventHandler.h @@ -75,7 +75,14 @@ class KeyEventHandler final { uint32_t aCharCode, const IgnoreModifierState& aIgnoreModifierState); - already_AddRefed GetHandlerElement(); + /** + * Check whether the handler element is disabled. Note that this requires + * a QI to getting GetHandlerELement(). Therefore, this should not be used + * first in multiple checks. + */ + bool KeyElementIsDisabled() const; + + already_AddRefed GetHandlerElement() const; ReservedKey GetIsReserved() { return mReserved; }