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.

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

computeAccessibleDescription throws when passed a detached element #1026

Open
jlp-craigmorten opened this issue Feb 8, 2024 · 1 comment · May be fixed by #1027
Open

computeAccessibleDescription throws when passed a detached element #1026

jlp-craigmorten opened this issue Feb 8, 2024 · 1 comment · May be fixed by #1027

Comments

@jlp-craigmorten
Copy link
Contributor

jlp-craigmorten commented Feb 8, 2024

Triaging from guidepup/virtual-screen-reader#48

When computeAccessibleDescription(element) is passed an element that is not attached to the document it throws the following error:

TypeError: root.getElementById is not a function

      at node_modules/dom-accessibility-api/sources/util.ts:103:5
          at Array.map (<anonymous>)
      at root (node_modules/dom-accessibility-api/sources/util.ts:101:17)
      at computeAccessibleDescription (node_modules/dom-accessibility-api/sources/accessible-description.ts:16:31)
      at Object.<anonymous> (src/test/int/aaa.int.test.ts:34:35)

The issue stems from https://github.com/eps1lon/dom-accessibility-api/blob/d7e6e3dbea2460777d1355406c8d0899d5346a2d/sources/util.ts#L96C16-L96C32:

  1. In the case of a detached element the node.getRootNode resolves to the node itself.
  2. getElementById() is a method unique to the Document interface and thus doesn't exist on the node and we get an error.

To reproduce:

import {
  computeAccessibleDescription,
  computeAccessibleName,
  getRole,
} from "dom-accessibility-api";

describe("Isolated Node", () => {
  // Passes
  it("should compute the role", async () => {
    const isolatedNode = document.createElement("button");
    isolatedNode.textContent = "test-button-text-content";

    expect(getRole(isolatedNode)).toEqual("button");
  });

  // Passes
  it("should compute the accessible name", async () => {
    const isolatedNode = document.createElement("button");
    isolatedNode.textContent = "test-button-text-content";

    expect(computeAccessibleName(isolatedNode)).toEqual(
      isolatedNode.textContent
    );
  });

  // Fails with `TypeError: root.getElementById is not a function`
  it("should compute the accessible description", async () => {
    const html = `<button id="button" aria-describedby="description">test-button-text-content</button>
    <p id="description">
      test-description-text-content
    </p>`;

    const isolatedNode = document.createElement("body");
    isolatedNode.innerHTML = html;

    expect(
      computeAccessibleDescription(isolatedNode.querySelector("#button"))
    ).toEqual(isolatedNode.textContent);
  });
});

There are some options:

  1. Use node.ownerDocument in all cases - there appears to be precedent for just using this elsewhere in the project (opposed to feature detecting node.getRootNode). Not certain what the impact would be here?
  2. Leave the root node logic alone and use root.querySelector(`#${CSS.escape(id)}`); as an alternative to getElementById().
  3. Add docs to state that this isn't a supported usage - ideally with an error that explains why instead of an uncaught exception.
@jlp-craigmorten
Copy link
Contributor Author

Having a quick play it seems:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant