[FEATURE] Support for boolean tag attributes (#880)

TagBasedViewHelpers now have proper support for boolean attributes. Before this change, it was very cumbersome to generate these with Fluid, now it's implemented similar to popular JavaScript frameworks: ``` <my:viewhelper async="{true}" /> Result: <tag async="async" /> <my:viewhelper async="{false}" /> Result: <tag /> ``` This can also be used in combination with variable casting: ``` <my:viewhelper async="{myString as boolean}" /> ``` For compatibility reasons empty strings still lead to the attribute being omitted from the tag. This might change in the future, however we don't want to break templates now. ViewHelpers can define an argument manually and implement the desired behavior in the render method (this is how the ImageViewHelper in TYPO3 deals with empty `alt` attributes).
TYPO3 · Aug 9, 2024 · 4d5b0c9 · 4d5b0c9
1 parent ba2c48b
commit 4d5b0c9
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 34 deletions.
diff --git a/src/Core/ViewHelper/AbstractTagBasedViewHelper.php b/src/Core/ViewHelper/AbstractTagBasedViewHelper.php
@@ -116,8 +116,11 @@ public function initialize()
         }
 
         foreach ($this->additionalArguments as $argumentName => $argumentValue) {
+            // This condition is left here for compatibility reasons. Removing this will be a breaking change
+            // because TagBuilder renders empty strings as empty attributes (as it should be). We might remove
+            // this condition in the future to have a clean solution.
             if ($argumentValue !== null && $argumentValue !== '') {
-                $this->tag->addAttribute($argumentName, (string)$argumentValue);
+                $this->tag->addAttribute($argumentName, $argumentValue);
             }
         }
 

diff --git a/src/Core/ViewHelper/TagBuilder.php b/src/Core/ViewHelper/TagBuilder.php
@@ -170,7 +170,7 @@ public function ignoreEmptyAttributes(bool $ignoreEmptyAttributes): void
      * Adds an attribute to the $attributes-collection
      *
      * @param string $attributeName name of the attribute to be added to the tag
-     * @param string|\Traversable|array|null $attributeValue attribute value, can only be array or traversable
+     * @param string|bool|\Traversable|array|null $attributeValue attribute value, can only be array or traversable
      *                                                       if the attribute name is either "data" or "area". In
      *                                                       that special case, multiple attributes will be created
      *                                                       with either "data-" or "area-" as prefix
@@ -194,6 +194,16 @@ public function addAttribute(string $attributeName, $attributeValue, bool $escap
                 $this->addAttribute($attributeName . '-' . $name, $value, $escapeSpecialCharacters);
             }
         } else {
+            // This should probably also check for null, but we can't do that for now because of backwards compatibility
+            if ($attributeValue === false) {
+                $this->removeAttribute($attributeName);
+                return;
+            }
+
+            if ($attributeValue === true) {
+                $attributeValue = $attributeName;
+            }
+
             if (trim((string)$attributeValue) === '' && $this->ignoreEmptyAttributes) {
                 return;
             }

diff --git a/tests/Functional/Cases/TagBasedTest.php b/tests/Functional/Cases/TagBasedTest.php
@@ -25,67 +25,136 @@ public static function renderTagBasedViewHelperDataProvider(): array
                 [],
                 '<div />',
             ],
-            'empty registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="" />',
-                "{test:tagBasedTest(registeredTagAttribute: '')}",
+
+            // Arguments that are explicitly defined with type boolean
+            // still retain the original boolean behavior:
+            // string input is interpreted in a way that "true" equals true
+            // and "false" equals false
+            'string as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="test" />',
+                "{test:tagBasedTest(registeredBooleanArgument: 'test')}",
+                [],
+                '<div registeredBooleanArgument="registeredBooleanArgument" />',
+            ],
+            'string true as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="true" />',
+                "{test:tagBasedTest(registeredBooleanArgument: 'true')}",
+                [],
+                '<div registeredBooleanArgument="registeredBooleanArgument" />',
+            ],
+            'string false as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="false" />',
+                "{test:tagBasedTest(registeredBooleanArgument: 'false')}",
+                [],
+                '<div />',
+            ],
+            'string null as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="null" />',
+                "{test:tagBasedTest(registeredBooleanArgument: 'null')}",
+                [],
+                '<div registeredBooleanArgument="registeredBooleanArgument" />', // @todo this should probably behave differently
+            ],
+            'empty registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="" />',
+                "{test:tagBasedTest(registeredBooleanArgument: '')}",
                 [],
                 '<div />',
             ],
