Skip to content

feat(matches): replace hasAccessibleName matcher with a re-entrancy-safe hasNameFromAuthor #5263

Description

@WilcoFiers

Summary

The hasAccessibleName matcher runs a full accessible name computation, which re-enters getElementSpec. Replace it with a matcher that only reads the node's own naming attributes and resolved idrefs, consolidate the near-duplicate private helper in implicit-html-roles.js into the same function, and remove the special case in get-element-spec.js that currently breaks the cycle.

Motivated by the design discussion on #5262, where relational matchers (withAncestor and similar) would let definitions nest and bypass the existing guard.

Background

There are two functions doing almost the same job with different semantics:

  • lib/commons/matches/has-accessible-name.js — the matcher, calls accessibleTextVirtual (full accname).
  • lib/commons/standards/implicit-html-roles.js:57-70 — a private helper used by the aside, form and section implicit roles. Its comment already describes this hazard: "can't go through the normal accessible name computation as it leads into an infinite loop of asking for the role of the element while the implicit role needs the name."

Both reach getElementSpec, the first directly and the second via arialabelledbyText:

getElementSpec → variant matches → accname → nativeTextAlternative / subtreeTextgetElementSpec

The cycle is currently broken by a shape check in lib/commons/standards/get-element-spec.js:33-37, gated on the noMatchAccessibleName option.

This is not a live bug. It holds because of four facts that aren't asserted anywhere:

  1. img.variant.nonEmptyAlt is the only variant using hasAccessibleName.
  2. Both getElementSpec calls inside the accname computation pass noMatchAccessibleName: true (lib/commons/text/native-text-alternative.js:40, lib/commons/text/subtree-text.js:22).
  3. The one unguarded call reachable from inside accname (lib/commons/aria/implicit-role.js:34, the chromium branch) only fires for elements with no implicitHtmlRoles entry, and none of those have accname-dependent variants.
  4. Definitions can't nest, so the top-level hasOwnProperty check sees everything.

Point 4 stops being true as soon as matchers can take nested definitions. Point 1 stops being true the moment anyone adds a second one.

Additional problems with the current guard

  • It does return standard, skipping the variant.default merge that the normal path performs, and leaking the variant key that the normal path strips.
  • img has no top-level contentTypes — they live only in variant.usemap and variant.default (lib/standards/html-elms.js:381-394). subtree-text.js:22 reads contentTypes through the guarded call, so for <img> it comes back undefined and the contentTypes?.includes('embedded') early return never fires.

Proposal

Add lib/commons/aria/has-name-from-author.js, taking (vNode, { checkTitle = false }) and returning true when any of the following hold:

  • aria-label has non-empty content after sanitize (via arialabelText, which uses getAriaValue and is node-local)
  • aria-labelledby resolves to at least one existing element (via getResolvedRefs, without computing their accessible text)
  • checkTitle and title is non-empty after sanitize

getResolvedRefs imports only getRootNode, tokenList / nodeLookup / getNodeFromTree and standards, so resolving the refs without computing their text is safe.

Then:

  • Replace the private helper in implicit-html-roles.js. Keep the existing per-caller options: aside passes checkTitle: true, form and section don't.
  • Add a hasNameFromAuthor matcher wrapping it with checkTitle: true.
  • Reduce matches.hasAccessibleName to a thin alias of hasNameFromAuthor, marked @deprecated, so the full accname computation can no longer be reached from any matcher. Its only in-repo consumer is img.variant.nonEmptyAlt; no rule or check uses it as a definition key.
  • Remove the noMatchAccessibleName option and the accname bail-out loop at get-element-spec.js:33-37, plus the option at its two call sites. This also fixes the img.contentTypes bug above. The defaults merge at get-element-spec.js:49-56 is unaffected.

This is also what severs the module cycle. commons/matches/semantic-role.js currently reaches accessible-text-virtual via get-roleimplicit-roleimplicit-html-rolesarialabelledby-text. Aliasing the matcher alone would not break that; replacing arialabelledbyText inside implicit-html-roles.js does.

Behaviour changes

aria-labelledby resolving to an element with no accessible text. <section aria-labelledby="d"> with <div id="d"></div> currently has no accessible name and no role; afterwards it counts as named and gets role=region. Same for form and aside. aria-labelledby="does-not-exist" is unaffected, since the ref doesn't resolve. This is an authoring bug in practice, but it is a real change and needs an integration sweep, not just unit tests.

hasAccessibleName matcher semantics. Since it becomes an alias, custom rules or axe.configure data using it will get the weaker predicate. This is a breaking change for that (narrow) API surface: needs a BREAKING CHANGE footer, an @deprecated tag pointing at the new name, and a CHANGELOG migration note.

Tasks

  • Add has-name-from-author.js with JSDoc stating explicitly that it does not compute referenced text, and why
  • Unit tests: whitespace-only aria-label, unresolvable idref, idref resolving to empty content, element internals, reflected properties, title with and without checkTitle, SerialVirtualNode
  • Replace the private helper in implicit-html-roles.js, preserving per-caller checkTitle
  • Add the hasNameFromAuthor matcher; reduce hasAccessibleName to a deprecated alias
  • Migrate img.variant.nonEmptyAlt in lib/standards/html-elms.js
  • Remove the noMatchAccessibleName option and the bail-out loop at get-element-spec.js:33-37, plus the option at its two call sites
  • Regression test that <img> resolves contentTypes through subtreeText
  • Regression and virtual-rule tests for the section / form / aside role changes
  • Verify commons/matches/** no longer reaches text/accessible-text-virtual through any import path
  • Update doc/API.md and CHANGELOG.md

Suggested sequencing: do the implicit-html-roles.js replacement and its test sweep as its own commit first, then the matcher work on top. If the sweep turns up more fallout than expected, there is still the option of keeping the bounded behaviour for those three roles and accepting that the module cycle survives.

Out of scope

Build-time validation of which matchers are permitted in htmlElms variant data (including disallowing condition and function-valued matchers there). Related and worth filing separately — after this change there should be no grandfathered exceptions left, which is what makes such a check enforceable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    featNew feature or enhancement

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions