Problem/Motivation
Like all form elements, details elements are assigned an aria-describedby attribute with a value of {element-id}--description. The problem is that the actual description is not given this ID. It is properly assigned for other form elements (at least all that I've tested).
This attribute is set in FormBuilder.php on line 971 (as of 8.3.4):
if (!empty($element['#description'])) {
$element['#attributes']['aria-describedby'] = $element['#id'] . '--description';
}
The default template for a details element is in the system module's details.html.twig:
{{ description }}
The main theme template is in classy's details.html.twig:
<div class="details-description">{{ description }}</div>
No where is this proper ID being set.
The description attributes seem to be provided in form.inc within template_preprocess_form_element() and template_preprocess_fieldset(), but not within template_preprocess_details().
Tested with Bartik, Classy, and Seven.
Proposed resolution
Update all templates like this example:
{%- if description -%}
{% set description_attributes = create_attribute({id: attributes['aria-describedby']}) %}
<div{{ description_attributes.addClass('description-details') }}>{{ description }}</div>
{%- endif -%}
Remaining tasks
Provide a patch.
User interface changes
aria-describedby attributes will all be correct for details elements by properly wrapping the description in a DIV with the correct ID.
Before:

After:

| Comment | File | Size | Author |
|---|---|---|---|
| #57 | details-ariadescribedby-test.png | 437.43 KB | carlygerard |
| #48 | details_after.png | 29.84 KB | dcam |
| #48 | details_before.png | 28.31 KB | dcam |
Issue fork drupal-2896169
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:
Comments
Comment #2
mstef commentedComment #3
mstef commentedComment #4
mstef commentedComment #8
andrewmacpherson commentedTagging this, will look at it soon. I only noticed this issue because the Webform accessibility handbook page links here.
Comment #11
sahal_va commentedCreated the patch for above issue.
Comment #12
sahal_va commentedComment #14
joseph.olstadI had a look at the patch quickly, looks good so far, looks like remaining work is on refactoring the simpletests.
Comment #15
sahal_va commentedCorrected patch.
Comment #16
sahal_va commentedComment #17
sahal_va commentedComment #19
ridhimaabrol24 commentedFixing failed tests!
Comment #21
ridhimaabrol24 commentedFixing failed test!
Comment #22
ridhimaabrol24 commentedCorrecting the above error!
Comment #24
ridhimaabrol24 commentedAdded interdiff.patch by mistake instead of interdiff.txt. Hence its failing. However the patch in #22 passes the test bot. Hence moving this to 'Needs Review'.
Comment #25
sahal_va commentedI tested the issue with Bartik, Claro, Classy, and Seven. This seems to be fixed.
Attaching the screenshots below for Seven.
Hi @ridhimaabrol24
Though changes in both patches are same, why #17 fails? I think I'm missing something essential for auto testing??
Comment #26
joseph.olstad+1 , great work! I checked the drupal bootstrap contrib theme (version 3x) and it doesn't override this twig so this fix should also fix the for bootstrap installs as well and likely others.
Get it in!
Comment #27
ridhimaabrol24 commentedHi @sahal_va
Please see the interdiff attached in #19 to see what fixed those tests!
Comment #29
mohit_aghera commentedChanging to needs review as patch is already passing all the test cases.
Comment #34
hswong3i commentedComment #37
needs-review-queue-bot commentedThe 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.
Comment #38
mgiffordTagging for https://www.w3.org/WAI/WCAG21/Understanding/name-role-value
Comment #44
dcam commentedI 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.
Comment #46
smustgrave commentedHiding 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.
Comment #47
mgiffordThis looks good to me.
I missed this issue @smustgrave - sorry about the delay.
Comment #48
dcam commentedAdded screenshots.
Comment #49
dcam commentedA change record has been added at https://www.drupal.org/node/3509534.
Comment #50
smustgrave commentedSince this is changing the default twig it should be noted in the CR or a new CR be created
Comment #51
dcam commented@smustgrave I did note it with this line in the CR:
Do I need to add more, edit the line, or note it in a different way?
Comment #52
dcam commentedPer @nod_ in Slack:
I'm working on it.
Comment #53
dcam commentedAlthough @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.
Comment #54
nod_We should be able to do that from twig only, no php changes needed to
form.incComment #55
nod_umm but then we get duplication, I'm asking for other opinions
Comment #56
dcam commentedIt 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.
Comment #57
carlygerardI 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.
Comment #58
smustgrave commentedFeedback appears to be addressed on this one.
Comment #60
catchCommitted/pushed to 11.x, thanks!