-            'true as registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="{var}" />',
-                '{test:tagBasedTest(registeredTagAttribute: var)}',
+            'variable with true as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{var}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: var)}',
                 ['var' => true],
-                '<div registeredTagAttribute="1" />',
+                '<div registeredBooleanArgument="registeredBooleanArgument" />',
             ],
-            'false as registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="{var}" />',
-                '{test:tagBasedTest(registeredTagAttribute: var)}',
+            'variable with false as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{var}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: var)}',
                 ['var' => false],
-                '<div registeredTagAttribute="" />',
+                '<div />',
             ],
-            'null as registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="{var}" />',
-                '{test:tagBasedTest(registeredTagAttribute: var)}',
+            'variable with null as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{var}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: var)}',
                 ['var' => null],
                 '<div />',
             ],
-            'undefined variable as registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="{var}" />',
-                '{test:tagBasedTest(registeredTagAttribute: var)}',
+            'undefined variable as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{var}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: var)}',
+                [],
+                '<div />',
+            ],
+            'casted variable as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{var as boolean}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: \'{var as boolean}\')}',
+                ['var' => '0'],
+                '<div />',
+            ],
+            'boolean literal true as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{true}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: true)}',
+                [],
+                '<div registeredBooleanArgument="registeredBooleanArgument" />',
+            ],
+            'boolean literal false as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{false}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: false)}',
                 [],
                 '<div />',
             ],
