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);
	}

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

Priority: Critical » Normal
Status: Needs work » Active

Never forget that you can always override a theme function.

mgifford’s picture

Version: 5.1 » 7.x-dev

Yes, 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?

function theme_radio($element) {
  _form_set_class($element, array('form-radio'));
  $output = '<input type="radio" ';
  $output .= 'id="' . $element['#id'] . '" ';
  $output .= 'name="' . $element['#name'] . '" ';
  $output .= 'value="' . $element['#return_value'] . '" ';
  $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'] . '">' . $output . ' ' . $element['#title'] . '</label>';
  }

  unset($element['#title']);
  return theme('form_element', $element, $output);
}
Damien Tournoud’s picture

Sounds reasonable. Why not suggesting a patch?

mgifford’s picture

Assigned: Unassigned » mgifford
Status: Active » Needs review
FileSize
956 bytes

Ok, 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.

Status: Needs review » Needs work

The last submitted patch failed testing.

Damien Tournoud’s picture

Please see http://drupal.org/patch/create on how to create a patch that would make our (picky) test system happy.

mgifford’s picture

FileSize
1 KB

I used diff -Naur which added some extra stuff.

Thanks for the link, I used the new extension so hopefully it works.

Mike

StevenPatz’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

mgifford’s picture

Title: Radio buttons are not accessible. » Radio buttons are not accessible.
FileSize
1.02 KB

3rd 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).

StevenPatz’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

mgifford’s picture

FileSize
1.04 KB

Ok, maybe it was because I didn't do it from the root directory... Hopefully this works.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.04 KB

Don't think I changed the status to test the patch.

svendecabooter’s picture

Patch works as advertised.
Tested with form elements of #type radios, checkbox and checkboxes.

mgifford’s picture

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

svendecabooter’s picture

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

mgifford’s picture

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

Cliff’s picture

Definitely 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!

catch’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs backport to D6, +Quick fix

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

webchick’s picture

Version: 7.x-dev » 6.x-dev

Committed to HEAD, thanks! :)

Marking back to 6.x for consideration.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Patch (to be ported)

This does not apply at least because of D6 -> D7 code style changes. Otherwise I'd be happy to commit, so please reroll.

Dave Reid’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
1.21 KB

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

webchick’s picture

Issue tags: +Accessibility
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Dave: your second hunk had code style problems for D6 still, but I fixed those and committed the patch that way. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D6, -Quick fix, -Accessibility

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