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

Accordion - Elements must only use allowed ARIA attributes #2472

Closed
adamliversidge opened this issue Dec 13, 2021 · 22 comments · Fixed by #4628
Closed

Accordion - Elements must only use allowed ARIA attributes #2472

adamliversidge opened this issue Dec 13, 2021 · 22 comments · Fixed by #4628
Labels
accessibility concern Bug, feature request or question about the accessibility of a portion of a product (not a WCAG fail) accessibility statement These GitHub issues are or have previously been featured in our accessibility statement accessibility accordion 🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Milestone

Comments

@adamliversidge
Copy link

Description of the issue

Axe DevTools reports "ARIA attribute: aria-labelledby is not well supported. Use a different role attribute or element."

Element location
#accordion-default-content-1

Need to ensure ARIA attributes are allowed for an element's role

Steps to reproduce the issue

Run the accessibility checker with an expanded accordion

Actual vs expected behaviour

Error should not be reported for the expanded accordion

Environment (where applicable)

GOV.UK Frontend v3.14.0
https://design-system.service.gov.uk/components/accordion/default/index.html

@adamliversidge adamliversidge added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels Dec 13, 2021
@vanitabarrett vanitabarrett added accordion 🕔 days and removed awaiting triage Needs triaging by team labels Dec 14, 2021
@spSlaine
Copy link

Suggestion - add role="region" to the div.govuk-accordion__section-content

<div class="govuk-accordion" data-module="govuk-accordion" id="accordion-default">
  <div class="govuk-accordion__section ">
    <div class="govuk-accordion__section-header">
      <h2 class="govuk-accordion__section-heading">
        <span class="govuk-accordion__section-button" id="accordion-default-heading-1">
          Writing well for the web
        </span>
      </h2>
    </div>
    <div id="accordion-default-content-1" class="govuk-accordion__section-content" aria-labelledby="accordion-default-heading-1" role="region">
      <p class='govuk-body'>This is the content for Writing well for the web.</p>
    </div>
  </div>
</div>

@36degrees
Copy link
Contributor

Adding aria-labelledby to a <div> with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification.

Unless we have good evidence that it's useful to make the accordion sections landmark regions (in which case we should probably make them into <section>s), we should just remove the aria-labelledby attribute entirely.

