[TwigBridge] Set Form ID on form element, prevent duplicate form elem…

…ent attributes
symfony · Jul 26, 2023 · fe12153 · fe12153
1 parent ad72245
commit fe12153
Show file tree

Hide file tree

Showing 15 changed files with 45 additions and 41 deletions.
diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig
@@ -1,7 +1,7 @@
 {% use "bootstrap_3_layout.html.twig" %}
 
 {% block form_start -%}
-    {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-horizontal')|trim}) %}
+    {% set row_attr = row_attr|merge({class: (row_attr.class|default('') ~ ' form-horizontal')|trim}) %}
     {{- parent() -}}
 {%- endblock form_start %}
 

diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
@@ -400,7 +400,7 @@
     {%- else -%}
         {% set form_method = "POST" %}
     {%- endif -%}
-    <form{% if name != '' %} name="{{ name }}"{% endif %} method="{{ form_method|lower }}"{% if action != '' %} action="{{ action }}"{% endif %}{{ block('attributes') }}{% if multipart %} enctype="multipart/form-data"{% endif %}>
+    <form{% if name != '' %} name="{{ name }}"{% endif %} method="{{ form_method|lower }}"{% if action != '' %} action="{{ action }}"{% endif %}{% with { attr: row_attr} %}{{ block('attributes') }}{% endwith %}{% if multipart %} enctype="multipart/form-data"{% endif %}>
     {%- if form_method != method -%}
         <input type="hidden" name="_method" value="{{ method }}" />
     {%- endif -%}

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3HorizontalLayoutTestCase.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3HorizontalLayoutTestCase.php
@@ -142,7 +142,7 @@ public function testStartTag()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="form-horizontal">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" class="form-horizontal">', $html);
     }
 
     public function testStartTagWithOverriddenVars()
@@ -157,7 +157,7 @@ public function testStartTagWithOverriddenVars()
             'action' => 'http://foo.com/directory',
         ]);
 
-        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory" class="form-horizontal">', $html);
+        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form_form" class="form-horizontal">', $html);
     }
 
     public function testStartTagForMultipartForm()
@@ -171,7 +171,7 @@ public function testStartTagForMultipartForm()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="form-horizontal" enctype="multipart/form-data">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" class="form-horizontal" enctype="multipart/form-data">', $html);
     }
 
     public function testStartTagWithExtraAttributes()
@@ -182,7 +182,7 @@ public function testStartTagWithExtraAttributes()
         ]);
 
         $html = $this->renderStart($form->createView(), [
-            'attr' => ['class' => 'foobar'],
+            'row_attr' => ['class' => 'foobar'],
         ]);
 
         $this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar form-horizontal">', $html);

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap4HorizontalLayoutTestCase.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap4HorizontalLayoutTestCase.php
@@ -193,7 +193,7 @@ public function testStartTag()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form">', $html);
     }
 
     public function testStartTagWithOverriddenVars()
@@ -208,7 +208,7 @@ public function testStartTagWithOverriddenVars()
             'action' => 'http://foo.com/directory',
         ]);
 
-        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory">', $html);
+        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form_form">', $html);
     }
 
     public function testStartTagForMultipartForm()
@@ -222,7 +222,7 @@ public function testStartTagForMultipartForm()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" enctype="multipart/form-data">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" enctype="multipart/form-data">', $html);
     }
 
     public function testStartTagWithExtraAttributes()
@@ -233,7 +233,7 @@ public function testStartTagWithExtraAttributes()
         ]);
 
         $html = $this->renderStart($form->createView(), [
-            'attr' => ['class' => 'foobar'],
+            'row_attr' => ['class' => 'foobar'],
         ]);
 
         $this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar">', $html);

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractDivLayoutTestCase.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractDivLayoutTestCase.php
@@ -414,7 +414,7 @@ public function testForm()
     ]
     [@method="post"]
     [@action="http://example.com"]
-    [@class="my&class"]
+    [@id="form_name"]
 '
         );
     }

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractLayoutTestCase.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractLayoutTestCase.php
@@ -2437,7 +2437,7 @@ public function testStartTag()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form">', $html);
     }
 
     public function testStartTagForPutRequest()
@@ -2469,7 +2469,7 @@ public function testStartTagWithOverriddenVars()
             'action' => 'http://foo.com/directory',
         ]);
 
-        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory">', $html);
+        $this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form_form">', $html);
     }
 
     public function testStartTagForMultipartForm()
@@ -2483,7 +2483,7 @@ public function testStartTagForMultipartForm()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" enctype="multipart/form-data">', $html);
+        $this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" enctype="multipart/form-data">', $html);
     }
 
     public function testStartTagWithExtraAttributes()
@@ -2494,7 +2494,7 @@ public function testStartTagWithExtraAttributes()
         ]);
 
         $html = $this->renderStart($form->createView(), [
-            'attr' => ['class' => 'foobar'],
+            'row_attr' => ['class' => 'foobar'],
         ]);
 
         $this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar">', $html);

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AbstractTableLayoutTestCase.php b/src/Symfony/Bridge/Twig/Tests/Extension/AbstractTableLayoutTestCase.php
@@ -271,7 +271,7 @@ public function testForm()
     ]
     [@method="post"]
     [@action="http://example.com"]
