Merge pull request #52 from Icinga/fix/wrapped-element-within-container

Fix infinite loop when a wrapper wraps the element in an extra container

src/HtmlDocument.php

@@ -391,7 +391,7 @@ protected function renderWrapped()
         $wrapper = $this->wrapper;
 
         if (isset($this->renderedBy)) {
-            if ($wrapper === $this->renderedBy) {
+            if ($wrapper === $this->renderedBy || $wrapper->contains($this->renderedBy)) {
                 // $this might be an intermediate wrapper that's already about to be rendered.
                 // In case of an element (referencing $this as a wrapper) that is a child of an
                 // outer wrapper, it is required to ignore $wrapper as otherwise it's a loop.

tests/FormElementDecoratorTest.php

@@ -0,0 +1,64 @@
+<?php
+
+namespace ipl\Tests\Html;
+
+use ipl\Html\Form;
+use ipl\Tests\Html\TestDummy\PositionedFormElementDecorator;
+use ipl\Tests\Html\TestDummy\SimpleFormElementDecorator;
+use ipl\Tests\Html\TestDummy\WithinContainerFormElementDecorator;
+
+class FormElementDecoratorTest extends TestCase
+{
+    public function testPositionedFormElementDecorator()
+    {
+        $form = (new Form())
+            ->setDefaultElementDecorator(new PositionedFormElementDecorator())
+            ->addElement('text', 'decorated-form-element');
+
+        $html = <<<'HTML'
+<form method="POST">
+  <div class="positioned-decorator">
+    <input type="text" name="decorated-form-element">
+  </div>
+</form>
+HTML;
+
+        $this->assertHtml($html, $form);
+    }
+
+    public function testSimpleFormElementDecorator()
+    {
+        $form = (new Form())
+            ->setDefaultElementDecorator(new SimpleFormElementDecorator())
+            ->addElement('text', 'decorated-form-element');
+
+        $html = <<<'HTML'
+<form method="POST">
+  <div class="simple-decorator">
+    <input type="text" name="decorated-form-element">
+  </div>
+</form>
+HTML;
+
+        $this->assertHtml($html, $form);
+    }
+
+    public function testWithinContainerFormElementDecorator()
+    {
+        $form = (new Form())
+            ->setDefaultElementDecorator(new WithinContainerFormElementDecorator())
+            ->addElement('text', 'decorated-form-element');
+
+        $html = <<<'HTML'
+<form method="POST">
+  <div class="within-container-decorator">
+    <div class="container">
+      <input type="text" name="decorated-form-element">
+    </div>
+  </div>
+</form>
+HTML;
+
+        $this->assertHtml($html, $form);
+    }
+}

tests/HtmlDocumentTest.php

@@ -154,6 +154,28 @@ public function testWrapperReuseWorks()
         );
     }
 
+    public function testWrapperCanPositionWrappedElement()
+    {
+        $a = h::tag('tag', 'content');
+        $a->addWrapper(h::tag('div', ['class' => 'wrapper'], $a));
+
+        $this->assertHtml(
+            '<div class="wrapper"><tag>content</tag></div>',
+            $a
+        );
+    }
+
+    public function testWrapperCanPositionWrappedElementInAnExtraContainer()
+    {
+        $a = h::tag('tag', 'content');
+        $a->addWrapper(h::tag('div', ['class' => 'outer'], h::tag('div', ['class' => 'inner'], $a)));
+
+        $this->assertHtml(
+            '<div class="outer"><div class="inner"><tag>content</tag></div></div>',
+            $a
+        );
+    }
+
     public function testAcceptsObjectsWhichCanBeCastedToString()
     {
         $object = new ObjectThatCanBeCastedToString();

tests/TestCase.php

@@ -10,6 +10,7 @@
     // Support older PHPUnit versions
     class_alias('PHPUnit_Util_XML', 'PHPUnit\\Util\\Xml');
 }
+
 // phpcs:enable
 
 abstract class TestCase extends \PHPUnit\Framework\TestCase
@@ -22,6 +23,12 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase
      */
     protected function assertHtml($expectedHtml, ValidHtml $actual)
     {
+        $expectedHtml = str_replace(
+            "\n",
+            '',
+            preg_replace('/^\s+/m', '', trim($expectedHtml))
+        );
+
         if (method_exists(Xml::class, 'load')) {
             $expectedHtml = Xml::load($expectedHtml, true);
             $actualHtml = Xml::load($actual->render(), true);

tests/TestDummy/PositionedFormElementDecorator.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace ipl\Tests\Html\TestDummy;
+
+use ipl\Html\BaseHtmlElement;
+use ipl\Html\Contract\FormElement;
+use ipl\Html\Contract\FormElementDecorator;
+
+class PositionedFormElementDecorator extends BaseHtmlElement implements FormElementDecorator
+{
+    protected $tag = 'div';
+
+    protected $defaultAttributes = ['class' => 'positioned-decorator'];
+
+    /** @var FormElement */
+    protected $formElement;
+
+    public function decorate(FormElement $formElement)
+    {
+        $decorator = new static();
+        $decorator->formElement = $formElement;
+
+        $formElement->prependWrapper($decorator);
+
+        return $decorator;
+    }
+
+    protected function assemble()
+    {
+        $this->add($this->formElement);
+    }
+}

tests/TestDummy/SimpleFormElementDecorator.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace ipl\Tests\Html\TestDummy;
+
+use ipl\Html\BaseHtmlElement;
+use ipl\Html\Contract\FormElement;
+use ipl\Html\Contract\FormElementDecorator;
+
+class SimpleFormElementDecorator extends BaseHtmlElement implements FormElementDecorator
+{
+    protected $tag = 'div';
+
+    protected $defaultAttributes = ['class' => 'simple-decorator'];
+
+    /** @var FormElement */
+    protected $formElement;
+
+    public function decorate(FormElement $formElement)
+    {
+        $decorator = new static();
+        $decorator->formElement = $formElement;
+
+        $formElement->prependWrapper($decorator);
+
+        return $decorator;
+    }
+}

tests/TestDummy/WithinContainerFormElementDecorator.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace ipl\Tests\Html\TestDummy;
+
+use ipl\Html\BaseHtmlElement;
+use ipl\Html\Contract\FormElement;
+use ipl\Html\Contract\FormElementDecorator;
+use ipl\Html\Html;
+
+class WithinContainerFormElementDecorator extends BaseHtmlElement implements FormElementDecorator
+{
+    protected $tag = 'div';
+
+    protected $defaultAttributes = ['class' => 'within-container-decorator'];
+
+    /** @var FormElement */
+    protected $formElement;
+
+    public function decorate(FormElement $formElement)
+    {
+        $decorator = new static();
+        $decorator->formElement = $formElement;
+
+        $formElement->prependWrapper($decorator);
+
+        return $decorator;
+    }
+
+    protected function assemble()
+    {
+        $this->add(Html::tag('div', ['class' => 'container'], $this->formElement));
+    }
+}