diff --git a/browser/base/content/test/permissions/browser_reservedkey.js b/browser/base/content/test/permissions/browser_reservedkey.js index 2036188584b5..b077fd9cdbc1 100644 --- a/browser/base/content/test/permissions/browser_reservedkey.js +++ b/browser/base/content/test/permissions/browser_reservedkey.js @@ -6,8 +6,6 @@ add_task(async function test_reserved_shortcuts() { key1.setAttribute("key", "O"); key1.setAttribute("reserved", "true"); key1.setAttribute("count", "0"); - // We need to have the attribute "oncommand" for the "command" listener to fire - key1.setAttribute("oncommand", "//"); key1.addEventListener("command", () => { let attribute = key1.getAttribute("count"); key1.setAttribute("count", Number(attribute) + 1); @@ -19,8 +17,6 @@ add_task(async function test_reserved_shortcuts() { key2.setAttribute("key", "P"); key2.setAttribute("reserved", "false"); key2.setAttribute("count", "0"); - // We need to have the attribute "oncommand" for the "command" listener to fire - key2.setAttribute("oncommand", "//"); key2.addEventListener("command", () => { let attribute = key2.getAttribute("count"); key2.setAttribute("count", Number(attribute) + 1); @@ -31,8 +27,6 @@ add_task(async function test_reserved_shortcuts() { key3.setAttribute("modifiers", "shift"); key3.setAttribute("key", "Q"); key3.setAttribute("count", "0"); - // We need to have the attribute "oncommand" for the "command" listener to fire - key3.setAttribute("oncommand", "//"); key3.addEventListener("command", () => { let attribute = key3.getAttribute("count"); key3.setAttribute("count", Number(attribute) + 1); @@ -226,3 +220,98 @@ add_task(async function test_backspace_delete() { BrowserTestUtils.removeTab(tab); }); + +// TODO: Make this to run on Windows too to have automated tests also there. +if ( + navigator.platform.includes("Mac") || + navigator.platform.includes("Linux") +) { + add_task( + async function test_reserved_shortcuts_conflict_with_user_settings() { + await new Promise(resolve => { + SpecialPowers.pushPrefEnv( + { set: [["test.events.async.enabled", true]] }, + resolve + ); + }); + + const keyset = document.createXULElement("keyset"); + const key = document.createXULElement("key"); + key.setAttribute("id", "conflict_with_known_native_key_binding"); + if (navigator.platform.includes("Mac")) { + // Select to end of the paragraph + key.setAttribute("modifiers", "ctrl,shift"); + key.setAttribute("key", "E"); + } else { + // Select All + key.setAttribute("modifiers", "ctrl"); + key.setAttribute("key", "a"); + } + key.setAttribute("reserved", "true"); + key.setAttribute("count", "0"); + key.addEventListener("command", () => { + const attribute = key.getAttribute("count"); + key.setAttribute("count", Number(attribute) + 1); + }); + + keyset.appendChild(key); + const container = document.createXULElement("box"); + container.appendChild(keyset); + document.documentElement.appendChild(container); + + const pageUrl = + "data:text/html,
Test
"; + const tab = await BrowserTestUtils.openNewForegroundTab( + gBrowser, + pageUrl + ); + + await SpecialPowers.spawn( + tab.linkedBrowser, + [key.getAttribute("key")], + async function(aExpectedKeyValue) { + content.promiseTestResult = new Promise(resolve => { + content.addEventListener("keyup", event => { + if (event.key.toLowerCase() == aExpectedKeyValue.toLowerCase()) { + resolve( + content + .getSelection() + .getRangeAt(0) + .toString() + ); + } + }); + }); + } + ); + + EventUtils.synthesizeKey(key.getAttribute("key"), { + ctrlKey: key.getAttribute("modifiers").includes("ctrl"), + shiftKey: key.getAttribute("modifiers").includes("shift"), + }); + + const selectedText = await SpecialPowers.spawn( + tab.linkedBrowser, + [], + async function() { + return content.promiseTestResult; + } + ); + is( + selectedText, + "Test", + "The shortcut key should select all text in the editor" + ); + + is( + key.getAttribute("count"), + "0", + "The reserved shortcut key should be consumed by the focused editor instead" + ); + + document.documentElement.removeChild(container); + + BrowserTestUtils.removeTab(tab); + } + ); +} diff --git a/dom/events/GlobalKeyListener.cpp b/dom/events/GlobalKeyListener.cpp index 38e6d2e6c8ad..d5f8ca3d467a 100644 --- a/dom/events/GlobalKeyListener.cpp +++ b/dom/events/GlobalKeyListener.cpp @@ -13,6 +13,7 @@ #include "mozilla/EventStateManager.h" #include "mozilla/HTMLEditor.h" #include "mozilla/KeyEventHandler.h" +#include "mozilla/NativeKeyBindingsType.h" #include "mozilla/Preferences.h" #include "mozilla/ShortcutKeys.h" #include "mozilla/StaticPtr.h" @@ -21,6 +22,7 @@ #include "mozilla/dom/Event.h" #include "mozilla/dom/EventBinding.h" #include "mozilla/dom/KeyboardEvent.h" +#include "mozilla/widget/IMEData.h" #include "nsAtom.h" #include "nsCOMPtr.h" #include "nsContentUtils.h" @@ -29,6 +31,7 @@ #include "nsIContent.h" #include "nsIContentInlines.h" #include "nsIDocShell.h" +#include "nsIWidget.h" #include "nsNetUtil.h" #include "nsPIDOMWindow.h" @@ -396,11 +399,26 @@ bool GlobalKeyListener::IsReservedKey(WidgetKeyboardEvent* aKeyEvent, return false; } - if (reserved == ReservedKey_True) { - return true; + if (reserved != ReservedKey_True && + !nsContentUtils::ShouldBlockReservedKeys(aKeyEvent)) { + return false; } - return nsContentUtils::ShouldBlockReservedKeys(aKeyEvent); + // Okay, the key handler is reserved, but if the key combination is mapped to + // an edit command or a selection navigation command, we should not treat it + // as reserved since user wants to do the mapped thing(s) in editor. + if (MOZ_UNLIKELY(!aKeyEvent->IsTrusted() || !aKeyEvent->mWidget)) { + return true; + } + widget::InputContext inputContext = aKeyEvent->mWidget->GetInputContext(); + if (!inputContext.mIMEState.IsEditable()) { + return true; + } + return MOZ_UNLIKELY(!aKeyEvent->IsEditCommandsInitialized( + inputContext.GetNativeKeyBindingsType())) || + aKeyEvent + ->EditCommandsConstRef(inputContext.GetNativeKeyBindingsType()) + .IsEmpty(); } bool GlobalKeyListener::HasHandlerForEvent(dom::KeyboardEvent* aEvent, diff --git a/dom/events/IMEStateManager.cpp b/dom/events/IMEStateManager.cpp index 8aaf529ace83..e187eb158767 100644 --- a/dom/events/IMEStateManager.cpp +++ b/dom/events/IMEStateManager.cpp @@ -1350,6 +1350,8 @@ static bool IsNextFocusableElementTextControl(const Element* aInputContent) { static void GetInputType(const IMEState& aState, const nsIContent& aContent, nsAString& aInputType) { + // NOTE: If you change here, you may need to update + // widget::InputContext::GatNativeKeyBindings too. if (aContent.IsHTMLElement(nsGkAtoms::input)) { const HTMLInputElement* inputElement = HTMLInputElement::FromNode(&aContent); diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 6436541f25c7..ff7a7c1d6968 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -2011,16 +2011,24 @@ void BrowserParent::SendRealKeyEvent(WidgetKeyboardEvent& aEvent) { // you also need to update // TextEventDispatcher::DispatchKeyboardEventInternal(). if (aEvent.mMessage == eKeyPress) { - // XXX Should we do this only when input context indicates an editor having - // focus and the key event won't cause inputting text? - Maybe writingMode; - if (aEvent.mWidget) { - if (RefPtr dispatcher = - aEvent.mWidget->GetTextEventDispatcher()) { - writingMode = dispatcher->MaybeQueryWritingModeAtSelection(); + // If current input context is editable, the edit commands are initialized + // by TextEventDispatcher::DispatchKeyboardEventInternal(). Otherwise, + // we need to do it here (they are not necessary for the parent process, + // therefore, we need to do it here for saving the runtime cost). + if (!aEvent.AreAllEditCommandsInitialized()) { + // XXX Is it good thing that the keypress event will be handled in an + // editor even though the user pressed the key combination before the + // focus change has not been completed in the parent process yet or + // focus change will happen? If no, we can stop doing this. + Maybe writingMode; + if (aEvent.mWidget) { + if (RefPtr dispatcher = + aEvent.mWidget->GetTextEventDispatcher()) { + writingMode = dispatcher->MaybeQueryWritingModeAtSelection(); + } } + aEvent.InitAllEditCommands(writingMode); } - aEvent.InitAllEditCommands(writingMode); } else { aEvent.PreventNativeKeyBindings(); } diff --git a/widget/IMEData.h b/widget/IMEData.h index 3f1099cbcaf4..6e34f1d4c4cd 100644 --- a/widget/IMEData.h +++ b/widget/IMEData.h @@ -8,6 +8,7 @@ #include "mozilla/CheckedInt.h" #include "mozilla/EventForwards.h" +#include "mozilla/NativeKeyBindingsType.h" #include "mozilla/ToString.h" #include "nsPoint.h" @@ -420,6 +421,17 @@ struct InputContext final { return mHTMLInputType.LowerCaseEqualsLiteral("password"); } + NativeKeyBindingsType GetNativeKeyBindingsType() const { + MOZ_DIAGNOSTIC_ASSERT(mIMEState.IsEditable()); + // See GetInputType in IMEStateManager.cpp + if (mHTMLInputType.IsEmpty()) { + return NativeKeyBindingsType::RichTextEditor; + } + return mHTMLInputType.EqualsLiteral("textarea") + ? NativeKeyBindingsType::MultiLineEditor + : NativeKeyBindingsType::SingleLineEditor; + } + // https://html.spec.whatwg.org/dev/interaction.html#autocapitalization bool IsAutocapitalizeSupported() const { return !mHTMLInputType.EqualsLiteral("password") && diff --git a/widget/TextEventDispatcher.cpp b/widget/TextEventDispatcher.cpp index 60ae85e6b2ec..0139f7da4c8e 100644 --- a/widget/TextEventDispatcher.cpp +++ b/widget/TextEventDispatcher.cpp @@ -5,6 +5,7 @@ #include "TextEventDispatcher.h" +#include "IMEData.h" #include "PuppetWidget.h" #include "TextEvents.h" @@ -694,6 +695,14 @@ bool TextEventDispatcher::DispatchKeyboardEventInternal( keyEvent.mFlags.mOnlySystemGroupDispatchInContent = true; } + // If an editable element has focus and we're in the parent process, we should + // retrieve native key bindings right now because even if it matches with a + // reserved shortcut key, it should be handled by the editor. + if (XRE_IsParentProcess() && mHasFocus && + (aMessage == eKeyDown || aMessage == eKeyPress)) { + keyEvent.InitAllEditCommands(mWritingMode); + } + DispatchInputEvent(mWidget, keyEvent, aStatus); return true; } diff --git a/widget/TextEventDispatcher.h b/widget/TextEventDispatcher.h index e4f4f6bbf3b7..6ab131f0792c 100644 --- a/widget/TextEventDispatcher.h +++ b/widget/TextEventDispatcher.h @@ -542,11 +542,11 @@ class TextEventDispatcher final { * Then, WillDispatchKeyboardEvent() is always called. * @return true if an event is dispatched. Otherwise, false. */ - bool DispatchKeyboardEventInternal(EventMessage aMessage, - const WidgetKeyboardEvent& aKeyboardEvent, - nsEventStatus& aStatus, void* aData, - uint32_t aIndexOfKeypress = 0, - bool aNeedsCallback = false); + // TODO: Mark this as MOZ_CAN_RUN_SCRIPT instead. + MOZ_CAN_RUN_SCRIPT_BOUNDARY bool DispatchKeyboardEventInternal( + EventMessage aMessage, const WidgetKeyboardEvent& aKeyboardEvent, + nsEventStatus& aStatus, void* aData, uint32_t aIndexOfKeypress = 0, + bool aNeedsCallback = false); /** * ClearNotificationRequests() clears mIMENotificationRequests.