Skip to content

fix(aria-allowed-attr): restrict br and wbr elements to aria-hidden only - #4974

Merged
straker merged 3 commits into
dequelabs:developfrom
nami8824:fix/restrict-br-wbr-aria-attrs
Apr 7, 2026
Merged

fix(aria-allowed-attr): restrict br and wbr elements to aria-hidden only#4974
straker merged 3 commits into
dequelabs:developfrom
nami8824:fix/restrict-br-wbr-aria-attrs

Conversation

@nami8824

Copy link
Copy Markdown
Contributor

Closes: #3177

@nami8824
nami8824 requested a review from a team as a code owner December 30, 2025 16:57
@CLAassistant

CLAassistant commented Dec 30, 2025

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@nami8824
nami8824 force-pushed the fix/restrict-br-wbr-aria-attrs branch from b181510 to 76c9db8 Compare December 30, 2025 17:07

@WilcoFiers WilcoFiers 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.

@nami8824 Thank you very much for this contribution! It is greatly appreciated.

Comment thread lib/checks/aria/aria-allowed-attr-evaluate.js Outdated
@nami8824
nami8824 force-pushed the fix/restrict-br-wbr-aria-attrs branch from e5af01d to 43733a2 Compare February 11, 2026 10:25
@nami8824
nami8824 requested a review from dbjorge February 14, 2026 05:32

@straker straker 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.

Thanks for the changes. Sorry it took me so long to get to the I didn't notice you pushed changes and the pr still said changes requested. Overall it's looking, just a few things we'll need to fix before we can merge.

Comment thread lib/checks/aria/aria-allowed-attr-elm-evaluate.js
Comment thread lib/checks/aria/aria-allowed-attr-role.json Outdated
Comment thread lib/checks/aria/aria-allowed-attr-elm.json Outdated
@straker
straker requested a review from WilcoFiers March 25, 2026 22:38
@nami8824
nami8824 force-pushed the fix/restrict-br-wbr-aria-attrs branch from 43733a2 to c4af071 Compare April 1, 2026 04:31
@nami8824

nami8824 commented Apr 1, 2026

Copy link
Copy Markdown
Contributor Author

I've addressed the review feedback. Could you take another look?

@nami8824
nami8824 requested a review from straker April 1, 2026 04:45
straker
straker previously requested changes Apr 1, 2026
Comment thread lib/checks/aria/aria-allowed-attr-elm-evaluate.js
Comment thread lib/checks/aria/aria-allowed-attr-elm-evaluate.js
@straker

straker commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Wait... I see what you mean by the nodeName. Yes because the nodeName attribute is there you can't just set data(values) and that's why you are doing the values and message key yourself. Ignore everything I've said :D

@straker
straker dismissed their stale review April 1, 2026 14:46

Resolved

@straker straker 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.

Thanks for all the work you put into this!

@straker straker 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.

Sorry for the back and forth review. I found one minor thing that we should change. When the element has both global and non-global ARIA attributes, the resulting fix message is:

<br aria-busy="true" id="fail9" aria-expanded=false />

"Fix all of the following:
  ARIA attribute is not allowed: aria-expanded=\"false\"
  ARIA attributes are not allowed on br elements: aria-busy=\"true\", aria-expanded=\"false\""

This is because the aria-allowed-attr check fails for the non-global ARIA attribute aria-expanded, and then the new aria-allowed-attr-elm check also sees 2 attributes not allowed. I think the fix is to ignore non-global ARIA attributes in the new aria-allowed-elm-check that way the message doesn't have duplication.

"Fix all of the following:
  ARIA attribute is not allowed: aria-expanded=\"false\"
  ARIA attribute is not allowed on br elements: aria-busy=\"true\"

You can use the function get-global-aria-attrs to figure out which ones are global or not.

nami8824 added 3 commits April 2, 2026 10:32
Split aria-allowed-attr check into role-based (aria-allowed-attr-role)
and element-based (aria-allowed-attr-elm) checks. Add allowedAriaAttrs
property to html-elms standard to define per-element ARIA attribute
restrictions. br and wbr now only allow aria-hidden.
@nami8824
nami8824 force-pushed the fix/restrict-br-wbr-aria-attrs branch from c4af071 to f4eda3d Compare April 2, 2026 01:32
const invalid = [];

