fix(aria-allowed-attr): restrict br and wbr elements to aria-hidden only - #4974
Conversation
b181510 to
76c9db8
Compare
WilcoFiers
left a comment
There was a problem hiding this comment.
@nami8824 Thank you very much for this contribution! It is greatly appreciated.
e5af01d to
43733a2
Compare
straker
left a comment
There was a problem hiding this comment.
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.
43733a2 to
c4af071
Compare
|
I've addressed the review feedback. Could you take another look? |
|
Wait... I see what you mean by the |
straker
left a comment
There was a problem hiding this comment.
Thanks for all the work you put into this!
There was a problem hiding this comment.
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.
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.
c4af071 to
f4eda3d
Compare
| const invalid = []; | ||
|
|
||
| for (const attrName of virtualNode.attrNames) { | ||
| if (validateAttr(attrName) && !allowedAriaAttrs.includes(attrName)) { |
There was a problem hiding this comment.
I removed validateAttr() because any attribute returned by getGlobalAriaAttrs() is already guaranteed to exist in standards.ariaAttrs.
|
Thanks for the review. I've updated |
## [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 🤖 🎉
Closes: #3177