Use @aria-describedby for fieldset descriptions

Looks like a useful addition. Patch applies nicely in git.

drupal_theme_fieldset_aria-describedby.patch queued for re-testing.

Looks good. There's an instance in core here:
admin/config/people/accounts

Weren't as many as I had expected though. Are there other good examples you've been looking at.

Any work on fieldsets should also be aware #1168246: Freedom For Fieldsets! Long Live The DETAILS.

#4: drupal_theme_fieldset_aria-describedby.patch queued for re-testing.

re-rolling to keep up with Core.

Seems like a simple addition to add more semantics.

#9: drupal_theme_fieldset_aria-describedby-1645420-9.patch queued for re-testing.

Hm. While this seems to make sense from a pure technical perspective...

...the "descriptions" that are usually output for fieldsets/details do not actually "describe" the fieldset/details. Most often, they are rather introduction texts that explain how to use the controls that are contained.

I'd understand a real description of a fieldset/details element as something like:

Contains advanced settings to customize the behavior related to foo and bar.

But that's not the case. The "descriptions" we have there are really advisory intro texts, along the lines of:

The foo and bar behavior (as configured on the Baz page) can be customized to control how this content appears for users. Use the following controls to adjust the contextual behavior, or go to the Blargh page to control the global behavior.

In short, "description" != "description". I'm not sure whether this makes sense.

Committed and pushed to 8.x. Thanks!

Attached patch fixes various problems with the committed patch.

@sun this applies nicely & provides the same results in the HTML .

This looks like a good improvement on the earlier patch.

Apparently we should be careful with aria-describedby. It crashes the browser in IE8+some versions of Jaws if target has more that 1024 characters of content.

Something funny with how Jaws does aria-describedby that causes memory issues that lead to crash situation.

#21: drupal8_aria-fieldset_1645420.patch queued for re-testing.

Rerolling the patch!

Well, I think that JAWS/IE is actually right in breaking badly on a too long description. Circles back into what I already said before: Our #description values do not contain the appropriate content for this aria-describedby reference.

But anyway, let's move forward with this.

I'm going to set this back to needs review to verify that it doesn't crash IE8 for JAWS users.

Sun, thanks for restating your concerns. I do take them seriously and am reaching out to see if I can get a few more opinions.

drupal_theme_fieldset_aria-describedby.patch queued for re-testing.

#25: drupal8_aria-fieldset_1645420-24.patch queued for re-testing.

@webchick, I am tagging this "RTBC Feb 18" as it has been ready, but just been really hard to find a Jaws user to test this patch. Otherwise it works as described.

Remove if required.

I have looked at the aria-describedby code... It is correct when a form field is created. I have tested it with NVDA 12 and Firefox 14 and it reads ok.

Is there a bug report we could link to for the JAWS issue?

I've never seen one for JAWS. Other screen readers do:
http://code.google.com/p/google-axs-chrome/issues/list
http://www.nvda-project.org/wiki/Development#IssueTracker

Closest I could find in JAWS (or VoiceOver) was:
http://stackoverflow.com/questions/tagged/voiceover
http://stackoverflow.com/questions/tagged/jaws

So, no. @catch I don't think there's anything we can file a bug report with.

#25: drupal8_aria-fieldset_1645420-24.patch queued for re-testing.

Interdiff between @Liam Morland's patch in 21 & mine in 25.

#25: drupal8_aria-fieldset_1645420-24.patch queued for re-testing.

#25: drupal8_aria-fieldset_1645420-24.patch queued for re-testing.

Testing.

I had difficulty testing this one manually because most of the things that were fieldsets in D7 are now "details" in D8. Yay HTML5!

Here's how I managed to find a fieldset:

1. Enable the core "Languages" module (drush -y en languages).
2. Go to Home > Administration > Configuration > Regional and language (admin/config/regional/language), and add a language.
3. Go to search/node and expand Advanced search.

But, no core fieldsets have descriptions, apparently, so you'll have to hack node.module in function node_form_search_form_alter and add one if you want to see the patch in action.

