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.
The 'for' attribute for the label field associated with the File element (type: managed_file) points to the wrapping div container and not the file input field. The file input field has '-upload' added to the end of the id.
Attached is a patch that does that, but it doesn't feel like the correct way.
I would rather not have an if (element type is ... ) then ... situation.
Please Review.
This is similar to: #1147994: Wrap composite elements in a fieldset element.
Comment | File | Size | Author |
---|---|---|---|
#7 | webform-managed_file_label_accessibility-2077237-7.patch | 832 bytes | Liam Morland |
webform-7.x-4.x-dev-managed_file_label_accessibility-1.patch | 833 bytes | thekevinday | |
Comments
Comment #1
quicksketchSimilar issue over here: #945438: Prevent label @for from pointing at non-existent @id
You're right that files in particular have got a weird problem with their labels. However I don't think we should be putting a hack right in a theme function to solve the problem. If you have any other suggestions for approaches that are less hacky, I'm definitely looking for options.
Comment #2
thekevinday CreditAttribution: thekevinday commentedThe drupal core function takes the '#id' variable and uses it for the 'for' attribute for label fields.
The webform theme function passes the '#id' to the label, and then builds the child elements.
The child element theme functions will take the '#id' of the parent and alter it for themselves.
This causes the file input field to have a label with '-upload' appended to the 'id' attribute.
The accessibility problem happens because the label field points to just '#id' and the actual field it should point to is the '#id'-upload.
I could either see this as a core bug because they are taking the 'id' attribute and changing it to a 'for' attribute (why not just have a '#for' variable??).
Or I could see this as a theme/design bug in webform where the wrapping 'div' container should not have an id and the file input field should have an id without the '-upload' appended.
Comment #3
quicksketchProbably because most attributes affect the form element, not the label tag. I think it'd be confusing to have a form element that could have both an #id and #for attributes when the #id affects the element and #for affects the label.
Anyway, I think we should be able to fix this on the Webform side. This might go hand-in-hand with some of the work at #2059215: Replace use of IDs to classes on Webform wrappers to avoid duplicate IDs.
Comment #4
mgiffordSo what do we want to do? The patch still applies to the 4.x branch. Is it worth bringing it in?
Comment #5
Liam MorlandLooks good to me. Here is a reroll with minor refactor.
Comment #7
Liam MorlandComment #9
DanChadwick CreditAttribution: DanChadwick commentedThanks, folks. Committed to 7.x-4.x and 8.x.