[FIX] web: conflicting accesskey/data-hotkey attrs

Issue:
Sometime an element with an [accesskey] attribute could be triggered
through the standard mechanism (accesskeys) instead of the hotkey
service mechanism (elements with [data-hotkey] attributes).

Steps to reproduce:
- open a record on a form view with a pager
- open some dialog (without using the keyboard)
- use ALT+N => the pager behind the dialog switch to the next record

This comes from the browser default mechanism for elements with an
[accesskey] attribute.

The hotkey service should have taken over on this standard behavior but
it did not because of the dialog which became the UI active element.

NB: this occurs only on a certain set of browser/OS combination. See the
following table https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/accesskey#try_it

Before this commit:
The hotkey service would take over the accesskey standard behavior ONLY
inside the UI active element.

After this commit:
The hotkey service will always take over the accesskey standard
behavior, disregarding the UI active element.

closes odoo/odoo#91261

X-original-commit: 582963a6a18ea3a2e8d3e050af4978067bfe21e1
Signed-off-by: Bruno Boi <boi@odoo.com>
Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

addons/web/static/src/core/hotkeys/hotkey_service.js

@@ -70,14 +70,15 @@ export const hotkeyService = {
                 return;
             }
 
-            // FIXME : this is a temporary hack. It replaces all [accesskey] attrs by [data-hotkey] on all elements.
-            const elementsWithoutDataHotkey = getVisibleElements(
-                activeElement,
-                "[accesskey]:not([data-hotkey])"
-            );
-            for (const el of elementsWithoutDataHotkey) {
-                el.dataset.hotkey = el.accessKey;
-                el.removeAttribute("accesskey");
+            // Replace all [accesskey] attrs by [data-hotkey] on all elements.
+            // This is needed to take over on the default accesskey behavior
+            // and also to avoid any conflict with it.
+            const elementsWithAccessKey = document.querySelectorAll("[accesskey]");
+            for (const el of elementsWithAccessKey) {
+                if (el instanceof HTMLElement) {
+                    el.dataset.hotkey = el.accessKey;
+                    el.removeAttribute("accesskey");
+                }
             }
 
             // Special case: open hotkey overlays

addons/web/static/tests/core/hotkeys/hotkey_service_tests.js

@@ -15,7 +15,7 @@ import {
     triggerHotkey,
 } from "../../helpers/utils";
 
-const { Component, xml } = owl;
+const { Component, useState, xml } = owl;
 const serviceRegistry = registry.category("services");
 
 let env;
@@ -52,6 +52,104 @@ QUnit.test("register / unregister", async (assert) => {
     assert.verifySteps([key]);
 });
 
+QUnit.test("[accesskey] attrs replaced by [data-hotkey]", async (assert) => {
+    const div = document.createElement("div");
+    div.className = "foo";
+    div.accessKey = "a";
+    div.onclick = () => assert.step("click");
+    div.textContent = "foo";
+    target.appendChild(div);
+
+    // div must only have [accesskey] attribute
+    assert.containsOnce(target, ".foo");
+    assert.containsOnce(target, ".foo[accesskey]");
+    assert.containsNone(target, ".foo[data-hotkey]");
+
+    // press any hotkey, i.e. the left arrow
+    triggerHotkey("arrowleft");
+    await nextTick();
+
+    // div should now only have [data-hotkey] attribute
+    assert.containsOnce(target, ".foo");
+    assert.containsOnce(target, ".foo[data-hotkey]");
+    assert.containsNone(target, ".foo[accesskey]");
+
+    // try to press the related hotkey, just to make sure it works
+    assert.verifySteps([]);
+    triggerHotkey("a", true);
+    await nextTick();
+    assert.verifySteps(["click"]);
+});
+
+QUnit.test("[accesskey] attrs replaced by [data-hotkey], part 2", async (assert) => {
+    class UIOwnershipTakerComponent extends Component {
+        setup() {
+            useActiveElement("bouh");
+        }
+    }
+    UIOwnershipTakerComponent.template = xml`<p class="owner" t-ref="bouh">bouh</p>`;
+    class MyComponent extends Component {
+        setup() {
+            this.state = useState({ foo: true });
+            this.step = assert.step.bind(assert);
+        }
+    }
+    MyComponent.components = { UIOwnershipTakerComponent };
+    MyComponent.template = xml`
+        <main>
+            <UIOwnershipTakerComponent t-if="state.foo" />
+            <div t-on-click="() => { this.step('click'); }" accesskey="a">foo</div>
+        </main>
+    `;
+    const comp = await mount(MyComponent, target, { env });
+
+    // UIOwnershipTakerComponent should be there and it should be the ui active element
+    assert.containsOnce(target, "main .owner");
+    assert.strictEqual(target.querySelector("main .owner"), env.services.ui.activeElement);
+
+    // div must only have [accesskey] attribute
+    assert.containsOnce(target, "main div");
+    assert.containsOnce(target, "main div[accesskey]");
+    assert.containsNone(target, "main div[data-hotkey]");
+
+    // press any hotkey, i.e. the left arrow
+    triggerHotkey("arrowleft");
+    await nextTick();
+
+    // div should now only have [data-hotkey] attribute
+    assert.containsOnce(target, "main div");
+    assert.containsOnce(target, "main div[data-hotkey]");
+    assert.containsNone(target, "main div[accesskey]");
+
+    // try to press the related hotkey, it should not work as the ui active element is different
+    assert.notEqual(
+        env.services.ui.getActiveElementOf(target.querySelector("main div")),
+        env.services.ui.activeElement
+    );
+    assert.verifySteps([]);
+    triggerHotkey("a", true);
+    await nextTick();
+    assert.verifySteps([]);
+
+    // remove the UIOwnershipTakerComponent
+    comp.state.foo = false;
+    await nextTick();
+    assert.strictEqual(
+        env.services.ui.getActiveElementOf(target.querySelector("main div")),
+        env.services.ui.activeElement
+    );
+
+    assert.containsNone(target, "main .owner");
+    assert.containsOnce(target, "main div");
+    assert.containsOnce(target, "main div[data-hotkey]");
+    assert.containsNone(target, "main div[accesskey]");
+
+    assert.verifySteps([]);
+    triggerHotkey("a", true);
+    await nextTick();
+    assert.verifySteps(["click"]);
+});
+
 QUnit.test("data-hotkey", async (assert) => {
     assert.expect(2);