Drupal Version: 5.1
The HTML generated by the theme_radio() function is not accessible. For it to be so, the label requires a 'for' attribute for the corresponding 'id'.
I have seen it argued that if the 'label' contains the 'input' element that then it accessible. This is NOT true, please see the following to verify:
http://www.w3.org/TR/WCAG10-HTML-TECHS/#forms-labels
http://www.w3.org/TR/2007/WD-WCAG20-TECHS-20070517/Overview.html#H44
A site I was developing needed to be WAI level 3 (AAA) compliant. My solution somewhat hacked solution is as follows.
Note, I created a unique ID for each input element by concatenating the ID, value and appending a random number to the end.
The random number was required as there was an issue with the poll module where the same poll form appears twice on the page.
function theme_radio($element) {
// Create a random number to append to the ID
// This is required as with e.g. poll mudule, the poll
// node appears twice resulting in duplicate IDs
$new_rand_no = (mt_rand(10,1000000));
_form_set_class($element, array('form-radio'));
$output = '<input type="radio" ';
$output .= 'name="' . $element['#name'] .'" ';
$output .= 'value="'. $element['#return_value'] .'" ';
$output .= 'id="'. $element['#id'] . '_' . $element['#return_value'] . $new_rand_no .'" ';
$output .= (check_plain($element['#value']) == $element['#return_value']) ? ' checked="checked" ' : ' ';
$output .= drupal_attributes($element['#attributes']) .' />';
if (!is_null($element['#title'])) {
$output = '<label class="option" for="' . $element['#id'] . '_' . $element['#return_value'] . $new_rand_no .'">'. $output .' '. $element['#title'] .'</label>';
}
unset($element['#title']);
return theme('form_element', $element, $output);
}
Comment | File | Size | Author |
---|---|---|---|
#23 | 152098-radio-checkbox-accessible-D6.patch | 1.21 KB | Dave Reid |
#14 | form.inc_labels4.patch | 1.04 KB | mgifford |
#13 | form.inc_labels4.patch | 1.04 KB | mgifford |
#10 | form.inc_labels3.patch | 1.02 KB | mgifford |
#7 | form.inc_labels2.patch | 1 KB | mgifford |
Comments
Comment #1
chx CreditAttribution: chx commentedNever forget that you can always override a theme function.
Comment #2
mgiffordYes, you can override a theme function, but by default Drupal should be aiming to produce code that Validates against WCAG 2.0 if it is applicable and possible, right?
So this is from http://api.drupal.org/api/function/theme_radio/7
Comparing it to the tech specs here:
http://www.w3.org/TR/2007/WD-WCAG20-TECHS-20070517/Overview.html#H71
Shouldn't this function be modified to include the for="ID" reference similar to that recommended by hunthunthunt on June 15, 2007?
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedSounds reasonable. Why not suggesting a patch?
Comment #4
mgiffordOk, I've done this now. Thanks for the encouragement. I extended this for checkboxes too as it was the same issue.
I'm not sure if this is a problem or not:
Check for updates:
A label that doesn't associate with any form elements is, well, kinda lame..
Wouldn't make accessibility any easier.
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedPlease see http://drupal.org/patch/create on how to create a patch that would make our (picky) test system happy.
Comment #7
mgiffordI used diff -Naur which added some extra stuff.
Thanks for the link, I used the new extension so hopefully it works.
Mike
Comment #8
StevenPatzComment #10
mgifford3rd time lucky???
Seems more changed in the CVS than I had thought. Trying this time against the CVS version (which I should have done the first time).
Comment #11
StevenPatzComment #13
mgiffordOk, maybe it was because I didn't do it from the root directory... Hopefully this works.
Comment #14
mgiffordDon't think I changed the status to test the patch.
Comment #15
svendecabooterPatch works as advertised.
Tested with form elements of #type radios, checkbox and checkboxes.
Comment #16
mgiffordThanks Sven.. So what's the process to get this into core? Would it just get bundled in 6.10 or is this something that would hit in Drupal 7?
Seems to me it's something that would help 6.x folks without having any significant costs.
Comment #17
svendecabooterThe process is always to get it in the latest version (HEAD) of core.
If there are reasons to backport it into earlier versions of Drupal, it might be done.
However since this can be solved with theming in earlier versions of Drupal, I don't think it will be considered to be backported...
Comment #18
mgiffordThanks.. So if Webchick (and co.) decide to bring it into Drupal 7 then great. But even though it's passed the tests, gotten reviewed and is such a simple change, what needs to happen to bring it into core so that general accessibility is enhanced?
Just wait and see or see if I can rally some +1's?
I think that Drupal just has this kinda stuff taken care of. Unfortunately with WCAG 2.0 just released I think we're just at the beginning of a review process for accessibility.
Comment #19
Cliff CreditAttribution: Cliff commentedDefinitely this needs to get into core for Drupal 7.
Backporting it to earlier versions would also be a most welcome sign to the accessibility community at large (which overlaps significantly with usability) that Drupal is seriously intended to be a CMS for everyone. In other words, backporting it would help us win over more converts to Drupal.
Also, please consider this: I'm developing my site today in D6. I've got enough local concerns to cover—figuring out the roles I need, figuring out modules to install, getting the taxonomy right—that it doesn't make sense to expect me to spend more time figuring out how to make sure this basic need for all sites is taken care of by my site's theme. Accessibility is a core need, not a design frill.
Well done, mgifford!
Comment #20
catchSmall fix, several positive reviews,applies cleanly and looks RTBC to me. For backport, the decision on that rests with Gabor - once it's in D7 you can post the equivalent patch against D6 and see if it gets in there. I can't think of any reasons why it'd do any harm in D6, so it's got at least some chance of getting in.
Comment #21
webchickCommitted to HEAD, thanks! :)
Marking back to 6.x for consideration.
Comment #22
Gábor HojtsyThis does not apply at least because of D6 -> D7 code style changes. Otherwise I'd be happy to commit, so please reroll.
Comment #23
Dave ReidPorted for 6.x. Going to go ahead and mark as RTBC since it's a straight backport of the 7.x patch and tested locally.
Comment #24
webchickComment #25
Gábor HojtsyDave: your second hunk had code style problems for D6 still, but I fixed those and committed the patch that way. Thanks!