This was a complex issue that took a few hours of debugging, but I figured it out.

The issue:

The problem is with using code like this in theme functions:

$attributes = $element['#wrapper_attributes'];
...
return '<div ' . drupal_attributes($attributes) . '>' . theme('form_element', $element) . '</div>';

Code like this is used in theme_date_popup(), theme_date_timezone(), theme_date_select(), and theme_date_text().

Why this is an issue:

This is a problem because drupal_attributes() modifies the given array's elements in place:

function drupal_attributes(array $attributes = array()) {
  foreach ($attributes as $attribute => &$data) {
  ...

So when you call drupal_attributes(), $element['#wrapper_attributes'] is modified as well. This wasn't a problem in other code I saw, because nothing else in Drupal was rendering an element's attributes and then theming the element itself; they always theme the element's children instead. Since instead we are theming the element itself, $element['#wrapper_attributes'] has to be an array still so that theme implementations and hooks can modify the element attributes.

As it is, $element['#wrapper_attributes'] is set to a string like class="date-no-float container-inline-date" instead of an array, so if a theme function later calls $element['#wrapper_attributes'][] = 'some-class', it creates a fatal error.

Reproducing the issue:

This can be seen by overriding theme_form_element() in a form-element.func.php theme file and trying to modify the array that was erroneously passed as a string.

/**
 * Overrides theme_form_element().
 */
function THEME_form_element(&$variables) {
  $element = &$variables['element'];
  if ($element['#id'] == 'edit-field-day-und-0-value') {
    dpm($element['#wrapper_attributes']['class']);
  }

Fixing the issue:

I see two easy ways to fix this:

  // Create a copy of the array so that drupal_attributes() doesn't also modify
  // $element['#wrapper_attributes'].
  foreach ($attributes as $key => $value) {
    $copy[$key] = $value;
  }
  $attributes = $copy;

  return '<div ' . drupal_attributes($attributes) . '>' . theme('form_element', $element) . '</div>';

Or:

  // Theme the element before calling drupal_attributes(), which modifies 
  // $element['#wrapper_attributes'].
  $theme = theme('form_element', $element);

  return '<div ' . drupal_attributes($attributes) . '>' . $theme . '</div>';

The second option is similar to code in theme_date_display_interval(), and I think it looks nicer and faster.

Comments

solideogloria created an issue. See original summary.

solideogloria’s picture

Status: Active » Needs review
StatusFileSize
new2.69 KB

This patch uses the second fix option described in the issue.

solideogloria’s picture

The code/patch is the same for 7.x-3.0-alpha1

solideogloria’s picture

damienmckenna’s picture

This looks like a good idea, thanks for tracking it down.

damienmckenna’s picture

Status: Needs review » Fixed

Committed.

Status: Fixed » Closed (fixed)

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

solideogloria’s picture

Did you also commit this to 7.x-3.0-alpha? That branch needs the fix as well.

solideogloria’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev
damienmckenna’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev

I need to cherry-pick a few commits from 7.x-2.x and create another alpha.