Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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
I think the solution here is to add the same code from template_preprocess_fieldset() to template_preprocess_details(). Then in all details.html.twig, we include something like:
{%- if description.content -%}
<div{{ description.attributes.addClass('description-details') }}>{{ description.content }}</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.
| Comment | File | Size | Author |
|---|---|---|---|
| #37 | 2896169-nr-bot.txt | 150 bytes | needs-review-queue-bot |
| #25 | accessibilty-issue-resolved-after-patch.png | 86.96 KB | sahal_va |
| #25 | accessibility-test-before-patch.png | 87.13 KB | sahal_va |
| #25 | patching.png | 60.14 KB | sahal_va |
| #25 | details-after-patch.jpg | 81.21 KB | sahal_va |
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:
changes
Comments
Comment #2
mstef CreditAttribution: mstef commentedComment #3
mstef CreditAttribution: mstef commentedComment #4
mstef CreditAttribution: mstef commentedComment #8
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedTagging this, will look at it soon. I only noticed this issue because the Webform accessibility handbook page links here.
Comment #11
sahal_va CreditAttribution: sahal_va as a volunteer commentedCreated the patch for above issue.
Comment #12
sahal_va CreditAttribution: sahal_va as a volunteer 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 CreditAttribution: sahal_va as a volunteer commentedCorrected patch.
Comment #16
sahal_va CreditAttribution: sahal_va as a volunteer commentedComment #17
sahal_va CreditAttribution: sahal_va as a volunteer and at Zyxware Technologies commentedComment #19
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedFixing failed tests!
Comment #21
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedFixing failed test!
Comment #22
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedCorrecting the above error!
Comment #24
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association 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 CreditAttribution: sahal_va as a volunteer and at Zyxware Technologies 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 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedHi @sahal_va
Please see the interdiff attached in #19 to see what fixed those tests!
Comment #29
mohit_aghera CreditAttribution: mohit_aghera at QED42 commentedChanging to needs review as patch is already passing all the test cases.
Comment #34
hswong3i CreditAttribution: hswong3i at PantaRei Design Limited (Hong Kong) commentedComment #37
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer 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