Problem/Motivation
Improve accessibility for screen readers by using aria-description attributes.
Proposed resolution
Add the aria-describedby attribute to associate the form control with its description.
Remaining tasks
Ready to be committed.
User interface changes
None.
API changes
None.
Original report by mgifford
Following on this post here - http://groups.drupal.org/node/4859
I created a patch against D7 to bring this behavior into core. Sure, one could use theme override to do it right, but why not just do that in core.
People want to see the description of what they are going to do before they are going to do it. As I was talking to folks at DrupalCon about this as well, screen readers need the description text before the form that they are going to fill out.
It's a simple fix (and I've made some wee small enhancements to that function as well).
(for legacy issues whose initial post was not the issue summary)
Comment | File | Size | Author |
---|---|---|---|
#60 | aria-describedby.patch | 1.19 KB | Liam Morland |
#51 | aria-describedby.patch | 1.17 KB | Liam Morland |
#46 | aria-describedby_1.patch | 1.17 KB | Everett Zufelt |
#40 | aria-describedby.patch | 1.14 KB | Liam Morland |
#22 | aria-describedby.patch | 1.18 KB | Liam Morland |
Comments
Comment #2
mgiffordComment #3
mgiffordHoping previous post's patch gets added to the queue for testing.
Comment #5
mgiffordAny suggestions from anyone on how to track down the single failure out of the 10K tests that were done on this patch:
Test single fields 136 1 0
I looked into modules/profile/profile.test but really suspect this isn't an issue with the patch, but the assumptions in the test.
Just not sure how to resolve it.
Mike
Comment #6
Everett Zufelt CreditAttribution: Everett Zufelt commentedI'm not sure that this is necessarily an accessibility advantage. One the one hand it would be nice for this text to be available to a screen-reader user in a apparent way. However, often times screen-reader users tab from form field to form field and would only have this text read if it was marked up as a label for the form field.
Setting to postponed until there is some user testing of screen-reader users / all users, to see if the proposed modifications will really mean accessibility / usability improvements.
Comment #7
kat3_drx CreditAttribution: kat3_drx commentedI can introduce this issue into some usability testing with screen readers and blind users or limited vision users. Starting a new round testing in the near future. Can test with this patch applied, and D7 without to get data.
Comment #8
mgiffordto add a bit more clarity to this, on this page:
admin/structure/block/manage/search/form/configure
The suggestion was largely to switch the description so that it wouldn't follow the form. Right now the HTML looks like:
and I think that this is ultimately what we want it to look like:
User testing & feedback from screen reader users is critical!
Comment #9
Everett Zufelt CreditAttribution: Everett Zufelt commentedWe won't be making this markup change in D7. I'm not sure if rearranging the order of label - input - description is desirable, but we can use aria-describedby to link input and description inD8.
Comment #10
Everett Zufelt CreditAttribution: Everett Zufelt commentedComment #11
bowersox CreditAttribution: bowersox commentedsub
Comment #12
Everett Zufelt CreditAttribution: Everett Zufelt commentedMarked #935982: Programmatically associate form element and description with aria-describedby as a duplicate.
Description
Currently the relationship between a form input element and the form element description is not programmatically associated. Using the aria-describedby
property we can programmatically associate form fields with their descriptions. This means that screen-readers who tab through a form will have access
to the form element description without exiting "forms mode".
* for users of technology that supports WAI-ARIA 1.0.
Edit: Users of AT that supports the aria-describedby property of WAI-ARIA 1.0
Comment #14
Liam MorlandWhat if we just changed theme_form_element so that the description was wrapped in a label element with a @for pointing to the form control? The HTML standard allows more than one label element per form control. This would solve the issue even for UAs that don't support ARIA.
Comment #15
Liam MorlandHere is a patch implementing my suggestion. It is rolled against D8, but that function hasn't changed so it could work with D7 as well.
Minor CSS changes are needed to prevent the description from coming up bold.
Comment #17
Liam MorlandI don't understand the error message. Can any one shed some light on this?
Comment #18
Everett Zufelt CreditAttribution: Everett Zufelt commentedThanks for the patch.
In the accessibility API on different platforms there is room for a role, name, and description. label normally maps to name and aria-describedby maps to description. I think that this is the most meaningful relationship that we can make, and so we should continue to look at using aria-describedby for the field descriptions.
Role is either determined by the element itself, or the aria role="" on the element.
Comment #19
mgifford#15: description-label.patch queued for re-testing.
Comment #20
mgiffordLiam, you were wondering about this SimpleTest error, right:
Custom checkboxes option description found. Other form.test 426 FormElementTestCase->testOptions()
as well as the one on the same line for radios. I reran the module and am hoping that this resolves it. Sometimes bots give us grief.
Everett, so we should re-write this patch for us of http://www.w3.org/TR/wai-aria/states_and_properties#aria-describedby
I haven't used this yet, but there are other references here http://www.marcozehe.de/2008/03/23/easy-aria-tip-2-aria-labelledby-and-a...
Comment #22
Liam Morland@#20 Yes, that is the error I am confused about.
@#18 Attached is a patch that adds @aria-describedby to the form control and a corresponding @id to the description. It only generates the description div if there is a description and the element has an @id. Previously, it did not check for the existence of @id. Perhaps someone who knows more about the form API will know if there are times when there is no @id, but there is a description.
Comment #23
Everett Zufelt CreditAttribution: Everett Zufelt commentedI took a look at the most recent patch.
Initially I thought that this looked good. After a bit more thought I pondered what happens when we have compound elements, like the Date element. With date elements, AFAIK, the element ID and ari-describedby property would be applied to the date dive itself, and not to any of the form controls that make up the date control.
I think that we will have to apply aria-describedby in the theming functions for each element specifically, so that we can best handle these edge case scenarios.
Someone more familar with form API can correct me if I am wrong on this.
Comment #24
Liam MorlandFor a compound element, such as date, the components, such as month and year, should be wrapped in a fieldset. The fieldset should have an aria-describedby attribute pointing to the description. I don't think it would be correct to have each component also point to that description since the description describes the entire date, not just the month or year.
Comment #25
Everett Zufelt CreditAttribution: Everett Zufelt commented@Liam
Agreed, compound elements should be wrapped in a fieldset, see: #504962: Provide a compound form element with accessible labels
We will have to do testing to see how assistive tech (screen-readers, treat aria-describedby on fieldsets. I agree that aria-describedby should be on the fieldset, but if that is giving us no benefit with screen-readers then we might need to re-think.
Comment #26
Liam MorlandIs there something I can do to help with this? The issue you mention appears to be stalled.
Comment #27
Everett Zufelt CreditAttribution: Everett Zufelt commentedYes, it didn't make it into 7, so it was stalled for 8.
As I see it we need to do two things.
1. Fix that issue, I see you have posted a comment there.
2. Test to see how screen-readers treat aria-describedby on fieldsets. This doesn't have to be tested if we can find others who have already tested, or who are willing to test. I'll put out a tweet about it.
Comment #28
Everett Zufelt CreditAttribution: Everett Zufelt commentedI did some testing of aria-describedby on fieldsets
1. JAWS 11 / FF 3.6
When tabbing to the first element in a fieldset (in forms mode) the legend and text in the aria-describedby reference are spoken. Noting happens on subsequent controls in the fieldset, or when navigating to controls in the virtual buffer.
2. NVDA 2010.1 / FF 3.6
Same as for JAWS
3. VoiceOver and Safari 5
Nothing is spoken when navigating to the controls in any way. However, when navigating to the fieldset (in object navigation mode) the aria-described by text is spoken as a description of the group. The user must then interact with the group to fin the controls.
From the above I am comfortable adding aria-describedby to fieldsets. It will provide an enhancement for some users.
Comment #29
mgiffordPatch applies nicely but the output doesn't look at all like Everett's HTML. I just wanted to report that it's been tested & needs to be written to focus the information on the fieldset.
Liam, was great hanging out with you & Everett at http://www.accessconf.ca
Comment #30
Liam MorlandIt was great to meet both of you.
Everett's HTML is a test case for using aria-describedby to associate a description with a fieldset. That will be used when compound elements are wrapped in a fieldset. That part of the problem is being addressed in #504962: Provide a compound form element with accessible labels .
The patch in this issue does the first part of the problem, which is using aria-describedby for descriptions of elements in general.
Comment #31
mgiffordI'd lost track of this issue so was looking at what to test for.
In looking at this code however:
Occurred to me that it is entirely possible that the description has an ID already. In which case we should use that rather than replace it. I think that should be possible but haven't checked.
Can this patch be properly tested without the grouped fieldsets issue in #504962?
Comment #32
Everett Zufelt CreditAttribution: Everett Zufelt commented@mgifford
It doesn't appear to me that the description currently has an id. Can you test to see that aria-describedby is being properly applid to:
1. form elements (input, select, etc) with a description.
2. The div wrapping compound elements (date, checkboxes, radios).
3. Not being applied to any form elements, or compound elements, without a description
@liam
I think that we can proceed with the patch without the fieldset issue being committed. This will not provide any benefit to compound elements at this time, but will apply and work nicely when the fieldset issue is fixed.
Comment #33
mgiffordIt's not a question about whether or not a description does presently have an ID assigned to it. This would override it so that a form description couldn't have an id assigned for it.
I think we should check if an ID is assigned before forcing in a new one.
Comment #34
Liam MorlandThe D8 code has no provision for applying an @id to the description div element. It is hard-coded that the div only gets a @class, so we're safe to add our own.
Comment #35
mgiffordExcellent. Thanks for checking Liam.
Looking at the source & searching for --description it is easy to see how these are nicely paired to descriptions.
Date:
Radio buttons:
Weight:
But it only shows up once for the Image upload:
I do think though that this last one with the images can be treated in a separate issue. The rest seems to work very well.
Comment #36
Liam MorlandImage upload clearly needs work: The label @for doesn't point to anything. This is another case where the multiple input elements should be wrapped in a fieldset, the label should be the legend, and the fieldset should have the @aria-describedby pointing to the description. I agree that fixing this should be a separate issue.
Comment #37
Everett Zufelt CreditAttribution: Everett Zufelt commentedI think we will have to find a different approach, other than modifying drupal_render()
The goal is that D8 will be html5, but that contributed themes can be xhtml 1.0 by overriding / altering content. AFAIK, there is no way for a theme to alter drupal_render(), so they would not be able to remove the aria-describedby attributes, therefore causing them to not validate under xhtml 1.0
I'll ping @jacine to get her input.
Comment #38
Liam MorlandWhat if drupal_attributes() was patched so that it would remove any aria-* attributes if the installation or theme was configured to not display them?
Comment #39
Everett Zufelt CreditAttribution: Everett Zufelt commented@Liam
Interesting suggestion. Can you integrate it into the discussion at #966396: Decide how to include WAI-ARIA landmark roles please.
If we had a setting for ARIA (or html5 even) on / off, then when off we could remove all role and aria-* attributes.
Comment #40
Liam MorlandHere is a patch that adds @aria-describedby in form_builder() instead of drupal_render().
Comment #41
JacineComment #42
Everett Zufelt CreditAttribution: Everett Zufelt commentedThis looks good to me. I think we need to make sure that we file a follow-up once this is committed for the grouped elements.
Adding aria-describedby to #attributes means that if for some reason a theme wishes to remove this they can do so in a preprocess function.
Comment #43
Everett Zufelt CreditAttribution: Everett Zufelt commentedI took one more look through.
I think we can remove the condition !empty($element['#id']) from form_builder(). Earlier in the function, near the beginning, there is already a test, and if it returns false an #id is generated for $element.
As for theme_form_element() I think we should leave the test, as it is possible, however unlikely, that #id will be unset. We might also want to still print #description even if there isn't an #id on $element.
I'm also wondering if it might be worthwhile to add the URL to WAI-ARIA description of aria-describedby to the comment in form_builder(). This might be unnecessary, but could be helpful for those who are unfamiliar with WAI-ARIA.
I think that if we do this we can mark this RTBC.
Comment #44
Everett Zufelt CreditAttribution: Everett Zufelt commentedJust noticed that in form_builder() the patch is actually acting on $element[$key][*]. This is within a foreach and $key is the child of the current element.
This works as expected, but I think that we are actually iterating over the elements more than once, once as children, and again as parents. I think we should move the logic out of the foreach so that we are performing this logic once per form element.
Comment #45
Everett Zufelt CreditAttribution: Everett Zufelt commentedTagging.
Comment #46
Everett Zufelt CreditAttribution: Everett Zufelt commentedMade changes from #43 - #44, left out the URL to WAI-ARIA aria-describedby.
Comment #47
Everett Zufelt CreditAttribution: Everett Zufelt commentedRegarding compound elements (image, file, date, radios, checkboxes), these are not the only elements that concern me. Currently the patch will also apply aria-describedby to table tableselect, etc. I'm not sure how to best handle this.
I suppose that for compound elements hopefully our solution for wrapping those (fieldset or otherwise) will carry semantics that will still allow the description to be mapped to accessibility APIs by UAs. For elements where this is not the case the attribute will still be in place, but will likely provide little to no benefit to AT.
Comment #48
JacineCan we please capitalize "add" and add a period at the end of the sentence. Otherwise, from my limited testing, it looks good.
Comment #49
mgifford#46: aria-describedby_1.patch queued for re-testing.
Comment #50
JacineThe patch applies and passes testing, but it still needs work per #48.
Comment #51
Liam MorlandAttached patch implements notes in #48.
Comment #52
Everett Zufelt CreditAttribution: Everett Zufelt commentedI still think we need to figure out how to deal with weird / complex elements, like tableselect. Should these elements receive aria-describedby, or should we create an array of acceptable #types that should receive aria-describedby and test in form_builder()?
Pro: elements that don't need aria-describedby don't get it.
Con: not very extensible to new / contrib elements.
Comment #53
Everett Zufelt CreditAttribution: Everett Zufelt commentedComment #54
mgiffordPatch applies nicely. It's easy to test it by creating content with core, say /node/add/forum
Everett, any ideas on how to deal with the weird & complex?
Comment #55
Everett Zufelt CreditAttribution: Everett Zufelt commentedNope.
On one hand we don't wnt to use a closed list of element types, because then the solution is not extensible. On the other hand a closed list ensures tat aria-describedby only gets applied where appropriate (e.g input, textarea, select), and not table, tableselect, etc, other compound elements.
Comment #56
mgiffordCan we just exclude the weird/complex from this solution & build something that we know works in the use cases we understand?
I'd like to have some way we can move ahead on this.
Comment #57
Liam MorlandMike, I agree. I think this patch should be committed so people will see how it works in the real world. That might help us to figure out a way to deal with complex elements.
Comment #58
mgifford#51: aria-describedby.patch queued for re-testing.
Comment #60
Liam MorlandUpdated patch rolled against latest D8.
Comment #61
mgiffordOk, I applied this and git accepted it with no problems. However, when I looked at the output of the fields in admin/config/development/performance that had description elements, I didn't see id's on the div tags or links to those ID's. Am I missing something? I cleared the cache.
Comment #62
Liam Morland@id and @aria-describedby are showing up for me in admin/config/development/performance.
Comment #63
mgifford#60: aria-describedby.patch queued for re-testing.
Comment #64
tlattimore CreditAttribution: tlattimore commentedI can confirm that the patch applies cleanly and that the id and aria-describeby are showing up for me on admin/config/development/performance.
Comment #65
Ron Williams CreditAttribution: Ron Williams commentedReviewed with eddib as part of the DrupalCon core sprint to test. Works as described.
Comment #66
chx CreditAttribution: chx commentedSo why this is not good for D7? it seems to me the only relevant markup change is adding an id attribute to the <div class=" description">.
Comment #67
mgifford@chx take a look at #35 for examples of the output.
The point of this is to tie in a description to a form element so that it is given more context. I put up a link to more info about aria-describedby in #20 above.
It's not about markup, it's about adding additional semantics. It adds meaning to the forms where there is none at the moment.
Hope this helps to clarify. And yes, would be great to backport it to D7.
Comment #68
webchickSounds like the aria-describedby attribute is not valid XHTML, so this is probably not a candidate for D7. Or am I wrong? Everett seems to be saying as much in #37.
Comment #69
mgiffordLooking to hear back from @Everett, but it seems to be fine with xHTML HERE - http://www.w3.org/TR/WCAG20-TECHS/ARIA1.html
Could well be that the docs are out of date though.
Comment #70
Liam MorlandARIA is not part of normal (X)HTML, but there are doctypes you can use which include it. I had understood that the idea was to include ARIA in D8 and have the Accessible Helper Module take care of it for D7, though that seems to have stalled.
Comment #71
mgiffordYes, the Accessible Helper module needs more love. I put an intern on it this summer for a bit, but it's not really playing the role I'd envisioned.
Ultimately accessibility doesn't tend to get the resources that are required to make it work properly.. Lots of folks just assume it's there and that it will just work somehow.
Comment #72
catchThanks folks. Committed/pushed to 8.x, moving back to 7.x for backport - not entirely clear if it can be or not.
Comment #73
Liam MorlandIt can be backported. The question is, Do we want to introduce ARIA markup into D7 core? Or should we be making a version of the Accessible Fix module which includes just those fixes which we have working now, such as this one?
Comment #74
bowersox CreditAttribution: bowersox commentedI believe that we DO have ARIA markup in core that is added by Javascript on the browser side (e.g. http://drupal.org/node/521904). But we DO NOT have ARIA markup in D7 core that is directly included in theme files/functions and HTML output on the server side. It was decided that we did not want to break HTML validity for D7.
For D8 we agreed to change our approach and allow ARIA markup directly into core theme files/functions: http://drupal.org/node/963760
So the patch above should not go into D7 core because it applies the ARIA markup on the server side. It would be possible (although tedious) to write a new patch that introduces a D7-only Javascript solution for adding aria-describedby on the browser side.
I would love to be corrected about this.
Comment #75
Everett Zufelt CreditAttribution: Everett Zufelt commented@Webchick is correct. aria-describedby is not valid xhtml 1.0. A decision was made in the d7 dev cycle to not invalidate markup, so I think we should stick with that. If someone feels strongly that this is the wrong decision please open a new issue as a task to discuss changing the markup validity policy.
Comment #76.0
(not verified) CreditAttribution: commentedCleanup issue with proper summary.