From d2fc219d5291a55df642cd8d87599e82853c6549 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Mon, 21 Feb 2022 06:51:52 +0000 Subject: [PATCH] [FIX] web: protect key navigation in editable Before this commit The hotkey service was not taking into account that a new hotkey press may have been triggered from an editable element. This led to unwanted behavior, i.e. pressing ArrowLeft on an input inside a sub Dropdown closed the sub Dropdown instead of navigating through the text written in the input. After this commit The hotkey service now detects if the hotkey press was triggered from an editable element (any input, textarea or other element with [contenteditable=true] attribute) and does not dispatch this hotkey, unless a candidate registered handler has explicitly asked to bypass the editable protection. In that case, its callback will still get called. task-2764821 Part-of: odoo/odoo#84969 --- .../src/core/commands/command_palette.js | 12 +++-- .../src/core/commands/command_service.js | 1 + .../static/src/core/hotkeys/hotkey_hook.js | 3 ++ .../static/src/core/hotkeys/hotkey_service.js | 22 +++++++-- .../core/hotkeys/hotkey_service_tests.js | 48 +++++++++++++++++++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/core/commands/command_palette.js b/addons/web/static/src/core/commands/command_palette.js index db51d63224b..4592bc3b5ba 100644 --- a/addons/web/static/src/core/commands/command_palette.js +++ b/addons/web/static/src/core/commands/command_palette.js @@ -84,9 +84,15 @@ export class CommandPalette extends Component { this.defaultDebounceSearch = debounce.apply(this, [this.search, 0]); useAutofocus(); - useHotkey("Enter", () => this.executeSelectedCommand()); - useHotkey("ArrowUp", () => this.selectCommandAndScrollTo("PREV"), { allowRepeat: true }); - useHotkey("ArrowDown", () => this.selectCommandAndScrollTo("NEXT"), { allowRepeat: true }); + useHotkey("Enter", () => this.executeSelectedCommand(), { bypassEditableProtection: true }); + useHotkey("ArrowUp", () => this.selectCommandAndScrollTo("PREV"), { + bypassEditableProtection: true, + allowRepeat: true, + }); + useHotkey("ArrowDown", () => this.selectCommandAndScrollTo("NEXT"), { + bypassEditableProtection: true, + allowRepeat: true, + }); /** * @type {{ commands: CommandItem[], diff --git a/addons/web/static/src/core/commands/command_service.js b/addons/web/static/src/core/commands/command_service.js index d5bae4b7066..c0f117cd3a2 100644 --- a/addons/web/static/src/core/commands/command_service.js +++ b/addons/web/static/src/core/commands/command_service.js @@ -64,6 +64,7 @@ export const commandService = { let isPaletteOpened = false; hotkeyService.add("control+k", openMainPalette, { + bypassEditableProtection: true, global: true, }); diff --git a/addons/web/static/src/core/hotkeys/hotkey_hook.js b/addons/web/static/src/core/hotkeys/hotkey_hook.js index ced0a7dee92..1ab5a2511c4 100644 --- a/addons/web/static/src/core/hotkeys/hotkey_hook.js +++ b/addons/web/static/src/core/hotkeys/hotkey_hook.js @@ -13,6 +13,9 @@ const { useEffect } = owl; * @param {Object} options additional options * @param {boolean} [options.allowRepeat=false] * allow registration to perform multiple times when hotkey is held down + * @param {boolean} [options.bypassEditableProtection=false] + * if true the hotkey service will call this registration + * even if an editable element is focused * @param {boolean} [options.global=false] * allow registration to perform no matter the UI active element */ diff --git a/addons/web/static/src/core/hotkeys/hotkey_service.js b/addons/web/static/src/core/hotkeys/hotkey_service.js index 81463b46382..81ae0cdbf06 100644 --- a/addons/web/static/src/core/hotkeys/hotkey_service.js +++ b/addons/web/static/src/core/hotkeys/hotkey_service.js @@ -17,12 +17,11 @@ const NAV_KEYS = [ "end", "backspace", "enter", - "escape", "tab", "delete", ]; const MODIFIERS = new Set(["alt", "control", "shift"]); -const AUTHORIZED_KEYS = new Set([...ALPHANUM_KEYS, ...NAV_KEYS]); +const AUTHORIZED_KEYS = new Set([...ALPHANUM_KEYS, ...NAV_KEYS, "escape"]); export const hotkeyService = { dependencies: ["ui"], @@ -86,9 +85,16 @@ export const hotkeyService = { return; } + const targetIsEditable = + (event.target instanceof Element && /input|textarea/i.test(event.target.tagName)) || + (event.target instanceof HTMLElement && event.target.isContentEditable); + const shouldProtectEditable = + targetIsEditable && [...ALPHANUM_KEYS, ...NAV_KEYS].includes(singleKey); + // Finally, prepare and dispatch. const infos = { hotkey, + shouldProtectEditable, _originalEvent: event, }; dispatch(infos); @@ -104,12 +110,13 @@ export const hotkeyService = { * * @param {{ * hotkey: string, + * shouldProtectEditable: boolean, * _originalEvent: KeyboardEvent * }} infos */ function dispatch(infos) { let dispatched = false; - const { hotkey, _originalEvent: event } = infos; + const { hotkey, shouldProtectEditable, _originalEvent: event } = infos; const activeElement = ui.activeElement; // Dispatch actual hotkey to all matching registrations @@ -126,6 +133,10 @@ export const hotkeyService = { continue; } + if (!reg.bypassEditableProtection && shouldProtectEditable) { + continue; + } + reg.callback(); dispatched = true; break; @@ -254,6 +265,9 @@ export const hotkeyService = { * @param {Object} options additional options * @param {boolean} [options.allowRepeat=false] * allow registration to perform multiple times when hotkey is held down + * @param {boolean} [options.bypassEditableProtection=false] + * if true the hotkey service will call this registration + * even if an editable element is focused * @param {boolean} [options.global=false] * allow registration to perform no matter the UI active element * @returns {number} registration token @@ -299,6 +313,7 @@ export const hotkeyService = { callback, activeElement: null, allowRepeat: options && options.allowRepeat, + bypassEditableProtection: options && options.bypassEditableProtection, global: options && options.global, }; registrations.set(token, registration); @@ -327,6 +342,7 @@ export const hotkeyService = { * @param {() => void} callback * @param {Object} options * @param {boolean} [options.allowRepeat=false] + * @param {boolean} [options.bypassEditableProtection=false] * @param {boolean} [options.global=false] * @returns {() => void} */ diff --git a/addons/web/static/tests/core/hotkeys/hotkey_service_tests.js b/addons/web/static/tests/core/hotkeys/hotkey_service_tests.js index cbce9621e6a..fd337d91ff3 100644 --- a/addons/web/static/tests/core/hotkeys/hotkey_service_tests.js +++ b/addons/web/static/tests/core/hotkeys/hotkey_service_tests.js @@ -615,3 +615,51 @@ QUnit.test("replace the overlayModifier for MacOs", async (assert) => { assert.verifySteps([]); }); + +QUnit.test("protects editable elements", async (assert) => { + assert.expect(4); + class Comp extends Component { + setup() { + useHotkey("arrowleft", () => assert.step("called")); + } + } + Comp.template = xml`
`; + await mount(Comp, target, { env }); + const input = target.querySelector(".foo"); + + assert.verifySteps([]); + document.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowLeft", bubbles: true })); + await nextTick(); + assert.verifySteps(["called"]); + + input.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowLeft", bubbles: true })); + await nextTick(); + assert.verifySteps( + [], + "the callback is not getting called when it is triggered from an editable" + ); +}); + +QUnit.test("protects editable elements: can bypassEditableProtection", async (assert) => { + assert.expect(5); + class Comp extends Component { + setup() { + useHotkey("arrowleft", () => assert.step("called"), { bypassEditableProtection: true }); + } + } + Comp.template = xml`
`; + await mount(Comp, target, { env }); + const input = target.querySelector(".foo"); + + assert.verifySteps([]); + document.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowLeft", bubbles: true })); + await nextTick(); + assert.verifySteps(["called"]); + + input.dispatchEvent(new KeyboardEvent("keydown", { key: "ArrowLeft", bubbles: true })); + await nextTick(); + assert.verifySteps( + ["called"], + "the callback still gets called even if triggered from an editable" + ); +});