Issue #2336355 by alexpott, Cottser: Fixed Refactor Attribute rendering so...

Issue #2336355 by alexpott, Cottser: Fixed Refactor Attribute rendering so that class= is not printed.

core/lib/Drupal/Core/Template/Attribute.php

@@ -150,13 +150,9 @@ public function addClass() {
       if (isset($this->storage['class']) && $this->storage['class'] instanceOf AttributeArray) {
         // Merge the values passed in from the class value array.
         $classes = array_merge($this->storage['class']->value(), $classes);
-        // Filter out any duplicate values.
-        $classes = array_unique($classes);
         $this->storage['class']->exchangeArray($classes);
       }
       else {
-        // Filter out any duplicate values.
-        $classes = array_unique($classes);
         $this->offsetSet('class', $classes);
       }
     }
@@ -184,8 +180,9 @@ public function removeClass() {
         $classes = array_merge($classes, (array) $arg);
       }
 
-      // Remove the values passed in from the value array.
-      $classes = array_diff($this->storage['class']->value(), $classes);
+      // Remove the values passed in from the value array. Use array_values() to
+      // ensure that the array index remains sequential.
+      $classes = array_values(array_diff($this->storage['class']->value(), $classes));
       $this->storage['class']->exchangeArray($classes);
     }
     return $this;

core/lib/Drupal/Core/Template/AttributeArray.php

@@ -29,6 +29,14 @@
  */
 class AttributeArray extends AttributeValueBase implements \ArrayAccess, \IteratorAggregate {
 
+  /**
+   * Ensures empty array as a result of array_filter will not print '$name=""'.
+   *
+   * @see \Drupal\Core\Template\AttributeArray::__toString()
+   * @see \Drupal\Core\Template\AttributeValueBase::render()
+   */
+  const RENDER_EMPTY_ATTRIBUTE = FALSE;
+
   /**
    * Implements ArrayAccess::offsetGet().
    */
@@ -67,7 +75,7 @@ public function offsetExists($offset) {
    */
   public function __toString() {
     // Filter out any empty values before printing.
-    $this->value = array_filter($this->value);
+    $this->value = array_unique(array_filter($this->value));
     return String::checkPlain(implode(' ', $this->value));
   }
 

core/lib/Drupal/Core/Template/AttributeValueBase.php

@@ -16,6 +16,13 @@
  */
 abstract class AttributeValueBase {
 
+  /**
+   * Renders '$name=""' if $value is an empty string.
+   *
+   * @see \Drupal\Core\Template\AttributeValueBase::render()
+   */
+  const RENDER_EMPTY_ATTRIBUTE = TRUE;
+
   /**
    * The value itself.
    *
@@ -48,8 +55,9 @@ public function __construct($name, $value) {
    *   The string representation of the attribute.
    */
   public function render() {
-    if (isset($this->value)) {
-      return String::checkPlain($this->name) . '="' . $this . '"';
+    $value = (string) $this;
+    if (isset($this->value) && static::RENDER_EMPTY_ATTRIBUTE || !empty($value)) {
+      return String::checkPlain($this->name) . '="' . $value . '"';
     }
   }
 

core/tests/Drupal/Tests/Core/Template/AttributeTest.php

@@ -69,6 +69,25 @@ public function testAddClasses() {
     $attribute->addClass();
     $this->assertEmpty($attribute['class']);
 
+    // Test various permutations of adding values to empty Attribute objects.
+    foreach (array(NULL, FALSE, '', []) as $value) {
+      // Single value.
+      $attribute->addClass($value);
+      $this->assertEmpty((string) $attribute);
+
+      // Multiple values.
+      $attribute->addClass($value, $value);
+      $this->assertEmpty((string) $attribute);
+
+      // Single value in array.
+      $attribute->addClass([$value]);
+      $this->assertEmpty((string) $attribute);
+
+      // Single value in arrays.
+      $attribute->addClass([$value], [$value]);
+      $this->assertEmpty((string) $attribute);
+    }
+
     // Add one class on empty attribute.
     $attribute->addClass('banana');
     $this->assertArrayEquals(array('banana'), $attribute['class']->value());
@@ -87,7 +106,7 @@ public function testAddClasses() {
 
     // Add an array of duplicate classes.
     $attribute->addClass(array('red', 'green', 'blue'), array('aa', 'aa', 'banana'), 'yy');
-    $this->assertArrayEquals(array('banana', 'aa', 'xx', 'yy', 'red', 'green', 'blue'), $attribute['class']->value());
+    $this->assertEquals('banana aa xx yy red green blue', (string) $attribute['class']);
   }
 
   /**
@@ -95,7 +114,8 @@ public function testAddClasses() {
    * @covers ::removeClass()
    */
   public function testRemoveClasses() {
-    $classes = array('example-class', 'aa', 'xx', 'yy', 'red', 'green', 'blue');
+    // Add duplicate class to ensure that both duplicates are removed.
+    $classes = array('example-class', 'aa', 'xx', 'yy', 'red', 'green', 'blue', 'red');
     $attribute = new Attribute(array('class' => $classes));
 
     // Remove one class.
@@ -109,6 +129,15 @@ public function testRemoveClasses() {
     // Remove an array of classes.
     $attribute->removeClass(array('red', 'green', 'blue'));
     $this->assertNotContains(array('red', 'green', 'blue'), $attribute['class']->value());
+
+    // Remove a class that does not exist.
+    $attribute->removeClass('gg');
+    $this->assertNotContains(array('gg'), $attribute['class']->value());
+    // Test that the array index remains sequential.
+    $this->assertArrayEquals(array('aa'), $attribute['class']->value());
+
+    $attribute->removeClass('aa');
+    $this->assertEmpty((string) $attribute);
   }
 
   /**