Empty class attributes being printed in elements using $attributes, $title_attributes, or $content_attributes

I can not reproduce this. This behaviour is changed last week, did you clear your caches?

But this probably should be rewritten as
<div class="content <?php print $content_attributes['class']; ?>"<?php print $content_attributes; ?>>

Btw this happens in multiple templates

See also #1286530: Meta: Templates hard code class attribute, and shouldn't

This is happening in a few places, titles in addition to the divs webchick mentioned. You won't see the empty class="" when inspecting with Chrome, you need to view source.

It looks like this is coming from core/includes/theme.inc line 2334:

  if (!isset($default_attributes)) {
    $default_attributes = drupal_attributes(array('class' => array()));
  }
  $variables += $default_variables + array(
    'attributes' => clone $default_attributes,
    'title_attributes' => clone $default_attributes,
    'content_attributes' => clone $default_attributes,
  );

We're setting an empty class on $default_attributes and cloning it, so anywhere these variables are not getting additional attributes defined will result in the empty class. Attempting to pass nothing results in the following error wherever the variables are being populated:

Notice: Indirect modification of overloaded element of Drupal\Core\Template\Attribute has no effect

Any suggestions on the cleanest way to correct this?

Here is one possible fix for this, not sure if it's the best approach.

Yeah, this is not the best approach (most likely). There's a couple of problems here.

First, adding that class => array() definition is causing the empty class="" attribute being added as @webchick mentioned in this issue. But it's not just that which is at issue. It's having the side effect of constructing the attribute class with a 'valid' "class" property in storage, so later on when we do stuff like this we don't get an error:

$variables['attributes']['class'][] = drupal_html_class($hook);

If we take out the class => array() argument from the attribute class construction then we don't have the problem of empty class="" attributes being added willy nilly, but we do run into a constraint of using class objects as arrays, in that multiple dimmensions of overloading are not really supported.

From: http://php.net/manual/en/arrayaccess.offsetset.php

This function is not called in assignments by reference and otherwise indirect changes to array dimensions overloaded with ArrayAccess (indirect in the sense they are made not by changing the dimension directly, but by changing a sub-dimension or sub-property or assigning the array dimension by reference to another variable). Instead, ArrayAccess::offsetGet() is called. The operation will only be successful if that method returns by reference, which is only possible since PHP 5.3.4.

From: http://www.php.net/manual/en/arrayaccess.offsetget.php

Starting with PHP 5.3.4, the prototype checks were relaxed and it's possible for implementations of this method to return by reference. This makes indirect modifications to the overloaded array dimensions of ArrayAccess objects possible.

A direct modification is one that replaces completely the value of the array dimension, as in $obj[6] = 7. An indirect modification, on the other hand, only changes part of the dimension, or attempts to assign the dimension by reference to another variable, as in $obj[6][7] = 7 or $var =& $obj[6]. Increments with ++ and decrements with -- are also implemented in a way that requires indirect modification.

While direct modification triggers a call to ArrayAccess::offsetSet(), indirect modification triggers a call to ArrayAccess::offsetGet(). In that case, the implementation of ArrayAccess::offsetGet() must be able to return by reference, otherwise an E_NOTICE message is raised.

That means that when we do this:

$variables['attributes']['class'][] = drupal_html_class($hook);

"attributes" is our Attributes class, and "class" does not exist yet, so the following [] results in a offsetGet() on class, rather than an offsetSet(), and so offsetSet is never called and array('class' => array(...)) is not added to Attributes.

This means a couple of things. Right now we're working around some of this by predefining the class array using array('class' => array()). My patch above cleans up this empty array from being output as an attribute, but it's more of a bandaid than a fix. If there was any other reason we wanted to instantiate a different attribute using an array, we can't do that - at least not by using the $variables[$Attributes]['thing'][] = 'stuff' syntax. Which becomes really problematic for easily extending classes (or other arrays of stuff) later on.

I tried testing setting offsetGet() to return by reference as recommended in the php.net notes above, but had no success resolving the problem. This is going to require a bit more work and eyes by someone with a better deep-level understanding. This problem potentially goes beyond just the Attributes class and the problem here, I'm seeing references to similar errors in the Fields work going on here:

http://drupal.org/sandbox/yched/1736366

See Karen's note here: http://drupal.org/node/1742734#comment-6379592
And another here: http://drupal.org/node/1744216#comment-6378162

After talking to @chx I took another look at this. Returning offsetGet() by reference does resolve this - with a couple of other tweaks. When returning by reference we must return a variable or another notice is thrown. In order to get this to work we have to instantiate $name in storage if it doesn't already exist and ensure it's created as an AttributeArray class. With that being returned it can then be populated by reference when being overloaded with a multi dimensional array.

If a string or bool is passed in it is classed appropriately. Here is some basic test code I tried using drush php-script:

$d = drupal_attributes();
$variables['attributes'] = clone $d;
$variables['attributes']['class'][] = 1;
$variables['attributes']['class'][] = 2;
$variables['attributes']['class'][] = 3;
echo var_dump($variables['attributes']);

$y = drupal_attributes();
$variables['attributes'] = clone $y;
$variables['attributes']['class'] = 1;
echo var_dump($variables['attributes']);

$z = drupal_attributes();
$variables['attributes'] = clone $z;
$variables['attributes']['bool'] = (bool) FALSE; 
echo var_dump($variables['attributes']);

Patch attached should resolve this.

#8: 1717186.Empty-class-attributes.2.patch queued for re-testing.

#8: 1717186.Empty-class-attributes.2.patch queued for re-testing.

Removing extra whitespace. Not sure why the patch is failing testing, I'm able to install fine locally.

The test details show error on install. It looks like the patch applies cleanly.

Try doing a fresh install after applying the patch and see if the error occurs.

Also, I could write a Simpletest for this, if you are not already doing so.

The testbot runs 5.3.3. The manual sayeth

Starting with PHP 5.3.4, the prototype checks were relaxed and it's possible for implementations of this method to return by reference. This makes indirect modifications to the overloaded array dimensions of ArrayAccess objects possible.

Writing a test case for this one.

I've got a test for this. It will check for any instance of class="" in the markup, but I think that would be good. It is added as a new test case under the Theme tests.

^^^ The failed test is expected. The tests fail when the empty class attribute is found anywhere in the markup. My patch doesn't include either of the fixes above.

Hrm, looking at #5 / #6 I do not quite get why #5 is not the best solution if we can't get a PHP bump.

#5 is a bandaid which cures a symptom. The actual problem is that 'class' => array() is being hard coded in because otherwise you run into the issue described in #6. #8 resolves the root problem of how this is functioning.

The patch in #5 may still be warranted to clean up after others but we'll still need the php bump to really fix things with #8 and also for others encountering the same issues such as the Fields team.

We're now on PHP 5.3.5, I've combined the (updated) fix and the test.

For the test could we not do that as a unit test? Seems like it ought to be a short route to triggering the bug.

#19: empty_class_attribute-1717186-19.patch queued for re-testing.

Test from #19 is passing locally now that #1982024: Lazy-load Attribute objects later in the rendering process only if needed is in. I'm not sure if this fix is still needed or not - we've at least solved the problem reported in the OP. Thoughts, anyone?

Tagging and bumping.

Closing this for now, feel free to reopen this discussion.

We are not initializing Attribute with an empty class anymore in core.

In markup if you are separating out one attribute from the pack you should expect this behavior is bound to happen and should do as #2 mentions.
One of the following:

Always some class expected:

  <div class="content {{ attributes.class }}"{{ attributes }}>

Merge and Print to avoid space:

  <div class="{{ attributes.class|merge(['content']) }}"{{ attributes }}>

Provide a fallback if empty:

  <div class="{{ attributes.class|default('content') }}"{{ attributes }}>

And best to get the cleanest markup:
Print all attributes

  <div{{ attributes }}>