Problem/Motivation

According to testing done at the Helsinki University current details element has excess aria attributes that add to screen reader output without providing useful information.
<summary role="button" aria-controls="edit-group-test" aria-expanded="false" aria-pressed="false" class="claro-details__summary">
Aria-attribute "aria-pressed" is perceived unnecessary and confusing. It was not found to be needed for legacy compatibility either in a quick search.

One blog seemed to have credible details on details and suggests that the code could be simplified further - but some of the currently maybe not required code may help legacy software users.
https://www.scottohara.me/blog/2018/09/03/details-and-summary.html
https://www.scottohara.me/blog/2022/09/12/details-summary.html

Steps to reproduce

Create a form that includes a details element.

For testing I used a stable Drupal 10.1.x and contrib module field_group - but the code for aria-pressed comes from core and is present in 11.x-dev.

Proposed resolution

Remove attribute "aria-pressed" from default details element.

User interface changes

Screenreader output is cleaner, there should not be other changes.

Issue fork drupal-3403198

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

simohell created an issue. See original summary.

simohell opened merge request !5497

simohell’s picture

simohell commented
Status: Active » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Reran tests and they failed still.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Issue tags: +aria

Looking at some recent examples, including Scott's above, I'm not sure that we need any aria for the detail/summary. There was an outstanding bug for NVDA, but I think that's been resolved now.

Some follow-up links:

Wouldn't it be nice if we could just get it down to very basic HTML - <summary class="claro-details__summary">

simohell’s picture

simohell commented
Status: Needs work » Needs review

Dropped testing for the attribute that got removed.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Taking #5 as accessibility sign off then :)

itmaybejj’s picture

itmaybejj commented

Oh splendid. I had an open ticket to remove the aria-pressed attribute from our themes, as it confuses VoiceOver into announcing the summary/details as if it is a form element rather than a fieldset.

This will fix that nicely.

quietone’s picture

quietone commented

I'm triaging RTBC issues. I read the IS, the comments and the MR. I didn't find any unanswered questions or other work to do.

There is support for this change from an Accessibility topic maintainer, #5.

Leaving at RTBC.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: 11.x-dev » 10.2.x-dev
Status: Reviewed & tested by the community » Fixed

Agree that from reading the docs aria-pressed indicates the pressed state of a toggle button, and while <details> could be considered a toggle button, the value of aria-expanded would seem to have more relevance. As these are always the same value then removing aria-pressed makes sense.

Backporting to 10.2.x as an accessibility fix.

Committed and pushed 91a743c141 to 11.x and 5d5a43b86f to 10.2.x. Thanks!

  • longwave committed 5d5a43b8 on 10.2.x 
    Issue #3403198 by simohell, mgifford, itmaybejj: Details do not benefit...

longwave closed merge request !5497

  • longwave committed 91a743c1 on 11.x 
    Issue #3403198 by simohell, mgifford, itmaybejj: Details do not benefit...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.