Give fieldsets an id

"ID and NAME tokens must begin with a letter ([A-Za-z]) and may be followed by any number of letters, digits ([0-9]), hyphens ("-"), underscores ("_"), colons (":"), and periods (".")." http://www.w3.org/TR/html4/types.html#type-name

I don't think #title will consistently fulfill that at all.

Crell #2: I actually think that this is an issue.. almost every form-item has an id, except for fieldsets, and this is not very consistent.

drumm #1: I have re-written the patch using form_clean_id so that the string is of the correct format.

Should the drupal_strtolower call become part of the form_clean_id() function?

I believe it should. Although the w3c does not specifically state so, it has become a standard throughout drupal and the web development community in general. I have attached a patch which still adds ids to fieldsets but also alters the form_clean_id function.

I agree with UltimateBoy. It looks to be an unsaid standard in Drupal anyway, so this will just give things that extra level of consistency.

I thought we should be nice to both $element['#id'] and $element['#attributes']['id'] if they already exist.

Ah yes.. good point. Applies cleanly and gets the job done. This is a simple enough patch, so I am marking as rtbc.

Shouldn't it be using the fieldset name instead of the title? Since the title can be translated, it makes the ID kind of useless.

This adds a #key property to all the form elements, and then uses that to construct the ID of fieldsets. I was having a look at #name and #id, but we can't really use those because they're required special use cases for input elements.

I like this patch, but is there a reason why you removed the drupal_strtolower call from form_clean_id()?

I removed the call to drupal_strtolower from the previous patch because now it's using the element's actual key rather then the element's title. So, there won't be any capitals being passed through it anyway. The call therefore isn't required.

The form element's key is what it's called when using $form['myelement']....The key for $form['myelement'] is "myelement".

Linking the fieldset id to the title is not recommended IMO. What if the site is multilanguage? A better approach would be using a static variable in the theme_fieldset method that increments itself every time a fieldset is placed. This is what I came out with (tested D6)

function theme_fieldset($element) {
  static $fieldsetNum = 0;
  if (!empty($element['#collapsible'])) {
    drupal_add_js('misc/collapse.js');

    if (!isset($element['#attributes']['class'])) {
      $element['#attributes']['class'] = '';
    }

    $element['#attributes']['class'] .= ' collapsible';
    if (!empty($element['#collapsed'])) {
      $element['#attributes']['class'] .= ' collapsed';
    } 
  }
  
  $element['#attributes']['class'] .= " fieldset-".$fieldsetNum++;

  return '<fieldset'. drupal_attributes($element['#attributes']) .'>'. ($element['#title'] ? '<legend>'. $element['#title'] .'</legend>' : '') . (isset($element['#description']) && $element['#description'] ? '<div class="description">'. $element['#description'] .'</div>' : '') . (!empty($element['#children']) ? $element['#children'] : '') . (isset($element['#value']) ? $element['#value'] : '') ."</fieldset>\n";
}

Cheers

The patch I posted does not use the title, it uses the form's key name instead, which is always the same no matter if the site is multilingual. The form_clean_id function that is used in the patch manages that static variable numbering that you're talking about for us.

Re-roll.

looks nice to me. bot is happy so rtbc

I wonder if it makes sense to prefix the ID with something. I imagine if you named your fieldset 'messages' or 'content' or something similar we'd end up with duplicate IDs on the page.

I'd also love to get a FAPI guru to ensure the additions to form_builder() are okay.

form_clean_id() ensures uniqueness and is used in this patch.

That's some mighty convoluted patch. Why are we not setting #id right where this patch hands down key instead? This would give all #input FALSE an #id.

#680196: Add specific classes to vertical tab li elements

form_process_fieldset injects:

  $element['#attributes']['id'] = $element['#id'];

Not sure which patch had a green light, but what I did is put the div id equal to class. To do that, I inserted the following code in lines 1515-1517 in form.inc:

if (!isset($element['#attributes']['id'])) {
      $element['#attributes']['id'] = 'fieldset-' . $element['#attributes']['class'];
}

I'm not a programmer so I'm not sure if it's the best way, but it doesn't rely on the #title so there will be no problems with i18n.

A bit of info if you're trying to add an id to a location fieldset. You can't add the id to the locations part of the $form, but instead have to add it to the location array under it. So:

$form['locations'][0]['#attributes']['id'] = t('location');

Rather than what I thought was more obvious:

$form['locations']['#attributes']['id'] = t('location');