eps1lon/dom-accessibility-api

`computeAccessibleDescription` throws when passed a detached element

Opened this issue · 1 comments

jlp-craigmorten commented

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 commented

Having a quick play it seems: