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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

Status: Needs review » Needs work

Similar 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.

thekevinday’s picture

The 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.

quicksketch’s picture

Status: Needs work » Active

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??).

Probably 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.

mgifford’s picture

Issue summary: View changes

So what do we want to do? The patch still applies to the 4.x branch. Is it worth bringing it in?

Liam Morland’s picture

Status: Active » Needs review
FileSize
804 bytes

Looks good to me. Here is a reroll with minor refactor.

Status: Needs review » Needs work

The last submitted patch, 5: webform-managed_file_label_accessibility-2077237-5.patch, failed testing.

Liam Morland’s picture

Status: Needs work » Needs review
FileSize
832 bytes

  • DanChadwick committed 87ef4cf on 7.x-4.x
    Issue #2077237 by Liam Morland, thekevinday: Fixed Accessibility Issue...
  • DanChadwick committed 2ef4927 on 8.x-4.x
    Issue #2077237 by Liam Morland, thekevinday: Fixed Accessibility Issue...
DanChadwick’s picture

Status: Needs review » Fixed

Thanks, folks. Committed to 7.x-4.x and 8.x.

Status: Fixed » Closed (fixed)

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