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

aXe claims "severe" a11y error on conditional content blocks #455

Closed
michaeldfallen opened this issue Apr 26, 2017 · 16 comments
Closed

aXe claims "severe" a11y error on conditional content blocks #455

michaeldfallen opened this issue Apr 26, 2017 · 16 comments

Comments

@michaeldfallen
Copy link

Using the aXe a11y scanner on conditionally revealing content it reports:

Elements must only use allowed ARIA attributes
Issue Type WCAG 2.0 (A): MUST
User Impact Critical

Having tried through a screen reader and in another tool (pa11y) they don't report this as a problem.

You can replicate by installing either aXe for chrome or aXe for firefox and run it against the govuk-elements examples

Do you have any suggestions? Should we rework elements to not present this error? or propose the check be loosened in aXes ruleset?

@selfthinker
Copy link
Contributor

The exact error message includes:

ARIA attribute is not allowed: aria-expanded="false"

And the info page about this general error says:

Checks that each element with an ARIA role uses only ARIA attributes allowed for that role.

I don't know why it says that. I checked the spec and couldn't find this property would not be allowed with certain roles. Did I miss anything?

@michaeldfallen
Copy link
Author

The rule they are applying seems to be the other way. As in not "this property isn't allowed" but "only properties that are allowed should be used".

I'm not entirely certain if that makes sense, good chance aXe is wrong here.

@adamliptrot-oc
Copy link

Also got this when trying to add automated accessibility testing with aXe.

@accessiblewebuk
Copy link
Member

accessiblewebuk commented Jun 14, 2017

Asked the question of Deque on Twitter - @Detonite responded that the violation is because the spec doesn't allow it for input - cc @michaeldfallen @selfthinker

@jkva
Copy link

jkva commented Jun 14, 2017

Or particularly for role radio, which is the effective role that the input type="radio" (which the violation is raised for) would have.

@ZoeBijl
Copy link

ZoeBijl commented Jun 14, 2017

If you look at ARIA in HTML it states that for an input with a type of radio allowed roles and attributes are as follows:

Role:menuitemradio
global aria-* attributes and any aria-* attributes applicable to the menuitemradio role.

For menuitemradio (irrelevant in this case, but for completion’s sake) the spec states the following attributes are allowed:

  • aria-posinset
  • aria-selected (state)
  • aria-setsize

So, aria-expanded is indeed invalid ARIA in this case. The resulting code isn’t necessarily inaccessible, especially given that the input requesting more information (be it e-mail or phone number) is the next one up in the tab order.

I think the invalid ARIA doesn’t add anything in this case and can be removed.

@gemmaleigh
Copy link
Contributor

gemmaleigh commented Jun 14, 2017

Thanks everyone,

GOVUK. ShowHideContent is responsible for setting these ARIA attributes.

I'll raise an issue over on the govuk_frontend_toolkit repo.

@selfthinker
Copy link
Contributor

I would challenge a) our user interaction of ticking a checkbox or radio box doing something (I presume there was lots of user research but just don't know any of that) and b) I would also challenge the spec.
aria-expanded can be used either to say if the element itself was expanded or not. In that case the spec makes absolute sense to not allow that attribute. But it can also be used together with aria-controls, in which case it refers to the element that is linked to via that (which is what we use for the conditional content). In that case I would challenge the spec and say it makes sense to be valid.

@jkva
Copy link

jkva commented Jun 14, 2017

@selfthinker In case of checkbox I could see it be valid, since control can be indicated via aria-controls and one cannot infer if checking collapses or expands, and vice-versa.

For a radio control I don't see the point - since only one can be selected at any time, one cannot infer if deselecting collapses or expands a controlled element.

In any case I'm not a fan of this type of content disclosure - especially in the case of radio controls, we cannot guarantee that the element being controlled is next in the tab order.

@fofr
Copy link
Contributor

fofr commented Jun 14, 2017

Having tried through a screen reader and in another tool (pa11y) they don't report this as a problem.

I think the invalid ARIA doesn’t add anything in this case and can be removed.

Has this been found to cause problems for screen readers (rather than validators)?

@selfthinker
Copy link
Contributor

@jkva, I can see it making sense for radio buttons as well. When you select one, it opens something, then you select another and it closes the previous one and opens the next. Just like in our example on Elements. But it's probably less clear that way for screen readers.

@fofr, I don't think anyone has reported that but it would be good to double check with a couple of screen readers. We have recently tested everything within Elements for accessibility issues and I don't think we found any issues with this interaction.
I assume it will be fine, which would be a reason to not necessarily fix it but rather report this "bug" to the W3C. I plan to do that.

But one way to fix it would be to move the aria-expanded to the div that actually gets expanded (and then also remove the aria-controls).

@LJWatson
Copy link

LJWatson commented Jun 22, 2017

The definition for aria-expanded states that the attribute can be used on elements with the following roles:

  • button
  • combobox
  • document
  • link
  • section
  • sectionhead
  • window

This means that aria-expanded can only be applied to the <button>, <select>, <body>, <a href>, or <section> elements, or elements with role="n" (where n is one of the roles listed above).

Nonetheless, some browsers expose the attribute and consequently some screen readers pick up that information and convey it to the user.

This said, making a radio button the control for a disclosure widget is not a good design pattern from a UX point of view IMO.

@joelanman
Copy link
Contributor

@LJWatson can you expand on why you don't think it's good to use a radio button for progressive disclosure? The progressive disclosure pattern is to only show relevant info or questions to users when we know it's relevant - based on their answer to another question. That question could take any form - checkbox, radio, etc, no? An example of using a radio for progressive disclosure is when we ask someone what country they live in on Register to vote. If they select 'outside the uk', we then show the question 'Where did you live before you moved to another country?'.

@LJWatson
Copy link

It creates an interaction that will be unfamiliar and confusing to many. A set of radio buttons offers multiple choice, whereas a disclosure trigger offers a binary choice. extending one of the multiple choices to also be a binary choice overloads the interaction.

There is also the problem that the aria-expanded attribute is not permitted on a radio button, and consequently it isn't widely supported by screen readers - adding further confusion to an already confusing pattern.

@joelanman
Copy link
Contributor

We have tested this pattern many times in Register to vote and other services, and as far as I'm aware we've never seen an issue with it, in fact it's something we've seen test well - are you aware of any research to show it's confusing?

@NickColley
Copy link
Contributor

Following the launch of the GOV.UK Design System, GOV.UK Elements will now only get major bug fixes and security patches.

A similar issue has been noted for the GOV.UK Design System version of this component, so we will investigate the best way forwards there: alphagov/govuk-frontend#979

Thanks a lot for the input, I'm sure this thread will help us resolving this. 👍

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

No branches or pull requests