Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow focusable elements in FACE/ElementInternals shadow roots to be aria-hidden #4659

Closed
bennypowers opened this issue Dec 17, 2024 · 2 comments
Closed

Allow focusable elements in FACE/ElementInternals shadow roots to be aria-hidden #4659

bennypowers opened this issue Dec 17, 2024 · 2 comments
Labels
feat New feature or enhancement ungroomed Ticket needs a maintainer to prioritize and label

Comments

@bennypowers
Copy link

bennypowers commented Dec 17, 2024 •
edited
Loading

Product

axe-core

Feature Description

This issue is to address what I believe to be a false positive result (i.e. failed audit when in fact the page would otherwise pass) when certain kinds of form-associated custom elements are in use. In particular, the case I'd like to address is a type of FACE which wraps a native input in shadow dom and reimplements it's semantics on the host element.

Please consider the following page:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width">
    <title>Interactive elements aria hidden in a FACE shadow root</title>
    <script type="module">
      class FormAssociatedCheckboxElement extends HTMLElement {
        static is = 'face-checkbox';
        static formAssociated = true;
        static observedAttributes = ['checked'];
        static { customElements.define(this.is, this); }
        #internals = this.attachInternals();
        get #input() { return this.shadowRoot.getElementById('input'); }
        get checked() { return this.hasAttribute('checked'); }
        set checked(v) { this.toggleAttribute('checked', !!v); }
        attributeChangedCallback(name, old, val) {
          this.#input.checked = val !== null;
          this.#internals.ariaChecked = String(!!this.checked);
        }
        connectedCallback() {
          this.#internals.role = 'checkbox';
          this.#input.addEventListener('click', (event) => {
            this.checked = this.#input.checked;
          });
        }
      }
    </script>
  </head>
  <body>
    <label for="checkbox">Checked?</label>
    <face-checkbox id="checkbox" checked>
      <template shadowrootmode="open"
                shadowrootdelegatesfocus="true">
        <input aria-hidden="true" id="input" type="checkbox">
      </template>
    </face-checkbox>
  </body>
</html>

This page contains a label and a form associated custom element with attached element internals. The shadow root of the element delegates focus to a checkbox element in the shadow root. Because the face element is form associated, and because it manages it's own semantics via it's attached ElementInternals object, it should be acceptable for the shadow checkbox to be hidden from AT. In fact, firefox' accessibility inspector shows that because of the work of the element author, the correct states are communicated to AT:

Image

Although ORCA/FireFox apparently handle this page as expected, axe DevTools insists that this is in fact an error:

Image

I propose that in cases like these, this requirement be relaxed. axe core could take one of a variety of approaches to validating the custom element code / element internals state:

  1. when encountering a form-associated custom element with a shadowroot and an aria-hidden interactive element inside, axe-core could ignore the shadow element and advise the user to run manual testing to verify that it works with AT
  2. axe core could override the attachElements method on HTMLElement, to capture the internals object for each element in a weakmap, and use that captured reference to assert on the internal consistency of those ElementInternals objects (see Custom Elements using ElementInternals to set role are flagged when aria-label is provided #4259 (comment)):
const aXeMap = new WeakMap;
const orig = HTMLElement.prototype.attachInternals;
HTMLElement.prototype.attachInternals = function () {
  const internals = orig.call(this);
  aXeMap.set(this, internals);
  return internals;
};
@bennypowers bennypowers added feat New feature or enhancement ungroomed Ticket needs a maintainer to prioritize and label labels Dec 17, 2024
@dbjorge
Copy link
Contributor

dbjorge commented Dec 17, 2024

Hi @bennypowers,

Thanks for the report, and especially for providing a great self-contained repro! We know generally that we need to think about ElementInternals support more broadly in the near future.

This issue is to address what I believe to be a false positive result

I don't think this specific case is a false positive; this pattern really does cause a variety of accessibility problems for ATs today. With the repro page you provided above:

  • Open the page in Chromium on MacOS, enable VoiceOver, tab to the control, and observe that VoiceOver doesn't announce the control's label as part of the control's name
  • Open the page in Chromium on Windows, enable NVDA, tab to the control, and observe that NVDA double-announces the control type and state
  • Open the page in Firefox on Windows, enable NVDA, tab to the control, and observe that although it is announced correctly, it breaks keyboard operability with space/enter keys
  • Open the page on Windows (either Chromium or Firefox), enable Windows Voice Access, speak a command to click on the control, and observe that it won't recognize the command

Because the face element is form associated, and because it manages it's own semantics via it's attached ElementInternals object, it should be acceptable for the shadow checkbox to be hidden from AT. In fact, firefox' accessibility inspector shows that because of the work of the element author, the correct states are communicated to AT.

It isn't enough for some browsers to produce reasonable looking accessibility trees; WCAG (and by extension axe-core) require that new techniques meet WCAG's accessibility supported criteria before axe-core can treat the technique as acceptable.

axe core could override the attachElements method on HTMLElement

Unfortunately, axe-core can't really use this approach because axe-core needs to remain compatible with users injecting it onto a page after the page is loaded. I posted some more details in a comment on the original issue.

@WilcoFiers
Copy link
Contributor

Closing this for the reason Dan describes. There are problems with Element Internals axe-core cannot currently handle, They are covered by #4259.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feat New feature or enhancement ungroomed Ticket needs a maintainer to prioritize and label
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@dbjorge @WilcoFiers @bennypowers