Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(aria-allowed-attr): pass aria-label on some HTML elements #2935

Merged
merged 6 commits into from
May 18, 2021

Conversation

WilcoFiers
Copy link
Contributor

@WilcoFiers WilcoFiers commented May 17, 2021

Allow aria-label and aria-labelledby on the following elements, if those elements have no role.

  • audio
  • applet
  • canvas
  • dl
  • embed
  • iframe
  • input
  • label
  • meter
  • object
  • svg
  • video

Closes issue: #2933, #2926

I did a bunch of testing with JAWS, NVDA and VoiceOver to come to this list, here are my test results:
https://codepen.io/wilcofiers/pen/WNpwMNj?editors=1000

"label" is the only one that isn't supported that I added anyway, because if you do <label aria-label="foo" for="bar"></label>, that label will be used in the accessible name of whatever form control has id="bar". dl is on the list (unlike ul, and ol) because it doesn't have an implicit role.

Allow aria-label and aria-labelledby on the following elements, if those elements have no role.
- audio
- applet
- canvas
- dl
- embed
- iframe
- input
- label
- meter
- object
- span
- svg
- video
@WilcoFiers WilcoFiers requested a review from a team as a code owner May 17, 2021 11:30
"label",
"meter",
"object",
"span",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think span should be allowed

Copy link

@zelliott zelliott Jun 25, 2021

Choose a reason for hiding this comment

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

@straker Are you able to link to the specific section of the ARIA spec that disallows aria-label on span? I'm looking through https://www.w3.org/TR/html-aria/, but I can't find anything that disallows it. It appears as though span allows "Global aria-* attributes", of which aria-label is one (per https://www.w3.org/TR/wai-aria-1.2/#global_states). Of course that list has the carve-out "Except where prohibited" for aria-label, but I can't find anything that prohibits aria-label for span or more generally for role-less elements.

FWIW, I generally agree with disallowing aria-label on span, but I'm not sure if it's a strict ARIA conformance violation.

Copy link
Contributor

@straker straker Jun 28, 2021

Choose a reason for hiding this comment

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

So there is no spec documented reason for this (although it's coming). You can see where we got most of the concept in this pr #2764. As it stands though, it's also a mix of a best practice since no screen reader supports it (#2712 (comment)).

Edit: looks like it's been updated since I last looked. The generic role now prevents aria-label/ledby.

lib/checks/aria/aria-prohibited-attr-evaluate.js Outdated Show resolved Hide resolved
test/checks/aria/aria-prohibited-attr.js Outdated Show resolved Hide resolved
test/checks/aria/aria-prohibited-attr.js Outdated Show resolved Hide resolved
lib/checks/aria/aria-prohibited-attr-evaluate.js Outdated Show resolved Hide resolved
lib/checks/aria/aria-prohibited-attr.json Outdated Show resolved Hide resolved
@WilcoFiers WilcoFiers force-pushed the aria-prohibit-whitelist branch 2 times, most recently from 07aab35 to 332d338 Compare May 18, 2021 08:38
doc/check-options.md Outdated Show resolved Hide resolved
@WilcoFiers WilcoFiers merged commit d2584ed into develop May 18, 2021
@WilcoFiers WilcoFiers deleted the aria-prohibit-whitelist branch May 18, 2021 16:17
straker pushed a commit that referenced this pull request May 18, 2021
* fix(aria-allowed-attr): pass aria-label on some HTML elements

Allow aria-label and aria-labelledby on the following elements, if those elements have no role.
- audio
- applet
- canvas
- dl
- embed
- iframe
- input
- label
- meter
- object
- span
- svg
- video

* chore: Deal with IE

* chore: Address feedback

* chore: Maybe like this...

* chore: Random guessing stuff

* chore: Update doc/check-options.md
@andrewleith
Copy link

I'm seeing the same issue on an <i> element. Is this a bug as well or is it not allowed to put the an aria-label attribute on this tag?

Markup that is failing:

<i tabindex="0" class="remove icon" aria-label="clear field"></i>

Message from Axe:

Elements must only use allowed ARIA attributes

@straker
Copy link
Contributor

straker commented Jun 1, 2021

@andrewleith <i> elements do not allow aria-label. Using <i> in this way is one of the common examples of why this rule was created since screen readers do not announce the aria-label on the element.

@andrewleith
Copy link

Interesting. Thanks for the info @straker :)
Are these rules written formally somewhere? It appears the markup I provided passes the NU validator but warns on the W3c one...

@straker
Copy link
Contributor

straker commented Jun 1, 2021

I'm not sure what you mean by formally, but Leonie Watson wrote an excellent article on this behavior, which was the basis of this rule.

@melaniephilipp
Copy link

melaniephilipp commented Jun 14, 2021

@straker @WilcoFiers Someone on the A11y Slack (external) channel today mentioned getting an error for aria-labelledby on a <video> element. Has this fix been pushed out to axe-core / axe browser extension yet?

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.

None yet

5 participants