-            'registered tag attribute' => [
-                '<test:tagBasedTest registeredTagAttribute="test" />',
-                "{test:tagBasedTest(registeredTagAttribute: 'test')}",
+            'null literal as registered bool attribute' => [
+                '<test:tagBasedTest registeredBooleanArgument="{null}" />',
+                '{test:tagBasedTest(registeredBooleanArgument: null)}',
                 [],
-                '<div registeredTagAttribute="test" />',
+                '<div />',
             ],
-            'unregistered argument' => [
+
+            // Unregistered ViewHelper arguments take strings as-is. To
+            // create a boolean argument, the passed value needs to have
+            // the correct type, either boolean or null
+            'string as unregistered argument' => [
                 '<test:tagBasedTest foo="bar" />',
                 "{test:tagBasedTest(foo: 'bar')}",
                 [],
                 '<div foo="bar" />',
             ],
+            'string true as unregistered argument' => [
+                '<test:tagBasedTest foo="true" />',
+                "{test:tagBasedTest(foo: 'true')}",
+                [],
+                '<div foo="true" />',
+            ],
+            'string false as unregistered argument' => [
+                '<test:tagBasedTest foo="false" />',
+                "{test:tagBasedTest(foo: 'false')}",
+                [],
+                '<div foo="false" />',
+            ],
+            'string null as unregistered argument' => [
+                '<test:tagBasedTest foo="null" />',
+                "{test:tagBasedTest(foo: 'null')}",
+                [],
+                '<div foo="null" />',
+            ],
             'empty unregistered argument' => [
                 '<test:tagBasedTest foo="" />',
                 "{test:tagBasedTest(foo: '')}",
                 [],
-                '<div />',
+                '<div />', // @todo this should render an empty attribute, however this would be a breaking change in templates
             ],
-            'true as unregistered argument' => [
+            'variable with true as unregistered argument' => [
                 '<test:tagBasedTest foo="{var}" />',
                 '{test:tagBasedTest(foo: var)}',
                 ['var' => true],
-                '<div foo="1" />',
+                '<div foo="foo" />',
             ],
-            'false as unregistered argument' => [
+            'variable with false as unregistered argument' => [
                 '<test:tagBasedTest foo="{var}" />',
                 '{test:tagBasedTest(foo: var)}',
                 ['var' => false],
-                '<div foo="" />',
+                '<div />',
             ],
-            'null as unregistered argument' => [
+            'variable with null as unregistered argument' => [
                 '<test:tagBasedTest foo="{var}" />',
                 '{test:tagBasedTest(foo: var)}',
                 ['var' => null],
@@ -97,6 +166,31 @@ public static function renderTagBasedViewHelperDataProvider(): array
                 [],
                 '<div />',
             ],
+            'casted variable as unregistered argument' => [
+                '<test:tagBasedTest foo="{var as boolean}" />',
+                '{test:tagBasedTest(foo: \'{var as boolean}\')}',
+                ['var' => '0'],
+                '<div />',
+            ],
+            'boolean literal true as unregistered argument' => [
+                '<test:tagBasedTest async="{true}" />',
+                '{test:tagBasedTest(async: true)}',
+                [],
+                '<div async="async" />',
+            ],
+            'boolean literal false as unregistered argument' => [
+                '<test:tagBasedTest async="{false}" />',
+                '{test:tagBasedTest(async: false)}',
+                [],
+                '<div />',
+            ],
+            'null literal as unregistered argument' => [
+                '<test:tagBasedTest async="{null}" />',
+                '{test:tagBasedTest(async: null)}',
+                [],
+                '<div />',
+            ],
+
             'data array' => [
                 '<test:tagBasedTest data="{foo: \'bar\', more: 1}" />',
                 '{test:tagBasedTest(data: {foo: \'bar\', more: 1})}',
@@ -176,15 +270,15 @@ public function renderTagBasedViewHelper(string $source, string $sourceInline, a
         $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source);
         $view->getRenderingContext()->getViewHelperResolver()->addNamespace('test', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\Fixtures\\ViewHelpers');
         $output = $view->render();
-        self::assertEquals($expected, $output);
+        self::assertEquals($expected, $output, 'tag variant uncached');
 
         $view = new TemplateView();
         $view->assignMultiple($variables);
         $view->getRenderingContext()->setCache(self::$cache);
         $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($sourceInline);
         $view->getRenderingContext()->getViewHelperResolver()->addNamespace('test', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\Fixtures\\ViewHelpers');
         $output = $view->render();
-        self::assertEquals($expected, $output);
+        self::assertEquals($expected, $output, 'inline variant uncached');
 
         // Second run to test cached template parsing
         $view = new TemplateView();
@@ -193,15 +287,15 @@ public function renderTagBasedViewHelper(string $source, string $sourceInline, a
         $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source);
         $view->getRenderingContext()->getViewHelperResolver()->addNamespace('test', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\Fixtures\\ViewHelpers');
         $output = $view->render();
-        self::assertEquals($expected, $output);
+        self::assertEquals($expected, $output, 'tag variant cached');
 
         $view = new TemplateView();
         $view->assignMultiple($variables);
         $view->getRenderingContext()->setCache(self::$cache);
         $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($sourceInline);
         $view->getRenderingContext()->getViewHelperResolver()->addNamespace('test', 'TYPO3Fluid\\Fluid\\Tests\\Functional\\Fixtures\\ViewHelpers');
         $output = $view->render();
-        self::assertEquals($expected, $output);
+        self::assertEquals($expected, $output, 'inline variant cached');
     }
 
     public static function throwsErrorForInvalidArgumentTypesDatProvider(): array

diff --git a/tests/Functional/Fixtures/ViewHelpers/TagBasedTestViewHelper.php b/tests/Functional/Fixtures/ViewHelpers/TagBasedTestViewHelper.php
@@ -17,5 +17,12 @@ public function initializeArguments(): void
     {
         parent::initializeArguments();
         $this->registerArgument('registeredArgument', 'string', 'test argument');
+        $this->registerArgument('registeredBooleanArgument', 'boolean', 'boolean argument', false, false);
+    }
+
+    public function render(): string
+    {
+        $this->tag->addAttribute('registeredBooleanArgument', $this->arguments['registeredBooleanArgument']);
+        return $this->tag->render();
     }
 }
diff --git a/tests/Unit/Core/ViewHelper/TagBuilderTest.php b/tests/Unit/Core/ViewHelper/TagBuilderTest.php
@@ -9,6 +9,7 @@
 
 namespace TYPO3Fluid\Fluid\Tests\Unit\Core\ViewHelper;
 
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\TestCase;
 use TYPO3Fluid\Fluid\Core\ViewHelper\TagBuilder;
@@ -242,4 +243,26 @@ public function tagIsNotRenderedIfTagNameIsEmpty(): void
         $tagBuilder->setTagName('');
         self::assertEquals('', $tagBuilder->render());
     }
+
+    public static function handlesBooleanAttributesCorrectlyDataProvider(): array
+    {
+        return [
+            'value false' => [false, '<foo />'],
+            'value true' => [true, '<foo async="async" />'],
+            'value null' => [null, '<foo async="" />'],
+            'string false' => ['false', '<foo async="false" />'],
+            'string true' => ['true', '<foo async="true" />'],
+            'string null' => ['null', '<foo async="null" />'],
+            'atttribute name' => ['async', '<foo async="async" />'],
+        ];
+    }
+
+    #[DataProvider('handlesBooleanAttributesCorrectlyDataProvider')]
+    #[Test]
+    public function handlesBooleanAttributesCorrectly(mixed $attributeValue, string $expected): void
+    {
+        $tagBuilder = new TagBuilder('foo');
+        $tagBuilder->addAttribute('async', $attributeValue);
+        self::assertEquals($expected, $tagBuilder->render());
+    }
 }