-    [@class="my&class"]
+    [@id="form_name"]
 '
         );
     }

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap3LayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap3LayoutTest.php
@@ -57,7 +57,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get">', $html);
+        $this->assertSame('<form name="form" method="get" id="form_form">', $html);
     }
 
     public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -69,7 +69,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="0">', $html);
+        $this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
     }
 
     public function testMoneyWidgetInIso()

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap4LayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap4LayoutTest.php
@@ -62,7 +62,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get">', $html);
+        $this->assertSame('<form name="form" method="get" id="form_form">', $html);
     }
 
     public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -74,7 +74,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="0">', $html);
+        $this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
     }
 
     public function testMoneyWidgetInIso()

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap5LayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap5LayoutTest.php
@@ -64,7 +64,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
 
         $html = $this->renderStart($form->createView());
 
-        self::assertSame('<form name="form" method="get">', $html);
+        self::assertSame('<form name="form" method="get" id="form_form">', $html);
     }
 
     public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -76,7 +76,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
 
         $html = $this->renderStart($form->createView());
 
-        self::assertSame('<form name="form" method="get" action="0">', $html);
+        self::assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
     }
 
     public function testMoneyWidgetInIso()

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
@@ -131,7 +131,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get">', $html);
+        $this->assertSame('<form name="form" method="get" id="form_form">', $html);
     }
 
     public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -143,7 +143,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="0">', $html);
+        $this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
     }
 
     public static function isRootFormProvider()

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php
@@ -58,7 +58,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get">', $html);
+        $this->assertSame('<form name="form" method="get" id="form_form">', $html);
     }
 
     public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -70,7 +70,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
 
         $html = $this->renderStart($form->createView());
 
-        $this->assertSame('<form name="form" method="get" action="0">', $html);
+        $this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
     }
 
     public function testHelpAttr()

diff --git a/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php b/src/Symfony/Component/Form/Extension/Core/Type/BaseType.php
@@ -71,21 +71,25 @@ public function buildView(FormView $view, FormInterface $form, array $options)
             }
 
             $rootFormAttrOption = $form->getRoot()->getConfig()->getOption('form_attr');
+            $rootFormName = $form->getRoot()->getName();
             if ($options['form_attr'] || $rootFormAttrOption) {
-                $options['attr']['form'] = \is_string($rootFormAttrOption) ? $rootFormAttrOption : $form->getRoot()->getName();
-                if (empty($options['attr']['form'])) {
+                if (true === $rootFormAttrOption && empty($rootFormName)) {
                     throw new LogicException('"form_attr" option must be a string identifier on root form when it has no id.');
                 }
+
+                $options['attr']['form'] = \is_string($rootFormAttrOption) ? $rootFormAttrOption : 'form_'.$rootFormName;
             }
         } else {
-            $id = \is_string($options['form_attr']) ? $options['form_attr'] : $name;
+            $id = $name;
             $fullName = $name;
             $uniqueBlockPrefix = '_'.$blockName;
 
             // Strip leading underscores and digits. These are allowed in
             // form names, but not in HTML4 ID attributes.
             // https://www.w3.org/TR/html401/struct/global#adef-id
             $id = ltrim($id, '_0123456789');
+
+            $options['row_attr'] += ['id' => \is_string($options['form_attr']) ? $options['form_attr'] : 'form_'.$id];
         }
 
         $blockPrefixes = [];

diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ButtonTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ButtonTypeTest.php
@@ -50,8 +50,8 @@ public function testFormAttrOnRoot()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
-        $this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child2']->vars['attr']['form']);
     }
 
     public function testFormAttrOnChild()
@@ -65,7 +65,7 @@ public function testFormAttrOnChild()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
         $this->assertArrayNotHasKey('form', $view['child2']->vars['attr']);
     }
 
@@ -95,8 +95,8 @@ public function testFormAttrAsStringWithNoId()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($stringId, $view->vars['id']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
-        $this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
+        $this->assertSame($stringId, $view->vars['row_attr']['id']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child2']->vars['attr']['form']);
     }
 }
diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php
@@ -785,8 +785,8 @@ public function testFormAttrOnRoot()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
-        $this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child2']->vars['attr']['form']);
     }
 
     public function testFormAttrOnChild()
@@ -800,7 +800,7 @@ public function testFormAttrOnChild()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
         $this->assertArrayNotHasKey('form', $view['child2']->vars['attr']);
     }
 
@@ -830,9 +830,9 @@ public function testFormAttrAsStringWithNoId()
             ->getForm()
             ->createView();
         $this->assertArrayNotHasKey('form', $view->vars['attr']);
-        $this->assertSame($stringId, $view->vars['id']);
-        $this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
-        $this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
+        $this->assertSame($stringId, $view->vars['row_attr']['id']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child1']->vars['attr']['form']);
+        $this->assertSame($view->vars['row_attr']['id'], $view['child2']->vars['attr']['form']);
     }
 
     public function testSortingViewChildrenBasedOnPriorityOption()