Visitar URL original
fix(label-content-name-mismatch): ignore nested group content in visible text by chutchins25 · Pull Request #5509 · dequelabs/axe-core · GitHub
Skip to content

fix(label-content-name-mismatch): ignore nested group content in visible text - #5509

Draft
chutchins25 wants to merge 5 commits into
developfrom
chut/5505-treeitem-nested-group
Draft

chutchins25 wants to merge 5 commits into
developfrom
chut/5505-treeitem-nested-group

Conversation

@chutchins25

Copy link
Copy Markdown
Contributor

label-content-name-mismatch counts every visible descendant as part of an element's visible label. For an expanded tree item, that includes the text of the child items in its nested role="group". This is the APG nested tree pattern, and the reported markup is:

<li role="treeitem" aria-label="Documents" aria-expanded="true">
  <span>Documents</span>
  <ul role="group">
    <li role="treeitem" aria-label="Report"><span>Report</span></li>
    <li role="treeitem" aria-label="Notes"><span>Notes</span></li>
  </ul>
</li>

The parent's visible text came out as Documents Report Notes, so every expanded parent item was reported as a violation now that the rule is on by default. The child items pass.

Browsers already leave nested groups out of name-from-content. Chrome names that item "Documents", and the same holds for role="group foo", role="foo group" and <fieldset>; <details> content is kept. This PR makes the rule's visible text match:

  • visibleVirtual gets an opt-in excludeRoles option. A child element whose role (with fallback roles) is listed contributes no text. details is never excluded, and neither is the element itself. No other caller passes the option, so their output is unchanged.
  • The check passes a new excludeRoles option, defaulting to ["group"], documented in doc/check-options.md. Setting it to [] restores the old behavior.

Notes for reviewers:

Verification:

  • New unit tests (visibleVirtual and the check) fail without the change and pass with it. The guard tests (button in a gridcell, nested list, unnamed region still counted) were checked by widening the role list.
  • New integration fixture #pass20 is reported as a violation with excludeRoles: [] and passes with the default.
  • #fail11, a leaf tree item whose text isn't in its name, still fails. It also fails if treeitem stops being named from content, which guards against fixing this through rule applicability instead.
  • The reporter's exact markup gives three passes and no violations in Chrome 156. A real leaf mismatch is still reported.
  • eslint and prettier --check pass. test:tsc, test:unit:core/commons/rule-matches/checks/api/virtual-rules/gather-internals and test:apg pass locally.
  • test:unit:integration passes apart from identical-links-same-purpose #pass15, and test:act passes apart from 78fd32 (line height). Both fail the same way on develop locally and are unrelated to this change.

Closes: #5505

@chutchins25

Copy link
Copy Markdown
Contributor Author

Self-review summary. Two review rounds covered design and correctness, tests, and polish/conventions. The review converged clean: no Critical or High findings in either round.

Applied in round 1 (d6e67620):

  • The exclusion check runs only on the recursive path, so noRecursing calls don't pay for getRole.
  • Removed an unreachable nodeType check. The flattened tree only holds element and text nodes, and text returns earlier.
  • The leaf-treeitem mismatch moved from a unit test to integration #fail11, where it runs through the rule's matches.
  • Renamed a test so it says why it passes, and added a comment explaining #pass23.

Considered and not changed:

  • axe.configure replaces the defaults. Configuring this check's options replaces the whole options object, excludeRoles included. Every check option already works this way, valueTextRoles among them.
  • getRole runs on every element child. It only does so when the option is set, which is only for widgets this rule is already evaluating. Skipped group subtrees aren't walked.
  • The exclusion applies to whatever element is being checked. It isn't limited to tree items. This is intended and matches Chrome; see the PR description.
  • No Shadow DOM test. The code walks the same flattened tree, so a slotted group takes the same path.
  • Scope stays at group. Chrome skips other nested containers too; covering those would be a separate change.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Excluding a whitespace-only group collapses “Save report” into “Savereport,” causing a false violation for a correctly labeled link; whitespace boundaries need preservation and a regression test.

1 open issue
What changed in this PR

Fixes false label-content-name-mismatch violations for expanded tree items by excluding nested group content from their visible labels, addressing #5505.

Changes:

  • Adds an opt-in excludeRoles option to visibleVirtual, preserving the root element and details content.
  • Defaults the check to exclude group descendants and documents the option.
  • Adds unit and integration coverage for nested groups, configuration overrides, and genuine label mismatches.
File Description
test/​integration/​rules/​label-content-name-mismatch/​label-content-name-mismatch.json Registers new passing and failing fixtures.
test/​integration/​rules/​label-content-name-mismatch/​label-content-name-mismatch.html Adds nested-tree and leaf-mismatch fixtures.
test/​commons/​text/​visible-virtual.js Tests role exclusion, fallback roles, and exceptions.
test/​checks/​label/​label-content-name-mismatch.js Tests nested-group handling and retained label checks.
lib/​commons/​text/​visible-virtual.js Implements optional descendant-role exclusion.
lib/​checks/​label/​label-content-name-mismatch.json Sets the default excluded role to group.
lib/​checks/​label/​label-content-name-mismatch-evaluate.js Passes role exclusions to visible-text collection.
doc/​check-options.md Documents the new check option and default.

Review effort: Balanced

Comment thread lib/commons/text/visible-virtual.js Resolved
@github-actions

Copy link
Copy Markdown
Contributor

Performance comparison

axe metric: 🟢 No significant regression (worst: /sites/mdn at -0.8% median, -13ms)

Base 6b52b84 vs head d6e6762.

Site Cold start Median Max
/sites/mdn 2,221ms → 2,207ms (-0.6%) 1,666ms → 1,653ms (-0.8%) 1,891ms → 2,049ms (+8.4%)
/sites/very-large-page 24,102ms → 24,063ms (-0.2%) 23,371ms → 23,167ms (-0.9%) 23,393ms → 23,766ms (+1.6%)

per-metric regressions: (worst ms increases regardless of axe status)

  • /sites/very-large-page runchecks_avoid-inline-spacing: +757ms (+83.4%)
  • /sites/very-large-page rule_avoid-inline-spacing: +680ms (+14.9%)

The performance comparison bot is just a rough check for spotting problems and is not authoritative. Just because something shows as being faster or slower than the current base does not mean it is true. All performance problems should be investigated manually.

@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Performance comparison

axe metric: 🟢 No significant regression (worst: /sites/mdn at +0.1% median, +1ms)

Base 6b52b84 vs head 26df381.

Site Cold start Median Max
/sites/mdn 2,132ms → 2,236ms (+4.9%) 1,713ms → 1,715ms (+0.1%) 1,906ms → 1,961ms (+2.9%)
/sites/very-large-page 25,207ms → 24,408ms (-3.2%) 23,898ms → 23,651ms (-1.0%) 24,446ms → 24,319ms (-0.5%)

The performance comparison bot is just a rough check for spotting problems and is not authoritative. Just because something shows as being faster or slower than the current base does not mean it is true. All performance problems should be investigated manually.

This branch has not been deployed

No deployments
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 this pull request may close these issues.

label-content-name-mismatch: treeitem with a nested group fails because the child treeitems' text counts as its visible label

2 participants