Due to the problems described here, http://api.drupal.org/comment/49228#comment-49228 , I've had to re-roll the patch so the ID on the description is output.

I could swear I tested that before I wrote the patch... :P

Lets try this again!

This is good to go I think. Here's a screenshot from admin/config/people/accounts

Woops - @mparker17 just reached out on Skype to inform me that the "screenshot you posted demonstrated an aria described-by attribute on a form-control, not aria described-by attribtue to description elements on *fieldsets*"

Back to Needs Review. I was going back to examples in #5 after walking through the steps in #40 & not wanting to "hack node.module in function node_form_search_form_alter ... to see the patch in action."

#44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

#44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

#44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

Adding the tags back in here.

44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

Rats, the patch doesn't apply anymore. I'll re-roll it.

Apparently testbot had a hiccup and didn't apply the patches the first time :P

Now we just need to deal with #46 and providing relevant examples.

Perhaps from admin/config/people/accounts

@mgifford I wasn't actually able to find a <fieldset> on that page when I tested the latest patch (in #44) last night... pretty much everything that used to be a <fieldset> is now a <details> element.

I spent a bit of time looking for fieldsets, but got distracted by dinner and being super-tired from a long week.

I don't know anything about the Form API's change from generally-using-<fieldset>s to generally-using-<details>, so I cannot tell if testing for #44's changes on a <details> element will produce a outcome relevant to this issue.

FWIW, when I tested last night, the <details> elements on admin/config/people/accounts had aria-describedby attributes, but there weren't corresponding id attributes in the page's HTML.

Marking as "Needs issue summary update" since that would be really handy.

Here are the remaining tasks:

1. Determine if this patch affects <details> elements as well, or whether we should file a separate issue for that.
2. Figure out a working (manual) test case. Tagging as "Needs tests".
3. Perform the test case with an a11y tool. Tagging as "Needs manual testing".

They are easy to find on the user page with this patch here:
https://drupal.org/node/504962

This wraps all radios & checkboxes in the fieldset.

Actually, tagging as "Novice", since the issue summary update and steps 1 and 2 should be pretty simple.

This change record might be a good place to start researching: All collapsible fieldsets have been replaced with HTML5 details elements.

44: drupal-aria-describedby-for-fieldset-desc-1645420-44.patch queued for re-testing.

This will need a re-roll after #2152209: Convert theme_fieldset() to Twig has landed.

Is the situation with that JAWS bug for descriptions longer than 1024 characters still the same today, or can we remove that adjustment?

I wonder whether it would make sense to move the follow-up patch into a new issue? The original patch was committed ~1 year ago already, and my original follow-up patch was meant to be committed as a super-quick follow-up only. Then we suddenly ran into that JAWS bug...

@mgifford or someone else: Could we create a meta issue for all (Form API related) accessibility issues that need to be resolved before beta/release? — A concrete overview (via parent issue link relationships) would be very helpful to have.

Looks like this might have been resolved in JAWS 14.0.1823 (April 2013) - http://www.freedomscientific.com/downloads/jaws/JAWS14-previous-enhancem...

This will still affect some users, but I think it might be alright to remove the adjustment. Partly this will depend on D8's lifecycle.

I've created an meta issue #2203649: Remaining Open Form API related Accessibility Issues - don't think I missed anything, but if so please edit it.

What happened to theme_fieldset() anyway? Not sure how to re-write this patch.

Updating this issue with task instructions in preparation for the sprints at Drupalcon austin.

Hi, working from mentored code sprints at Drupalcon Austin. I'm going to see if the patch needs to be rerolled.

Ok, not sure if anything needs to be done on this issue. Attached is a screen shot of a fieldset with a description from the site branding block configuration at /admin/structure/block/add/system_branding_block/bartik

<fieldset aria-describedby="edit-settings-block-branding--description" id="edit-settings-block-branding" class="form-wrapper form-item">

aria-describedby attribute is added in template_preprocess_fieldset() in core/includes/form.inc.

It doesn't look like the patch needs a reroll. Does core already address the issue?

John

Looks like it's fixed now in Core I couldn't replicate the problem on a fresh install.