Add a theme_form_element()/theme_fieldset() function that generates a unique id for form elements that don't have one

Thanks for reporting this. Yeah, you're right. The ID is missing from the wrapper div.

If you look in profile_checkboxes.module starting at line 58, the only form field attribute values changed for each case (i.e. radio button, checkboxes) are #type and #default_value. I believe this is indeed responsible for the missing ID though because the default field outputs all contain an ID for each corresponding wrapper div.

I don't understand why this is happening and unfortunately my current schedule doesn't provide me the time to debug right now. I would be appreciative if you or someone could diagnose the problem and report back here with your findings and perhaps provide a patch or code sample to remedy it.

I looked at this, and I don't think it's a bug.

If you expand the <div class="form-radios"> you'll find that each <input type="radio"> is surrounded by a <label>, surrounded by a <div>. That <div> has the ID, and not the outermost <div>. That makes sense, as it conforms to how all the other default profile field types are outputted.

I've attached an example

Brandon,

Profile Checkboxes doesn't do anything to manipulate/override the default profile field output so the exclusion of the ID from the outermost wrapper div for checkboxes and radio buttons must be deliberate.

Thanks for taking the time to investigate and post your findings.

Cheers,
John

I can confirm that the same effect of removing the wrapper IDs can be achieved by using the Form API instead of this module to affect the select type. Using hook_form_alter to change the field #type via $form['fieldset']['profile_field']['#type'] = 'radios'; also removes the wrapper ID.

It is possible to get around this using #prefix and #suffix to create a div wrapper with an ID -- maybe there's a way for this module to get what the ID value of the wrapper would be, and add it in?

@akalata,

I think your suggested solution of adding #prefix and #suffix div wrapper with an ID is a good one. Unfortunately my current work schedule permits me from testing it myself, but if you're able please give it a try and report back. Thanks!

John

Sounds like a limitation of Drupal6 FAPI that I've seen before. Fieldsets and wrappers don't get an ID when themed, even if given one through Form API.
Not really this modules fault.

I got around it by setting and defining a custom theme_fieldset function in the #theme of the form element. ... in an entirely different project. #prefix & #suffix looked too messy for me. Just FYI.

@dman,

Thanks for the clarification and vindication re: this issue. :-)

I like your theme function approach better using #prefix & #suffix attributes. Would you care to share a snippet or point me to a project that contains an example?

Cheers,
John

Something like this.
Looks like a lot, but it's just a paste from the core theme function.

..
...

  $form['advancedform_rules_global'] = array(
    '#type' => 'fieldset',
    '#title' => t('Rules for everybody'),
    '#theme' => 'fieldset_with_id',
  );

...
..



/**
 * Override of theme_fieldset.
 * 
 * This was needed to get the right id into the elements outermost wrapper.
 * May conflict with some themes :(
 * Copied from core with one change.
 *
 */
function theme_fieldset_with_id($element) {
  if ($element['#collapsible']) {
    drupal_add_js('misc/collapse.js');

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

    $element['#attributes']['class'] .= ' collapsible';
    if ($element['#collapsed']) {
     $element['#attributes']['class'] .= ' collapsed';
    }
  }
  // Fieldsets don't have ids. Make one
  $element['#attributes']['id'] = preg_replace('|[^a-z0-9]+|', '-', strtolower($element['#title'])) .'-wrapper';

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


/**
 * Override of theme_form_element.
 * 
 * This was needed to get the right id into the elements outermost wrapper.
 * Copied from core with one change.
 *
 */
function theme_form_element_with_id($element, $value) {
  $output  = '<div class="form-item" id="'. $element['#id'] .'-wrapper">'."\n";
  $required = !empty($element['#required']) ? '<span class="form-required" title="'. t('This field is required.') .'">*</span>' : '';

  if (!empty($element['#title'])) {
    $title = $element['#title'];
    if (!empty($element['#id'])) {
      $output .= ' <label for="'. $element['#id'] .'">'. t('!title: !required', array('!title' => filter_xss_admin($title), '!required' => $required)) ."</label>\n";
    }
    else {
      $output .= ' <label>'. t('!title: !required', array('!title' => filter_xss_admin($title), '!required' => $required)) ."</label>\n";
    }
  }

  $output .= " $value\n";

  if (!empty($element['#description'])) {
    $output .= ' <div class="description">'. $element['#description'] ."</div>\n";
  }

  $output .= "</div>\n";

  return $output;
}

Remember to declare the functions in hook_theme if you are going to use this.

An alternative would be to add a hook_form_element() in your template.php or in a module:
Copy the theme_form_element() function code from here, or from your existing core:
http://api.drupal.org/api/function/theme_form_element/6

Paste it in template.php as yourtheme_form_element() or in a module as yourmodule_form_element().

Then after this:

  if (!empty($element['#id'])) {
    $output .= ' id="'. $element['#id'] .'-wrapper"';
  }

Add this:

else {
  // Create an id if it doesn't have one
  $output .= ' id="edit-'. preg_replace('/_/', '-', $element['#name']) .'-wrapper"';
}

This could be added to the profile checkboxes module, but I agree, it seems to be a problem elsewhere.

xalen's solution seems the most elegant. Retitling.

There is no patch on this issue, so setting back to "active".

It would be elegant it it wasn't for the fact that theme_fieldset() doesn't use theme_element(), hence why a work-around is needed for them.

It works OK on just theme_checkboxes - which currently deliberately discards the #id (why?)
But I needed a solution that would insert IDs into fieldsets and wrapper divs.

Ok, sorry. Was just trying to clarify the direction moving forward. Retitling accordingly.

Nah, it's OK. In THIS issue, the checkboxes are all that were asked for, and that's sorta addressed here. The discussion drifted into the general FAPI ID thing. And core FAPI is not going to be fixed from here.

Clearly, this problem isn't specific to Profile Checkboxes. I think dman's proposed workaround is a good one. I'm going to set the status of this issue as fixed. Thanks!