merged branch bschussek/issue4615 (PR #4799)

Commits
-------

df5bb4a [Form] Unified rendering of errors for nested elements

Discussion
----------

[Form] Unified rendering of errors for nested elements

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes?
Symfony2 tests pass: yes
Fixes the following tickets: #4615
Todo: -

src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

@@ -20,6 +20,9 @@
 {% block form_widget_compound %}
 {% spaceless %}
     <div {{ block('widget_container_attributes') }}>
+        {% if form.parent is empty %}
+            {{ form_errors(form) }}
+        {% endif %}
         {{ block('form_rows') }}
         {{ form_rest(form) }}
     </div>
@@ -236,6 +239,10 @@
 
 {% block repeated_row %}
 {% spaceless %}
+    {#
+    No need to render the errors here, as all errors are mapped
+    to the first child (see RepeatedTypeValidatorExtension).
+    #}
     {{ block('form_rows') }}
 {% endspaceless %}
 {% endblock repeated_row %}
@@ -244,13 +251,7 @@
 {% spaceless %}
     <div>
         {{ form_label(form, label|default(null)) }}
-        {#
-        If the child is a compound form, the errors are rendered inside
-        the container. See also block form_rows.
-        #}
-        {% if not compound %}
-            {{ form_errors(form) }}
-        {% endif %}
+        {{ form_errors(form) }}
         {{ form_widget(form) }}
     </div>
 {% endspaceless %}
@@ -298,7 +299,6 @@
 
 {% block form_rows %}
 {% spaceless %}
-    {{ form_errors(form) }}
     {% for child in form %}
         {{ form_row(child) }}
     {% endfor %}

src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig

@@ -7,31 +7,13 @@
             {{ form_label(form, label|default(null)) }}
         </td>
         <td>
-            {% if not compound %}
-                {{ form_errors(form) }}
-            {% endif %}
+            {{ form_errors(form) }}
             {{ form_widget(form) }}
         </td>
     </tr>
 {% endspaceless %}
 {% endblock form_row %}
 
-{% block form_errors %}
-{% spaceless %}
-    {% if not compound %}
-        {{ parent() }}
-    {% else %}
-        {% if errors|length > 0 %}
-        <tr>
-            <td colspan="2">
-                {{ parent() }}
-            </td>
-        </tr>
-        {% endif %}
-    {% endif %}
-{% endspaceless %}
-{% endblock form_errors %}
-
 {% block hidden_row %}
 {% spaceless %}
     <tr style="display: none">
@@ -45,6 +27,13 @@
 {% block form_widget_compound %}
 {% spaceless %}
     <table {{ block('widget_container_attributes') }}>
+        {% if form.parent is empty and errors|length > 0 %}
+        <tr>
+            <td colspan="2">
+                {{ form_errors(form) }}
+            </td>
+        </tr>
+        {% endif %}
         {{ block('form_rows') }}
         {{ form_rest(form) }}
     </table>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php

@@ -1,7 +1,5 @@
 <div>
     <?php echo $view['form']->label($form, isset($label) ? $label : null) ?>
-    <?php if (!$compound): ?>
-        <?php echo $view['form']->errors($form) ?>
-    <?php endif ?>
+    <?php echo $view['form']->errors($form) ?>
     <?php echo $view['form']->widget($form) ?>
 </div>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_rows.html.php

@@ -1,4 +1,3 @@
-<?php echo $view['form']->errors($form) ?>
 <?php foreach ($form as $child) : ?>
     <?php echo $view['form']->row($child) ?>
 <?php endforeach; ?>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_widget_compound.html.php

@@ -1,4 +1,11 @@
 <div <?php echo $view['form']->renderBlock('widget_container_attributes') ?>>
+    <?php if (!$form->hasParent() && $errors): ?>
+    <tr>
+        <td colspan="2">
+            <?php echo $view['form']->errors($form) ?>
+        </td>
+    </tr>
+    <?php endif ?>
     <?php echo $view['form']->renderBlock('form_rows') ?>
     <?php echo $view['form']->rest($form) ?>
 </div>

src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_errors.html.php

@@ -1,51 +1,21 @@
-<?php if (!$compound): ?>
-    <?php if ($errors): ?>
-        <ul>
-            <?php foreach ($errors as $error): ?>
-                <li><?php
-                    if (null === $error->getMessagePluralization()) {
-                        echo $view['translator']->trans(
-                            $error->getMessageTemplate(),
-                            $error->getMessageParameters(),
-                            'validators'
-                        );
-                    } else {
-                        echo $view['translator']->transChoice(
-                            $error->getMessageTemplate(),
-                            $error->getMessagePluralization(),
-                            $error->getMessageParameters(),
-                            'validators'
-                        );
-                    }?></li>
-            <?php endforeach; ?>
-        </ul>
-    <?php endif ?>
-<?php else: ?>
-    <?php if (count($errors) > 0): ?>
-    <tr>
-        <td colspan="2">
-        <?php if ($errors): ?>
-            <ul>
-                <?php foreach ($errors as $error): ?>
-                    <li><?php
-                        if (null === $error->getMessagePluralization()) {
-                            echo $view['translator']->trans(
-                                $error->getMessageTemplate(),
-                                $error->getMessageParameters(),
-                                'validators'
-                            );
-                        } else {
-                            echo $view['translator']->transChoice(
-                                $error->getMessageTemplate(),
-                                $error->getMessagePluralization(),
-                                $error->getMessageParameters(),
-                                'validators'
-                            );
-                        }?></li>
-                <?php endforeach; ?>
-            </ul>
-        <?php endif ?>
-        </td>
-    </tr>
-    <?php endif; ?>
-<?php endif;
+<?php if ($errors): ?>
+    <ul>
+        <?php foreach ($errors as $error): ?>
+            <li><?php
+                if (null === $error->getMessagePluralization()) {
+                    echo $view['translator']->trans(
+                        $error->getMessageTemplate(),
+                        $error->getMessageParameters(),
+                        'validators'
+                    );
+                } else {
+                    echo $view['translator']->transChoice(
+                        $error->getMessageTemplate(),
+                        $error->getMessagePluralization(),
+                        $error->getMessageParameters(),
+                        'validators'
+                    );
+                }?></li>
+        <?php endforeach; ?>
+    </ul>
+<?php endif ?>

src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php

@@ -3,9 +3,7 @@
         <?php echo $view['form']->label($form, isset($label) ? $label : null) ?>
     </td>
     <td>
-        <?php if (!$compound): ?>
-            <?php echo $view['form']->errors($form) ?>
-        <?php endif ?>
+        <?php echo $view['form']->errors($form) ?>
         <?php echo $view['form']->widget($form) ?>
     </td>
 </tr>

src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_widget_compound.html.php

@@ -1,4 +1,7 @@
 <table <?php echo $view['form']->renderBlock('widget_container_attributes') ?>>
+    <?php if (!$form->hasParent()): ?>
+    <?php echo $view['form']->errors($form) ?>
+    <?php endif ?>
     <?php echo $view['form']->renderBlock('form_rows') ?>
     <?php echo $view['form']->rest($form) ?>
 </table>

src/Symfony/Component/Form/CHANGELOG.md

@@ -143,3 +143,4 @@ CHANGELOG
  * FormBuilder now maintains the order when explicitely adding form builders as children
  * ChoiceType now doesn't add the empty value anymore if the choices already contain an empty element
  * DateType, TimeType and DateTimeType now show empty values again if not required
+ * [BC BREAK] fixed rendering of errors for DateType, BirthdayType and similar ones

src/Symfony/Component/Form/Extension/Core/Type/FormType.php

@@ -175,6 +175,11 @@ class FormType extends AbstractType
             return false !== $options['property_path'];
         };
 
+        // Compound forms are not displayed inline
+        $inline = function (Options $options) {
+            return !$options['compound'];
+        };
+
         $resolver->setDefaults(array(
             'data'               => null,
             'data_class'         => $dataClass,
@@ -195,6 +200,7 @@ class FormType extends AbstractType
             'virtual'            => false,
             'compound'           => true,
             'translation_domain' => null,
+            'inline'             => $inline,
         ));
 
         $resolver->setAllowedTypes(array(

src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php

@@ -57,11 +57,13 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest
         $view = $form->createView();
         $html = $this->renderRow($view);
 
+        // The errors of the form are not rendered by intention!
+        // In practice, repeated fields cannot have errors as all errors
+        // on them are mapped to the first child.
+        // (see RepeatedTypeValidatorExtension)
+
         $this->assertMatchesXpath($html,
-'/ul
-    [./li[.="[trans]Error![/trans]"]]
-    [count(./li)=1]
-/following-sibling::div
+'/div
     [
         ./label[@for="name_first"]
         /following-sibling::input[@id="name_first"]
@@ -373,7 +375,8 @@ abstract class AbstractDivLayoutTest extends AbstractLayoutTest
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/div
     [
-        ./div/div[@id="name_child"][./ul/li[.="[trans]Error![/trans]"]]
+        ./div/label
+        /following-sibling::ul[./li[.="[trans]Error![/trans]"]]
     ]
     [count(.//li[.="[trans]Error![/trans]"])=1]
 '

src/Symfony/Component/Form/Tests/AbstractTableLayoutTest.php

@@ -59,12 +59,12 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest
         /following-sibling::td
             [./input[@id="name_second"]]
     ]
-    [count(../tr)=3]
 /following-sibling::tr[@style="display: none"]
     [./td[@colspan="2"]/input
         [@type="hidden"]
         [@id="name__token"]
     ]
+    [count(../tr)=3]
 '
         );
     }
@@ -76,12 +76,13 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest
         $view = $form->createView();
         $html = $this->renderRow($view);
 
+        // The errors of the form are not rendered by intention!
+        // In practice, repeated fields cannot have errors as all errors
+        // on them are mapped to the first child.
+        // (see RepeatedTypeValidatorExtension)
+
         $this->assertMatchesXpath($html,
 '/tr
-    [./td[@colspan="2"]/ul
-        [./li[.="[trans]Error![/trans]"]]
-    ]
-/following-sibling::tr
     [
         ./td
             [./label[@for="name_first"]]
@@ -95,12 +96,12 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest
         /following-sibling::td
             [./input[@id="name_second"]]
     ]
-    [count(../tr)=4]
 /following-sibling::tr[@style="display: none"]
     [./td[@colspan="2"]/input
         [@type="hidden"]
         [@id="name__token"]
     ]
+    [count(../tr)=3]
 '
         );
     }
@@ -235,9 +236,8 @@ abstract class AbstractTableLayoutTest extends AbstractLayoutTest
         $this->assertWidgetMatchesXpath($form->createView(), array(),
 '/table
     [
-        ./tr/td/table
-            [@id="name_child"]
-            [./tr/td/ul/li[.="[trans]Error![/trans]"]]
+        ./tr/td/ul[./li[.="[trans]Error![/trans]"]]
+        /following-sibling::table[@id="name_child"]
     ]
     [count(.//li[.="[trans]Error![/trans]"])=1]
 '