fix(aria-prohibited-attr): visible aria-labelledby requires review only - #5285
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the aria-prohibited-attr check behavior so that certain prohibited aria-labelledby cases are reported as incomplete (needs review) instead of violations, aligning the rule more closely with accessible-name computation and reducing false positives when labels are coming from visible references.
Changes:
- Add an
aria-labelledby-specific “needs review” path inaria-prohibited-attr-evaluatewhen references resolve to elements visible to screen readers. - Expand unit + integration coverage for visible/missing/hidden
aria-labelledbyreference scenarios (including shadow DOM). - Add a virtual-rule regression test to ensure prohibited
aria-labelledbyhandling doesn’t throw on pure virtual nodes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/integration/virtual-rules/aria-prohibited-attr.js | Adds a virtual-rule test to ensure prohibited aria-labelledby does not throw and continues to fail in virtual-only contexts. |
| test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.json | Updates integration expectations to include new incomplete/violation fixtures. |
| test/integration/rules/aria-prohibited-attr/aria-prohibited-attr.html | Adds new fixtures for visible/missing and hidden aria-labelledby references. |
| test/checks/aria/aria-prohibited-attr.js | Adds targeted unit tests for visible/missing/hidden aria-labelledby reference behavior, including multiple refs and shadow DOM. |
| lib/checks/aria/aria-prohibited-attr-evaluate.js | Implements the new “needs review” decision logic using getResolvedRefs + isVisibleToScreenReaders. |
Suppressed comments (1)
lib/checks/aria/aria-prohibited-attr-evaluate.js:112
- Issue #5180’s acceptance text says to report needs-review when at least one
aria-labelledbyreference is visible to screen readers, but the current check requires all resolved refs to be visible (every). Please confirm the intended behavior for mixed visibility (e.g., one visible ref and one hidden ref); if the requirement truly is “at least one visible”, this should besome(...)(and any hidden-ref fail logic would need to be handled separately).
// Does Acc name fall back to prohibited aria-label?
return !prohibited.includes('aria-label');
}
return resolved.every(ref => isVisibleToScreenReaders(ref));
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+61
to
+69
| @@ -61,6 +62,12 @@ export default function ariaProhibitedAttrEvaluate( | |||
| messageKey += prohibited.length > 1 ? 'Plural' : 'Singular'; | |||
| this.data({ role, nodeName, messageKey, prohibited }); | |||
|
|
|||
| // Don't fail if aria-labelledby would provide the accessible name | |||
| // @see https://github.com/dequelabs/axe-core/issues/5180 | |||
| if (hasIncompleteLabelledbyEdgeCase(virtualNode, prohibited)) { | |||
| return undefined; | |||
| } | |||
straker
approved these changes
Aug 5, 2026
straker
left a comment
straker
left a comment
Contributor
There was a problem hiding this comment.
Small question, but doesn't block the pr.
| assert.isUndefined(checkEvaluate.apply(checkContext, params)); | ||
| }); | ||
|
|
||
| it('should return undefined for a visible reference inside shadow DOM', () => { |
Contributor
There was a problem hiding this comment.
Should we add a test case for the element referencing across shadow DOM boundaries? It would be the same as missing reference, but would make sure aria-labelledby isn't crossing boundaries in the prohibited lookup
WilcoFiers
added a commit
that referenced
this pull request
Aug 5, 2026
All notable changes to this project will be documented in this file. See [commit-and-tag-version](https://github.com/absolute-version/commit-and-tag-version) for commit guidelines. ## [4.13.0](v4.12.1...v4.13.0) (2026-08-05) ### Features - **aria-actions:** add aria-actions to allowed ARIA attributes ([#5200](#5200)) ([029655d](029655d)), closes [#4584](#4584) [#5199](#5199), references [#5215](#5215) [#5215](#5215) - **aria-allowed-attr:** flag deprecated ARIA attributes as needs-review ([#5246](#5246)) ([518f3cc](518f3cc)), closes [#3341](#3341) - **aria-prohibited-attr:** allow many elements to be named and disallow label and body from being named ([#5259](#5259)) ([d8b1ea5](d8b1ea5)) - **aria-roles:** add sectionheader and sectionfooter roles ([#5238](#5238)) ([c36c109](c36c109)), closes [#4734](#4734), references [#4734](#4734) - **aria/get-aria-value:** new function to get aria values of a node ([#5109](#5109)) ([a7d8f3e](a7d8f3e)), references [#5042](#5042) - **aria/has-attr-value:** new function to check if node has aria value ([#5136](#5136)) ([61f2624](61f2624)), references [#5109](#5109) - **aria:** support role=image as equivalent to role=img ([#5248](#5248)) ([5aa8aaf](5aa8aaf)), closes [#4656](#4656), references [#5272](#5272) - **checks/aria:** support ARIA element internals properties ([#5172](#5172)) ([9b7f754](9b7f754)) - **checks/label:** support ARIA element internals properties ([#5170](#5170)) ([21c5f8b](21c5f8b)) - **checks/navigation:** support ARIA element internals properties ([#5167](#5167)) ([2c3a98f](2c3a98f)) - **commons/aria:** support ARIA element internals properties ([#5171](#5171)) ([31f09e7](31f09e7)) - **commons/dom:** support ARIA element internals properties ([#5163](#5163)) ([f0a12cf](f0a12cf)) - **commons/forms:** support ARIA element internals properties ([#5165](#5165)) ([27a4686](27a4686)) - **commons/matches/fromPrimative:** deprecate in favor of correct spelling ([#5270](#5270)) ([31cfb2e](31cfb2e)) - **commons/text:** support ARIA element internals properties ([#5169](#5169)) ([e841a33](e841a33)) - **commons/text:** support form-associated labels via element internals ([#5182](#5182)) ([57cfe0a](57cfe0a)), closes [#5045](#5045), references [#5170](#5170) [#5039](#5039) [#5151](#5151) [#5039](#5039) - **dom/getResolvedRefs:** new function to get the resolved virtual nodes of idrefs ([#5151](#5151)) ([489cdea](489cdea)), references [#5109](#5109) - **element-internals:** enable ElementInternals by default ([#5284](#5284)) ([2740d42](2740d42)), closes [#5277](#5277) - **i18n:** Add Swedish locale ([#5190](#5190)) ([dcd13f2](dcd13f2)), references [#5189](#5189) - **matches:** add inSectioningContent, hasChild, and isSummaryForDetails matches ([#5262](#5262)) ([c47cdcd](c47cdcd)) - **rules:** support ARIA element internals properties ([#5168](#5168)) ([065baf7](065baf7)) - **standards/ariaAttrs:** add caseInsensitive property for attributes ([#5224](#5224)) ([bcd791c](bcd791c)) ### Bug Fixes - **aria-allowed-role:** allow roles on a non-details summary ([#5242](#5242)) ([3bd9875](3bd9875)), closes [#3911](#3911), references [#3443](#3443) [#3911](#3911) - **aria-allowed-role:** restrict figure roles with child figcaption ([#5240](#5240)) ([178a635](178a635)), closes [#3443](#3443) - **aria-prohibited-attr:** visible aria-labelledby requires review only ([#5285](#5285)) ([fd6fa9f](fd6fa9f)) - **axe.d.ts:** make enabled property of RuleMetadata optional ([#5129](#5129)) ([90fce18](90fce18)) - **color-contrast:** fix various stacking context bugs ([#5214](#5214)) ([d5e5b04](d5e5b04)), references [#8](#8) [#5213](#5213) - **gather-internals:** handle non-HTMLElement nodes ([#5161](#5161)) ([06e84c3](06e84c3)) - **get-selector:** escape control characters in attribute selectors ([#5273](#5273)) ([4b60ac5](4b60ac5)), closes [#5204](#5204) [#5204](#5204) - **image-alt:** allow whitespace alt on presentational images ([#5218](#5218)) ([c5dd0ef](c5dd0ef)), closes [#5216](#5216) - **landmark-unique:** exclude section/form with non-landmark roles from landmark match ([#5085](#5085)) ([c5fd013](c5fd013)), closes [#4722](#4722) [#5064](#5064) - name the image role in role-img-alt and svg-img-alt metadata ([#5279](#5279)) ([995a269](995a269)), closes [#5272](#5272), references [#5248](#5248) [#5248](#5248) - **standards:** update aria-errormessage and aria-details to be idrefs ([#5157](#5157)) ([fb94f8a](fb94f8a)) This PR was opened by a robot 🤖 🎉
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If the only prohibited attribute is aria-labelledby, it doesn't reference hidden elements, mark the element as needs review. Ignore aria-label as a prohibited attribute if this is the case, as the acc name fallback would have skipped it anyway.
Closes: #5180, closes #4118