Merge pull request #4225 from neos/bugfix/4213/htmlAugmenterAttributes

BUGFIX: HtmlAugmenter & Attributes Rendering
neos · May 1, 2023 · d0a3a93 · d0a3a93
2 parents 79dd4e1 + b6c9085
commit d0a3a93
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 15 deletions.
diff --git a/Neos.Fusion/Classes/Service/HtmlAugmenter.php b/Neos.Fusion/Classes/Service/HtmlAugmenter.php
@@ -99,7 +99,7 @@ protected function mergeAttributes(\DOMNode $element, array &$newAttributes)
         /** @var $attribute \DOMAttr */
         foreach ($element->attributes as $attribute) {
             $oldAttributeValue = $attribute->hasChildNodes() ? $attribute->value : true;
-            $hasNewAttributeValue = array_key_exists($attribute->name, $newAttributes);
+            $hasNewAttributeValue = isset($newAttributes[$attribute->name]);
             $newAttributeValue = $newAttributes[$attribute->name] ?? null;
 
             if ($hasNewAttributeValue === false) {

diff --git a/Neos.Fusion/Classes/Service/RenderAttributesTrait.php b/Neos.Fusion/Classes/Service/RenderAttributesTrait.php
@@ -19,32 +19,39 @@
 trait RenderAttributesTrait
 {
     /**
-     * Render the tag attributes for the given key->values as atring,
-     * if an value is an iterable it will be concatenated with spaces as seperator
+     * Render the tag attributes for the given key->values as string,
+     * if a value is an iterable it will be concatenated with spaces as separator
      *
-     * @param iterable $attributes a
+     * @param iterable<mixed,int|string|null|bool|array<mixed,string|bool|null>> $attributes
      * @param bool $allowEmpty
      */
-    protected function renderAttributes(iterable $attributes, $allowEmpty = true): string
+    protected function renderAttributes(iterable $attributes, bool $allowEmpty = true): string
     {
         $renderedAttributes = '';
         foreach ($attributes as $attributeName => $attributeValue) {
             if ($attributeValue === null || $attributeValue === false) {
                 continue;
             }
+            if (is_array($attributeValue)) {
+                // [] => empty attribute ? questionable
+                // [true] => empty attribute
+                // [false] => empty attribute
+                // ["foo", null, false, "bar"] => "foo bar"
+                // [""] => empty attribute
+                $joinedAttributeValue = '';
+                foreach ($attributeValue as $attributeValuePart) {
+                    $joinedAttributeValue .= match (gettype($attributeValuePart)) {
+                        'boolean', 'NULL' => '',
+                        'string' => ' ' . trim($attributeValuePart),
+                        default => throw new \InvalidArgumentException('$attributes may contain values of type array<string|bool|null> type: array<' . get_debug_type($attributeValuePart) . '> given')
+                    };
+                }
+                $attributeValue = trim($joinedAttributeValue);
+            }
             $encodedAttributeName = htmlspecialchars((string)$attributeName, ENT_COMPAT, 'UTF-8', false);
             if ($attributeValue === true || $attributeValue === '') {
                 $renderedAttributes .= ' ' . $encodedAttributeName . ($allowEmpty ? '' : '=""');
             } else {
-                if (is_array($attributeValue)) {
-                    $joinedAttributeValue = '';
-                    foreach ($attributeValue as $attributeValuePart) {
-                        if ((string)$attributeValuePart !== '') {
-                            $joinedAttributeValue .= ' ' . trim($attributeValuePart);
-                        }
-                    }
-                    $attributeValue = trim($joinedAttributeValue);
-                }
                 $encodedAttributeValue = htmlspecialchars((string)$attributeValue, ENT_COMPAT, 'UTF-8', false);
                 $renderedAttributes .= ' ' . $encodedAttributeName . '="' . $encodedAttributeValue . '"';
             }

diff --git a/Neos.Fusion/Tests/Unit/Service/HtmlAugmenterTest.php b/Neos.Fusion/Tests/Unit/Service/HtmlAugmenterTest.php
@@ -342,7 +342,54 @@ public function __toString() {
                 'fallbackTagName' => null,
                 'exclusiveAttributes' => null,
                 'allowEmpty' => true,
-                'expectedResult' => '<p class="">Array attribute</p>',
+                'expectedResult' => '<p class>Array attribute</p>',
+            ],
+
+            // https://github.com/neos/neos-development-collection/issues/3582
+            'empty string rendered as empty attribute' => [
+                'html' => '<p>text</p>',
+                'attributes' => ['data-bla' => ''],
+                'fallbackTagName' => null,
+                'exclusiveAttributes' => null,
+                'allowEmpty' => true,
+                'expectedResult' => '<p data-bla>text</p>',
+            ],
+
+            // https://github.com/neos/neos-development-collection/issues/4213
+            'null should not remove existing attribute' => [
+                'html' => '<p data-bla>text</p>',
+                'attributes' => ['data-bla' => null],
+                'fallbackTagName' => null,
+                'exclusiveAttributes' => null,
+                'allowEmpty' => true,
+                'expectedResult' => '<p data-bla>text</p>',
+            ],
+
+            'apply empty string on existing empty attribute' => [
+                'html' => '<p data-bla>text</p>',
+                'attributes' => ['data-bla' => ''],
+                'fallbackTagName' => null,
+                'exclusiveAttributes' => null,
+                'allowEmpty' => true,
+                'expectedResult' => '<p data-bla>text</p>',
+            ],
+
+            'apply string on existing empty attribute' => [
+                'html' => '<p data-bla>text</p>',
+                'attributes' => ['data-bla' => 'foobar'],
+                'fallbackTagName' => null,
+                'exclusiveAttributes' => null,
+                'allowEmpty' => true,
+                'expectedResult' => '<p data-bla="foobar">text</p>',
+            ],
+
+            'false removes attribute' => [
+                'html' => '<p data-bla>text</p>',
+                'attributes' => ['data-bla' => false],
+                'fallbackTagName' => null,
+                'exclusiveAttributes' => null,
+                'allowEmpty' => true,
+                'expectedResult' => '<p>text</p>',
             ]
         ];
     }