(Relevant discussion on the use of <section>s in a GDS Way PR: alphagov/gds-way#781)

@jpveooys
Copy link

This also occurs in axe in Chrome with a collapsed accordion now that hidden="until-found" is used, for info.

@dav-idc dav-idc added accessibility regulations failure Does not meet the Public Sector Accessibility Regulations (WCAG 2.2 level AA) accessibility concern Bug, feature request or question about the accessibility of a portion of a product (not a WCAG fail) accessibility statement These GitHub issues are or have previously been featured in our accessibility statement labels Apr 11, 2023
@dav-idc
Copy link

dav-idc commented Apr 11, 2023

Hi @36degrees, I've taken a look at this issue and if everything mentioned in this Github issue is accurate, I think this is a WCAG failure and something we should fix.

This is my theory as for how it's currently failing:

This is an incorrect implementation of WAI-ARIA 1.1 and a failure of WCAG success criterion 4.1.2 Name, Role, Value.

For anyone watching this thread, let me know if you think I've misinterpreted the situation 👍

@dav-idc
Copy link

dav-idc commented Apr 11, 2023

If this is a WCAG failure, I'd like to add this to our accessibility statement as a known piece of 'non-accessible content', which is the name of the section where we record accessibility issues which constitute failures of WCAG 2.1 AA. It would be noted as a failure in the GOV.UK Frontend codebase and not one of our website products.

This is my drafted note:

The accordion component is currently using an aria-labelledby ARIA attribute on an unsupported <div> element. This is an incorrect implementation of WAI-ARIA 1.1 and a failure of WCAG success criterion 4.1.2 Name, Role, Value. We plan to fix this within 2023. Track our progress on the GitHub issue: Accordion - Elements must only use allowed ARIA attributes.

@36degrees
Copy link
Contributor

Which aspect(s) of 4.1.2 do you think it fails on?

@dav-idc
Copy link

dav-idc commented Apr 13, 2023

I didn't personally assess it as a failure of 4.1.2, the failure under 4.1.2 is reported by axe DevTools. Here's their explainer page for the rule it says we're breaking: https://dequeuniversity.com/rules/axe/4.6/aria-allowed-attr?application=AxeChrome

If we doubt that I could do a more thorough double-check.

@36degrees
Copy link
Contributor

It's going to catch quite a range of issues, some of which by its own description have 'no effect on the accessibility'. Definitely some of the issues it will catch might mean you fail 4.1.2, but I don't think there's a direct correlation.

Looked at another way, unless I'm missing something, I think the success criterion still holds 'true' despite the incorrect use of aria-labelledby?

For all user interface components (including but not limited to: form elements, links and components generated by scripts), the name and role can be programmatically determined; states, properties, and values that can be set by the user can be programmatically set; and notification of changes to these items is available to user agents, including assistive technologies

I still agree with fixing it, just not sure it needs to go in the statement unless we can articulate why it's failing.

@dav-idc
Copy link

dav-idc commented Apr 13, 2023

Hey @36degrees I agree, I wouldn't personally assess it as a failure of 4.1.2 based on our existing testing and evidence.

If we found a combination of assistive tech, browser and OS that ended up doing something funky or buggy because of the aria-labelledby, then it would be easier to call a fail. But We don't have evidence of that being the case. Right now it's just improper use of WAI-ARIA.

My recommendation: We add this to the accessibility statement under the "(c)" category that stresses that it's not a WCAG failure. That way we can test out our use of the accessibility strategy and correctly state that it's not a WCAG fail. I'll also note on the item that it is not a WCAG fail.

Here are those 3 options that are available in the accessibility statement for categorising issues:

  • (a) non-compliance with the accessibility regulations
  • (b) disproportionate burden
  • (c) the content is not within the scope of the accessibility regulations

How does that sound @36degrees? I know it's still not the same as your recommendation, which is to not add it to the accessibility statement.

@36degrees
Copy link
Contributor

Sounds good to me 👍🏻

@eschweitzer78
Copy link

Hello @davidc-gds, I was about to report this issue when I noticed it's open. FYI I found the reference text that states one must not use aria-labelledby in a div unless a role is set. W3 used capital letters as in MUST NOT in the paragraph this links to: https://www.w3.org/TR/html-aria/#example-elements-with-implicit-aria-roles-which-prohibit-naming-from-authors

@stephbatliner
Copy link

I was wondering if there are there any timelines around fixing this bug? Thank you

@dav-idc
Copy link

dav-idc commented Jul 20, 2023

Hello @davidc-gds, I was about to report this issue when I noticed it's open. FYI I found the reference text that states one must not use aria-labelledby in a div unless a role is set. W3 used capital letters as in MUST NOT in the paragraph this links to: https://www.w3.org/TR/html-aria/#example-elements-with-implicit-aria-roles-which-prohibit-naming-from-authors

Hey @eschweitzer78 thanks for locating the text in the 'ARIA in HTML' specification!

At this point, we've added this to our accessibility statement under the category 'Other identified and tracked accessibility concerns'. We still don't have evidence of this being a true WCAG 2.1 AA fail, but it is definitely an incorrect implementation according to the 'WAI-ARIA 1.1' specification. We can now also count it as conflicting with the 'ARIA in HTML' specification, as you've identified.

@dav-idc
Copy link

dav-idc commented Oct 17, 2023

@colinrotherham here's a message from @36degrees in a duplicate GitHub issue that came up a couple weeks ago:

We're busy working on the next major release at the minute but I'm hoping we'll be able to resolve this issue soon. As far as I know although it's flagged by Axe we're not aware of any problems it would cause users, but it has come up quite a lot so it'd be good to get it fixed anyway!

We list this in our accessibility statement as a non-WCAG-failing accessibility concern and say "We plan to fix this within 2023." But as far as when it's fixed and how it interacts with release planning, that's up to devs (you, Ollie, others)

@colinrotherham
Copy link
Contributor

Thanks @dav-idc

I'm happy to add a known exception like we do with the Radios Axe rule for #979

For when this gets picked up would you suggest either we:

  1. Move to <section> elements?
  2. Add the role="region" attribute?
  3. Remove aria-labelledby attribute?

@dav-idc
Copy link

dav-idc commented Oct 17, 2023

I would want @36degrees to weigh in if he has capacity, but I also figured I may as well just try and see what value we're currently getting out of the aria-labelledby attribute.

As far as I can tell when testing quickly with NVDA and JAWS, the aria-labelledby attribute on the hidden content div area doesn't change NVDA or JAWS annoucements for:

  • navigating sequentially into and out of the content area using up and down areas
  • navigating between focusable elements inside and outside the content area using tab and shift-tab

So basically... I don't know what adding aria-labelledby is actually doing for us in practical terms. 🤷
But I'd want someone else to test and confirm.

@colinrotherham
Copy link
Contributor

We did go ahead with the Axe 4.8 update for Puppeteer last week

The violation rule changed to aria-allowed-attr aria-prohibited-attr but still excluded as a known issue

@dav-idc
Copy link

dav-idc commented Jan 5, 2024

In case this issue is currently stuck because:

  1. don't want to use <section>
  2. don't want to remove aria-labelledby

You could explore using <article> instead of <div>.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/article

Based on the HTML spec for the article element, it seems like an acceptable use in many accordion cases, but probably not fully appropriate for all.

Might still be a decent calculated risk if there's hesitation to use section and hesitation to remove aria-labelledby? Not sure if that's the piece holding this up though.

36degrees added a commit that referenced this issue Jan 12, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [3]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
36degrees added a commit that referenced this issue Jan 12, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [4]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
36degrees added a commit that referenced this issue Jan 12, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [4]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
36degrees added a commit that referenced this issue Jan 12, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [4]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
36degrees added a commit that referenced this issue Jan 16, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [4]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
@36degrees 36degrees moved this from Backlog 🗄 to Ready to release 🚀 in GOV.UK Design System cycle board Jan 16, 2024
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 22, 2024
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 23, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 24, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 24, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 25, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 25, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
WolfyUK added a commit to DFE-Digital/education-benchmarking-and-insights that referenced this issue Jan 25, 2024
- Updated build pipelines to publish test results
- Fixed failing `InsightSchoolSteps` test
- Resolved accessibility issue raised at alphagov/govuk-frontend#2472
- Fixed bad project reference in `EducationBenchmarking.Platform.Search.App.csproj`
@36degrees 36degrees added this to the v5.1 milestone Jan 26, 2024
owenatgov pushed a commit that referenced this issue Jan 26, 2024
Adding `aria-labelledby` to a `<div>` with no explicit role is explicitly disallowed by the 'ARIA in HTML' specification [1] and in our testing provides little or no benefit to users, including users of assistive technologies [2].

We’ve explored using `<section>` elements for each accordion section (for which a label would make sense), however best practise is to limit the use of region landmarks ‘as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page’ [3].

We don’t have clear evidence that marking accordion sections up as regions is useful, so more research is needed in this area.

There has also been a suggestion that we could use `<article>` elements, but it would depend on how the accordion is being used. [4]

We may still want to pursue these options in the future, but until we make a decision on whether the sections should have a semantic role remove the `aria-labelledby` attribute – it’s trivial to reintroduce it later.

[1]: https://www.w3.org/TR/html-aria/#docconformance-naming
[2]: #2472 (comment)
[3]: https://www.scottohara.me/blog/2018/03/03/landmarks.html#region
[4]: #2472 (comment)
@36degrees 36degrees moved this from Ready to release 🚀 to Done 🏁 in GOV.UK Design System cycle board Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility concern Bug, feature request or question about the accessibility of a portion of a product (not a WCAG fail) accessibility statement These GitHub issues are or have previously been featured in our accessibility statement accessibility accordion 🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Development

Successfully merging a pull request may close this issue.