for (const attrName of virtualNode.attrNames) {
if (validateAttr(attrName) && !allowedAriaAttrs.includes(attrName)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I removed validateAttr() because any attribute returned by getGlobalAriaAttrs() is already guaranteed to exist in standards.ariaAttrs.

@nami8824

nami8824 commented Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I've updated aria-allowed-attr-elm-evaluate.js.

@nami8824
nami8824 requested a review from straker April 2, 2026 01:40

@straker straker 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.

Fantastic work. Thanks!

Reviewed for security.

@straker
straker merged commit c6245e7 into dequelabs:develop Apr 7, 2026
22 of 23 checks passed
@straker straker mentioned this pull request Apr 13, 2026
WilcoFiers added a commit that referenced this pull request Apr 13, 2026
### Bug Fixes

- **aria-allowed-attr:** restrict br and wbr elements to aria-hidden
only ([#4974](#4974))
([1d80163](1d80163))
- **target-size:** ignore position: fixed elements that are offscreen
when page is scrolled
([#5066](#5066))
([5906273](5906273)),
closes [#5065](#5065)
WilcoFiers added a commit that referenced this pull request Jun 1, 2026
##
[4.12.0](v4.11.4...v4.12.0)
(2026-06-01)

### Features

- add gather-internals.js external script
([#5099](#5099))
([c61d58b](c61d58b)),
closes [#5080](#5080)
- **aria-allowed/prohibited-attr, aria-required-parent/children:**
partially support element internals role
([#5080](#5080))
([417b48a](417b48a)),
closes [#5039](#5039)
[#4259](#4259)
- **axe.externalAPIs:** add public api for setting elementInternal data
([#5105](#5105))
([63bab8f](63bab8f))
- **core:** expose normalizeRunOptions
([#4998](#4998))
([b8e6a59](b8e6a59))
- expose axe.resetLocale() to restore the default locale
([#5108](#5108))
([c2b5292](c2b5292)),
closes [#5107](#5107)
- **getRules:** include rule enabled state in returned objects
([#5118](#5118))
([75bf772](75bf772)),
closes [#5116](#5116)
- **list,listitem:** support element internals role
([#5119](#5119))
([7d9d696](7d9d696))
- **new-rule:** check that aria-tab have an accessible name
([#5001](#5001))
([0d4e4e7](0d4e4e7)),
closes [#4842](#4842)
- **rules:** deprecate landmark-complementary-is-top-level rules
([#4992](#4992))
([9e09139](9e09139)),
closes [#4950](#4950)
- **utils:** add `getElementInternals` function
([#5077](#5077))
([1c15f82](1c15f82))

### Bug Fixes

- **aria-allowed-attr:** restrict br and wbr elements to aria-hidden
only ([#4974](#4974))
([c6245e7](c6245e7))
- **aria-conditional-attr:** add support for radio
([#5100](#5100))
([8223c98](8223c98))
- **aria-valid-attr-value:** handle multiple aria-errormessage IDs
([#4973](#4973))
([0489e30](0489e30))
- **aria:** prevent getOwnedVirtual from returning duplicate nodes
([#4987](#4987))
([48ca955](48ca955)),
closes [#4840](#4840)
- **commons/text:** exclude natively hidden elements from
aria-labelledby accessible name
([#5076](#5076))
([ea7202c](ea7202c)),
closes [#4704](#4704)
- **DqElement:** avoid calling constructors with cloneNode
([#5013](#5013))
([0281fa1](0281fa1))
- **existing-rule:** aria-busy now shows an error message for a use with
unallowed children
([#5017](#5017))
([2067b87](2067b87))
- **helpUrl:** ensure axe.configure always updates the help URLs
([#5114](#5114))
([c4f60ff](c4f60ff))
- **label-content-name-mismatch:** match visible text with aria-label
and exclude invisible text
([#5096](#5096))
([3a012a1](3a012a1))
- **locale:** ensure all subtags are correctly set
([#5112](#5112))
([13005ed](13005ed))
- **scrollable-region-focusable:** clarify the issue is in safari
([#4995](#4995))
([4ec5211](4ec5211)),
closes
[WebKit#190870](https://github.com/dequelabs/WebKit/issues/190870)
[WebKit#277290](https://github.com/dequelabs/WebKit/issues/277290)
- **scrollable-region-focusable:** do not fail scroll areas when all
content is visible without scrolling
([#4993](#4993))
([838707a](838707a))
- **target-size:** determine offset using clientRects if target is
display:inline
([#5012](#5012))
([a4b8091](a4b8091))
- **target-size:** ignore position: fixed elements that are offscreen
when page is scrolled
([#5066](#5066))
([1229a6e](1229a6e)),
closes [#5065](#5065)
- **target-size:** ignore widgets that are inline with other inline
elements ([#5000](#5000))
([a8dd81b](a8dd81b))
- **utils/getAncestry:** escape node name
([#5079](#5079))
([d1fabaa](d1fabaa)),
closes [#5078](#5078)
- **utils:** Add null check to parseCrossOriginStylesheet, closes
[#5074](#5074)
([#5075](#5075))
([f12ef32](f12ef32))
- **utils:** update isShadowRoot to use spec-compliant custom element
regex ([#5059](#5059))
([edc6ce2](edc6ce2)),
closes [#5030](#5030)

This PR was opened by a robot 🤖 🎉
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.

ARIA in HTML allowances changes for br and wbr

5 participants