feat(matches): replace hasAccessibleName matcher with a re-entrancy-safe hasNameFromAuthor
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, callsaccessibleTextVirtual(full accname).lib/commons/standards/implicit-html-roles.js:57-70— a private helper used by theaside,formandsectionimplicit 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 / subtreeText → getElementSpec
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:
img.variant.nonEmptyAltis the only variant usinghasAccessibleName.- Both
getElementSpeccalls inside the accname computation passnoMatchAccessibleName: true(lib/commons/text/native-text-alternative.js:40,lib/commons/text/subtree-text.js:22). - The one unguarded call reachable from inside accname (
lib/commons/aria/implicit-role.js:34, thechromiumbranch) only fires for elements with noimplicitHtmlRolesentry, and none of those have accname-dependent variants. - Definitions can't nest, so the top-level
hasOwnPropertycheck 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 thevariant.defaultmerge that the normal path performs, and leaking thevariantkey that the normal path strips. imghas no top-levelcontentTypes— they live only invariant.usemapandvariant.default(lib/standards/html-elms.js:381-394).subtree-text.js:22readscontentTypesthrough the guarded call, so for<img>it comes backundefinedand thecontentTypes?.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-labelhas non-empty content aftersanitize(viaarialabelText, which usesgetAriaValueand is node-local)aria-labelledbyresolves to at least one existing element (viagetResolvedRefs, without computing their accessible text)checkTitleandtitleis non-empty aftersanitize
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:asidepassescheckTitle: true,formandsectiondon't. - Add a
hasNameFromAuthormatcher wrapping it withcheckTitle: true. - Reduce
matches.hasAccessibleNameto a thin alias ofhasNameFromAuthor, marked@deprecated, so the full accname computation can no longer be reached from any matcher. Its only in-repo consumer isimg.variant.nonEmptyAlt; no rule or check uses it as a definition key. - Remove the
noMatchAccessibleNameoption and the accname bail-out loop atget-element-spec.js:33-37, plus the option at its two call sites. This also fixes theimg.contentTypesbug above. The defaults merge atget-element-spec.js:49-56is unaffected.
This is also what severs the module cycle. commons/matches/semantic-role.js currently reaches accessible-text-virtual via get-role → implicit-role → implicit-html-roles → arialabelledby-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.jswith 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,titlewith and withoutcheckTitle,SerialVirtualNode - Replace the private helper in
implicit-html-roles.js, preserving per-callercheckTitle - Add the
hasNameFromAuthormatcher; reducehasAccessibleNameto a deprecated alias - Migrate
img.variant.nonEmptyAltinlib/standards/html-elms.js - Remove the
noMatchAccessibleNameoption and the bail-out loop atget-element-spec.js:33-37, plus the option at its two call sites - Regression test that
<img>resolvescontentTypesthroughsubtreeText - Regression and virtual-rule tests for the
section/form/asiderole changes - Verify
commons/matches/**no longer reachestext/accessible-text-virtualthrough any import path - Update
doc/API.mdandCHANGELOG.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.
Source: dequelabs/axe-core