fix(aria-allowed-attr): restrict br and wbr elements to aria-hidden only #4974
Conversation
|
|
Update allowedAttr function to check element type and return only aria-hidden for br and wbr elements, as these elements should not accept other ARIA attributes per ARIA specification.
nami8824
force-pushed
the
fix/restrict-br-wbr-aria-attrs
branch
from
b181510 to
76c9db8
Compare
WilcoFiers
left a comment
WilcoFiers
left a comment
There was a problem hiding this comment.
@nami8824 Thank you very much for this contribution! It is greatly appreciated.
| const invalid = []; | ||
| const role = getRole(virtualNode); | ||
| let allowed = allowedAttr(role); | ||
| let allowed = allowedAttr(role, { vNode: virtualNode }); |
There was a problem hiding this comment.
I'm a bit reluctant about doing it this way for two reasons.
-
I think this is mixing node constraints with role constraints. I don't think this function should give both the allowed attributes based on roles, and the allowed attrs based on node names. I think we should probably have a separate method for something like this.
-
I think we should have role restrictions like this available on the standards object, so that it is configurable and findable, instead of having an inline exception for this case.
The way you've implemented this will ignore the role. I don't think that's the right thing to do here. A <br> with role=heading should allow all ARIA properties allowed on headings. Axe has a separate rule that reports using role=heading on br's as an issue. That way we keep the issues more clearly separated.
I appreciate this is going to make this work a little more complicated. @straker would you mind weighing in here too? I think it might be good to maybe propose a structure for the standards object before asking for more changes.
There was a problem hiding this comment.
Thank you for the feedback.
Regarding my original approach: the issue I was addressing was specifically about restricting ARIA attributes on certain node types (br / wbr), rather than on roles. Because of that, I initially thought it was necessary to pass the node information into allowedAttr in order to enforce node-level constraints.
About the standards object: when you mention moving these restrictions there, are you referring to lib/standards/html-elms.js? If so, would supporting this require changing the data structure in a way that affects other elements as well, or is it expected to be handled only for br / wbr?
3 participants
Closes: #3177