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

False positive: 'aria-valid-attr' fails for using 'aria-roledescription' on role button #1216

Closed
iamrafan opened this issue Nov 1, 2018 · 8 comments

Comments

@iamrafan
Copy link
Contributor

iamrafan commented Nov 1, 2018

aria-valid-attr fails for aria-roledescription as it was not supported across most of the Browser/ATs

image

Latest AT testing shows that now there is much broader support for the usage of aria-roledescription

  AT/Browser Announced as Is aria-roledescription announced?
1 Edge/Narrator Here Split button Yes
2 Edge/JAWS Here Button No
3 Edge/NVDA Split button here Yes
4 Chrome/NVDA Split button here Yes
5 Chrome/JAWS Here Split button Yes
6 IE/JAWS Here split button Yes
7 IE/NVDA Split button here Yes
8 Firefox/NVDA Split button here Yes
axe-core version: 3.1.2
axe-Coconut version: 3.5.0
@WilcoFiers
Copy link
Contributor

Totally blown away that this works on JAWS+IE... if that's right, than yes this passes the Axe-core baseline and we should allow it. That'd be fantastic. @marcysutton @mfairchild365 any thoughts?

@mfairchild365
Copy link
Contributor

@WilcoFiers Yep, it works in JAWS+IE. I tested with JAWS 2018.1808.10 and IE 11.

Some other thoughts:

Thought 1: I'm curious how well it works with other elements and ARIA roles. My guess is that if it works fine with the <button> element, that it would work fine with other elements too, but that is a big assumption.

Thought 2: The ARIA 1.2 spec says "Assistive technologies SHOULD use the value of aria-roledescription when presenting the role of an element, but SHOULD NOT change other functionality based on the role of an element that has a value for aria-roledescription."

The fact that it says "SHOULD" instead of "MUST" would suggest to me that this must pass the "accessibility supported" requirement even if assistive technologies don't implement it.

@attilavago
Copy link

This needs to get fixed ASAP. Creates confusion everywhere...

@mfairchild365
Copy link
Contributor

A further note. I did some more research and my findings can be found here: https://a11ysupport.io/tech/aria/aria-roledescription_attribute (feel free to contribute any different findings)

In summary: the attribute is well supported for interactive elements like <button> but not so much for grouping/sectioning elements like <section>. I tested both situations because of this text in the ARIA spec:

Authors SHOULD limit use of aria-roledescription to clarifying the purpose of non-interactive container roles like group or region, or to providing a more specific description of a widget.

As it pertains to this test in axe: I think it is still worth weighing the fact that the ARIA spec does not require that assistive technologies announce the custom role name.

@WilcoFiers
Copy link
Contributor

Talked about it with @marcysutton. We think to allow this for interactive elements only. Static elements should still be flagged as an issue because as the info on a11ysupport suggests, that isn't widely supported yet:

https://a11ysupport.io/tech/aria/aria-roledescription_attribute

@marcysutton marcysutton self-assigned this Dec 12, 2018
@marcysutton marcysutton assigned straker and unassigned marcysutton Jan 9, 2019
@straker
Copy link
Contributor

straker commented Jan 14, 2019

Hello everyone, I'll be working on this issue.

I wanted to expand @mfairchild365 tests and see what exactly the support was for aria-roledescription, so I tested it on every element with an implicit role as well as creating elements with the same WAI-ARIA role. Here's the full results: https://codepen.io/straker/full/Xoybem.

From the tests, there are only a handful of elements that are consistently supported across all browsers and screen readers:

  • <button>
  • <img>
  • <input type="button">
  • <input type="checkbox">
  • <input type="image">
  • <input type="radio">
  • <input type="reset">
  • <input type="submit">
  • <select>
  • <summary> (if the browser supports the summary element)

There's also consistent support for role="menuitemcheckbox" and role="menuitemradio".

After discussing further with @WilcoFiers we think we should only pass on these elements/roles. For all other elements with an implicit or valid WAI-ARIA role we think we should mark it as incomplete and author should review due to lack of support. For all other uses of aria-roledescription we think we should fail on any element that doesn't have an implicit role or valid WAI-ARIA role since without a role the aria-roledescription would not work as per the spec.

When using aria-roledescription, authors SHOULD also ensure that:

  1. The element to which aria-roledescription is applied has a valid WAI-ARIA role or has an implicit WAI-ARIA role semantic.

Proposed rule changes for aria-roledescription

  • aria-allowed-attr
    • would pass for elements with an implicit or WAI-ARIA role of:
      • button (<button>, <input type="button|image|reset|submit">, <summary>)
      • img (<img>)
      • checkbox (<input type="checkbox">)
      • radio (<input type="radio">)
      • combobox (<select>)
      • menuitemcheckbox
      • menuitemradio
    • would be marked incomplete for all other elements with an implicit or valid WAI-ARIA role
    • would fail otherwise
  • aria-valid-attr would always pass
  • aria-valid-attr-value would always pass

WilcoFiers pushed a commit that referenced this issue Feb 28, 2019
…1382)

Expand the `unsupported` property of an attribute by allowing certain elements to use the attribute.

Closes: #1216

## Reviewer checks

**Required fields, to be filled out by PR reviewer(s)**
- [x] Follows the commit message policy, appropriate for next version
- [x] Has documentation updated, a DU ticket, or requires no documentation change
- [x] Includes new tests, or was unnecessary
- [x] Code is reviewed for security by: @WilcoFiers
@kensgists
Copy link

ARIA 1.1 states aria-roledescription is allowed on "all elements of the base markup." Flagging a violation on usage of this attribute on specific roles creates false positives.

Consistent support "across all browsers and screen readers" is, at best, a nice to have. I think this might be classified as a "best practice" but is not appropriate as a violation.

It appears that the latest axe in production (3.2.2) flags aria-allowed-attr if you use aria-roledescription on any element outside of this hand-selected list of roles from the above comment.

When dealing with ARIA roles and attributes, I think it is best to be strict to the specification with things flagged as violations. If you feel it makes sense to limit allowed attributes beyond the specification, then camp those not with violations but as best practices.

@kensgists
Copy link

May also be worth pointing out that roledescription is supported at the API level on all desktop platforms (and likely iOS, if AX API is parallel -- unsure about Android):
https://www.w3.org/TR/core-aam-1.2/#details-id-156

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants