Orphaned Labels Being Generated for Radios, Checkboxes & Items

tagging.

also see:

#882666: Core form descriptions shouldn't use a label when not associated with a form

It's mostly just bad HTML, but it's also producing errors for some accessibility validation software.

This is resolved in the same place #884866: Form type item shouldn't be generating a label by default. and is the same problem so I'm going to treat the other one as a duplicate & work on it here.

I think I'll squish the notices here, but am a bit confused as to why the 1 fail is happening. The Required 'checkboxes' field is marked as required text should still be output. FormsTestCase->testRequiredFields() form.test shouldn't pose a problem as it should be exported by '!title !required !error' in this patch, just associated with an H3 rather than a label.

              // Make sure the form element is marked as required.
              $this->assertTrue(preg_match($required_marker_preg, $form_output), "Required '$type' field is marked as required");

The isset() should cut down the notices though.

Quick summary: <label> elements should only be used when associated & tied to a single form element. In the current Forms API the theme_label() function also generates them for radios, checkboxes & items which is incorrect HTML & confusing for the user. It may present accessibility issues to some users as well, but certainly does for validators like WebAIM that looks for orphaned labels.

@sun on the LABEL -> H3 thing, worth looking at #882666: Core form descriptions shouldn't use a label when not associated with a form as there is a precedent there now.

I do think that the heading, is a useful navigational aid for screen readers where as <span class="form-label"> doesn't present any support for navigating the stream of content on a page. I did think about applying a class as you have, but as a model I think <h3 class="form-label"> presents a better way to structure the data presented to the user.

Totally agreed to "This has to be standardized and a unique pattern to be properly themed."

I also agree it's a bit funny to have theme_form_element_label() be outputting things that are more than just labels. I was also looking at the theme_form_element_label() and it seems like there is a lot more in there than stuff than most themers should be modifying.

So, what about something like this:

function _form_element_label($variables) {
  $element = $variables['element'];
  // This is also used in the installer, pre-database setup.
  $t = get_t();

  // If title and required marker are both empty, output no label.
  if (empty($element['#title']) && empty($element['#required'])) {
    return '';

  // If the element is required, a required marker is appended to the label.
  $required = !empty($element['#required']) ? theme('form_required_marker', array('element' => $element)) : '';

  // If the element has an error, append the themeable marker to the label.
  $error = theme('form_error_marker', array('element' => $element));

  $title = filter_xss_admin($element['#title']);

  $attributes = array();
  // Style the label as class option to display inline with the element.
  if ($element['#title_display'] == 'after') {
    $attributes['class'] = 'option';
  }
  // Show label only to screen readers to avoid disruption in visual flows.
  elseif ($element['#title_display'] == 'invisible') {
    $attributes['class'] = 'element-invisible';
  }

  if (!empty($element['#id'])) {
    $attributes['for'] = $element['#id'];
  }

  $var = array('attributes' => $attributes, 'title' => $title, 'required' => $required, 'error' => $error);
  if (isset($element['#type']) && ($element['#type'] == 'item' || $element['#type'] == 'radios' || $element['#type'] == 'checkboxes')) {
    return theme_form_element_not_label($var);
  } else
  { 
    return theme_form_element_label($var);
  }
}

function theme_form_element_label($var) {
  // The leading whitespace helps visually separate fields from inline labels.
  $output = ' <label' . drupal_attributes($var['attributes']) . '>';
  $output .= $t('!title !required !error', array('!title' => $var['title'], '!required' => $var['required'], '!error' => $var['error']));
  $output .= "</label>\n";
  return $output;
}

function theme_form_element_not_label($var) {
  // The leading whitespace helps visually separate fields from inline headers.
  $output = ' <h3' . drupal_attributes($var['attributes']) . '>';
  $output .= $t('!title !required !error', array('!title' => $var['title'], '!required' => $var['required'], '!error' => $var['error']));
  $output .= "</h3>\n";
  return $output;
}

It's really rough, and don't think it's worth it at this stage of drafting up a patch until there's more agreement about where it is going

In the case of checkboxes and radios I don't see this as being a big deal. You either want text above a list or you don't. If don't want the text then the element-invisible should still be passed through. Otherwise you've got a clump of data that really should be grouped together in some logical fashion. If we're not going to be able to do it with fieldsets & legends then headings the next best thing. Maybe there is a way to do this in D7 however...

#504962: Provide a compound form element with accessible labels

Maybe with a few small changes to theme_checkboxes & theme_radios we'd be set? I'm up for drafting something if there's a hope of getting it into core.

I don't know how much of a concern <h3>'s are for machine-readability. I haven't read anything that points to this as a problem. These links are geared to page content, but I think that they would still apply to form data if we aren't able to group it in a fieldset.
http://www.raymondselda.com/importance-of-html-headings-for-accessibility/
http://webaim.org/techniques/semanticstructure/

For #title_display, we have presently lists for before, after, invisible & attribute (I do hope that attribute is extended in D8 so that it is able to insert the title in as an attribute for any form element, however at the moment this isn't the case). Adding a Heading element could be done here, but that's a bit of a deviation from the others which are primarily tied to visibility.

I think the main issue of concern is how to deal with the item #type. The only thing I'm certain about is that there is no instance that this should be a <label>. Stylistically changing it to <h3> makes some sense in Seven, however not in other themes and it needs to be consistent. Maybe the best we can do with items is <strong class="form-label">. However, I'm not exactly sure how it's has been used. How do people want to be able to use this type of form element?

Well, that works well to address the main issues, thanks @sun.

We'd have to add CSS to ensure that this is bold and styled like the label is:

  <div class="form-label">Content types </div>

That's not a big deal though. I tested:
- checkboxes - admin/config/development/generate/content
- radios - admin/config/regional/settings
- items - admin/structure/block/manage/system/help/configure

None of the previous errors were there. I like using headings, but it's not a deal breaker if they aren't there.

So how do we move this ahead? This patch removes a lot of warnings.

The theming needed to be applied so that there wasn't a change in the visual appearance due to this patch. I've added the css now to this patch & included screen shots of examples of the 3 places where there are changes.

Other than that it seems to be ready to go!

Yup. Sorry, got that now.

Also, found relevant code in Bartik & Seven to add it to. Garland doesn't do much to this type of label in my review.

We're going to need a mass file uploading module.... Anyways, here they are. All changes have been captured here.

And no, there should be no visible differences. from before the patch is applied as it is being applied with the same style.

Ya, I noticed this too. Not sure why. But was focusing on the patch.

#19: drupal.form-title-heading.19.patch queued for re-testing.

#21 has lots of screenshots so I'm removing that tag.

Ok, here's a re-rolled version using div.label's.

Ok, so I can try to follow http://tiger-fish.com/blog/drupal-6-using-xpath-your-simpletest-tests

I still find SimpleTests awkward to walk through.. Hopefully my mind will be clearer tomorrow & I can take this on.. I hab a cod.

My laptop bust & everything's been delayed. I don't think I'm going to be able to redo this test any time soon. I'm hoping things will resume to normalish next week after Canadian Thanksgiving.

I updated common.test & changed a patch to bartik's css & uploaded the tests for the bots.

oops.. cut/paste error.

#35: drupal.form-title-heading.23.patch queued for re-testing.

Would it make sense to draw out the model of a form here and describe in html how the various elements should occur for different types of forms? Define a best practice and then worry about how we want to get there?

I'd be fine to move the bulk of it over and close this as you suggest.

I was just looking for an instance where labels are used just to give emphasis in forms and was looking for additions like .form-item label, div.label

To help set up a practice for maintaining the existing labeling without spawning orphan labels all over the place in contrib modules.