Details elements have incorrect aria-describedby attributes

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Tagging for https://www.w3.org/WAI/WCAG21/Understanding/name-role-value

dcam → changed the visibility of the branch 11.x to hidden.

dcam → changed the visibility of the branch 2896169-details-elements-have to hidden.

I rerolled the former 9.x MR against 11.x. Changes for old themes were removed. Changes for new themes were added. I also added a basic test for the description wrapper markup.

smustgrave → closed merge request !1887

Hiding patches and old MRs.

This will need a CR because of the template change.

User Inteface section is meant for before/after screenshots so those should be added too please

@mgifford is this the correct approach btw? Could we removed "Needs accessibility review" tag

Moving to NW for other items.

This looks good to me.

I missed this issue @smustgrave - sorry about the delay.

Added screenshots.

A change record has been added at https://www.drupal.org/node/3509534 → .

Since this is changing the default twig it should be noted in the CR or a new CR be created

@smustgrave I did note it with this line in the CR:

In the default core/modules/system/templates/details.html.twig template a

wrapper was added around the description with the new description.attributes.

Do I need to add more, edit the line, or note it in a different way?

Per @nod_ in Slack:

can we add a wrapping div with only the corresponding id for aria? we already have that info in the template that would avoid changing the structure of description... I would much rather do that. We already have an extra div for errors so that wouldn't be strange to have one for description

I'm working on it.

Although @nod_ said "can we add a wrapping div with only the corresponding id for aria? we already have that info in the template..." I couldn't figure out how to get individual attributes out of an existing Attribute object, at least not cleanly. Given that, I figured that this was probably best done in the preprocess function and assigned to a new variable. If I've missed something, then let me know.

I updated the IS and CR with information about this change.

We should be able to do that from twig only, no php changes needed to form.inc

umm but then we get duplication, I think it's fine, details is not really a heavily customized template

umm but then we get duplication, I'm asking for other opinions

It looks like you were right to me. I don't see any duplication. I knew there had to be a way. Thank you for showing me.

I updated the IS and CR again.

I tested the up to date branch on an 11.1.x install, this works as expected for me--the aria-describedby value matches the ID of the description text.

Feedback appears to be addressed on this one.