Skip to content

Commit 3fd0ed7

Browse files
committed
[Form] Add new computed form_id view variable, use row_attr instead of attr for form element, remove string as allowed type for form_attr
1 parent 663d047 commit 3fd0ed7

14 files changed

+48
-74
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{% use "bootstrap_3_layout.html.twig" %}
22

33
{% block form_start -%}
4-
{% set attr = attr|merge({class: (attr.class|default('') ~ ' form-horizontal')|trim}) %}
4+
{% set row_attr = row_attr|merge({class: (row_attr.class|default('') ~ ' form-horizontal')|trim}) %}
55
{{- parent() -}}
66
{%- endblock form_start %}
77

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
{%- endblock form_widget_simple -%}
1919

2020
{%- block form_widget_compound -%}
21-
<div{% if not form.methodRendered %} {{ block('widget_container_attributes') }}{% endif %}>
21+
<div {{ block('widget_container_attributes') }}>
2222
{%- if form is rootform -%}
2323
{{ form_errors(form) }}
2424
{%- endif -%}
@@ -392,7 +392,8 @@
392392
{%- else -%}
393393
{% set form_method = "POST" %}
394394
{%- endif -%}
395-
<form{% if name != '' %} name="{{ name }}"{% endif %} method="{{ form_method|lower }}"{% if action != '' %} action="{{ action }}"{% endif %} {{ block('widget_container_attributes') }}{% if multipart %} enctype="multipart/form-data"{% endif %}>
395+
{%- set row_attr = row_attr|merge({id: form_id}) -%}
396+
<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 %}>
396397
{%- if form_method != method -%}
397398
<input type="hidden" name="_method" value="{{ method }}" />
398399
{%- endif -%}

src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3HorizontalLayoutTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public function testStartTag()
142142

143143
$html = $this->renderStart($form->createView());
144144

145-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" class="form-horizontal">', $html);
145+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="form-horizontal" id="form_form">', $html);
146146
}
147147

148148
public function testStartTagWithOverriddenVars()
@@ -157,7 +157,7 @@ public function testStartTagWithOverriddenVars()
157157
'action' => 'http://foo.com/directory',
158158
]);
159159

160-
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form" class="form-horizontal">', $html);
160+
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" class="form-horizontal" id="form_form">', $html);
161161
}
162162

163163
public function testStartTagForMultipartForm()
@@ -171,7 +171,7 @@ public function testStartTagForMultipartForm()
171171

172172
$html = $this->renderStart($form->createView());
173173

174-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" class="form-horizontal" enctype="multipart/form-data">', $html);
174+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="form-horizontal" id="form_form" enctype="multipart/form-data">', $html);
175175
}
176176

177177
public function testStartTagWithExtraAttributes()
@@ -182,10 +182,10 @@ public function testStartTagWithExtraAttributes()
182182
]);
183183

184184
$html = $this->renderStart($form->createView(), [
185-
'attr' => ['class' => 'foobar'],
185+
'row_attr' => ['class' => 'foobar'],
186186
]);
187187

188-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" class="foobar form-horizontal">', $html);
188+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar form-horizontal" id="form_form">', $html);
189189
}
190190

191191
public function testCheckboxRow()

src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap4HorizontalLayoutTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ public function testStartTag()
193193

194194
$html = $this->renderStart($form->createView());
195195

196-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form">', $html);
196+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form">', $html);
197197
}
198198

199199
public function testStartTagWithOverriddenVars()
@@ -208,7 +208,7 @@ public function testStartTagWithOverriddenVars()
208208
'action' => 'http://foo.com/directory',
209209
]);
210210

211-
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form">', $html);
211+
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form_form">', $html);
212212
}
213213

214214
public function testStartTagForMultipartForm()
@@ -222,7 +222,7 @@ public function testStartTagForMultipartForm()
222222

223223
$html = $this->renderStart($form->createView());
224224

225-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" enctype="multipart/form-data">', $html);
225+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" enctype="multipart/form-data">', $html);
226226
}
227227

228228
public function testStartTagWithExtraAttributes()
@@ -233,10 +233,10 @@ public function testStartTagWithExtraAttributes()
233233
]);
234234

235235
$html = $this->renderStart($form->createView(), [
236-
'attr' => ['class' => 'foobar'],
236+
'row_attr' => ['class' => 'foobar'],
237237
]);
238238

239-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" class="foobar">', $html);
239+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar" id="form_form">', $html);
240240
}
241241

242242
public function testCheckboxRow()

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap3LayoutTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
6060

6161
$html = $this->renderStart($form->createView());
6262

63-
$this->assertSame('<form name="form" method="get" id="form">', $html);
63+
$this->assertSame('<form name="form" method="get" id="form_form">', $html);
6464
}
6565

6666
public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -72,7 +72,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
7272

7373
$html = $this->renderStart($form->createView());
7474

75-
$this->assertSame('<form name="form" method="get" action="0" id="form">', $html);
75+
$this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
7676
}
7777

7878
public function testMoneyWidgetInIso()

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap4LayoutTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
6464

6565
$html = $this->renderStart($form->createView());
6666

67-
$this->assertSame('<form name="form" method="get" id="form">', $html);
67+
$this->assertSame('<form name="form" method="get" id="form_form">', $html);
6868
}
6969

7070
public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -76,7 +76,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
7676

7777
$html = $this->renderStart($form->createView());
7878

79-
$this->assertSame('<form name="form" method="get" action="0" id="form">', $html);
79+
$this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
8080
}
8181

8282
public function testMoneyWidgetInIso()

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap5LayoutTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
6767

6868
$html = $this->renderStart($form->createView());
6969

70-
self::assertSame('<form name="form" method="get" id="form">', $html);
70+
self::assertSame('<form name="form" method="get" id="form_form">', $html);
7171
}
7272

7373
public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -79,7 +79,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
7979

8080
$html = $this->renderStart($form->createView());
8181

82-
self::assertSame('<form name="form" method="get" action="0" id="form">', $html);
82+
self::assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
8383
}
8484

8585
public function testMoneyWidgetInIso()

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
135135

136136
$html = $this->renderStart($form->createView());
137137

138-
$this->assertSame('<form name="form" method="get" id="form">', $html);
138+
$this->assertSame('<form name="form" method="get" id="form_form">', $html);
139139
}
140140

141141
public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -147,7 +147,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
147147

148148
$html = $this->renderStart($form->createView());
149149

150-
$this->assertSame('<form name="form" method="get" action="0" id="form">', $html);
150+
$this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
151151
}
152152

153153
public function isRootFormProvider()

src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function testStartTagHasNoActionAttributeWhenActionIsEmpty()
6262

6363
$html = $this->renderStart($form->createView());
6464

65-
$this->assertSame('<form name="form" method="get" id="form">', $html);
65+
$this->assertSame('<form name="form" method="get" id="form_form">', $html);
6666
}
6767

6868
public function testStartTagHasActionAttributeWhenActionIsZero()
@@ -74,7 +74,7 @@ public function testStartTagHasActionAttributeWhenActionIsZero()
7474

7575
$html = $this->renderStart($form->createView());
7676

77-
$this->assertSame('<form name="form" method="get" action="0" id="form">', $html);
77+
$this->assertSame('<form name="form" method="get" action="0" id="form_form">', $html);
7878
}
7979

8080
public function testHelpAttr()

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,10 @@ public function buildView(FormView $view, FormInterface $form, array $options)
100100

101101
$rootFormAttrOption = $form->getRoot()->getConfig()->getOption('form_attr');
102102
if ($options['form_attr'] || $rootFormAttrOption) {
103-
$view->vars['attr']['form'] = \is_string($rootFormAttrOption) ? $rootFormAttrOption : $form->getRoot()->getName();
104-
if (empty($view->vars['attr']['form'])) {
105-
throw new LogicException('"form_attr" option must be a string identifier on root form when it has no id.');
106-
}
103+
$view->vars['attr']['form'] = $view->parent->vars['form_id'];
107104
}
108-
} elseif (\is_string($options['form_attr'])) {
109-
$view->vars['id'] = $options['form_attr'];
105+
} else {
106+
$view->vars['form_id'] = 'form_'.$view->vars['id'];
110107
}
111108

112109
$formConfig = $form->getConfig();
@@ -233,7 +230,7 @@ public function configureOptions(OptionsResolver $resolver)
233230
$resolver->setAllowedTypes('is_empty_callback', ['null', 'callable']);
234231
$resolver->setAllowedTypes('getter', ['null', 'callable']);
235232
$resolver->setAllowedTypes('setter', ['null', 'callable']);
236-
$resolver->setAllowedTypes('form_attr', ['bool', 'string']);
233+
$resolver->setAllowedTypes('form_attr', ['bool']);
237234

238235
$resolver->setInfo('getter', 'A callable that accepts two arguments (the view data and the current form field) and must return a value.');
239236
$resolver->setInfo('setter', 'A callable that accepts three arguments (a reference to the view data, the submitted value and the current form field).');

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,11 +409,12 @@ public function testForm()
409409
/following-sibling::input[@type="hidden"][@id="name__token"]
410410
]
411411
[count(.//input)=3]
412+
[@id="my&id"]
413+
[@class="my&class"]
412414
]
413415
[@method="post"]
414416
[@action="http://example.com"]
415-
[@id="my&id"]
416-
[@class="my&class"]
417+
[@id="form_name"]
417418
'
418419
);
419420
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,7 +2404,7 @@ public function testStartTag()
24042404

24052405
$html = $this->renderStart($form->createView());
24062406

2407-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form">', $html);
2407+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form">', $html);
24082408
}
24092409

24102410
public function testStartTagForPutRequest()
@@ -2436,7 +2436,7 @@ public function testStartTagWithOverriddenVars()
24362436
'action' => 'http://foo.com/directory',
24372437
]);
24382438

2439-
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form">', $html);
2439+
$this->assertSame('<form name="form" method="post" action="http://foo.com/directory" id="form_form">', $html);
24402440
}
24412441

24422442
public function testStartTagForMultipartForm()
@@ -2450,7 +2450,7 @@ public function testStartTagForMultipartForm()
24502450

24512451
$html = $this->renderStart($form->createView());
24522452

2453-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" enctype="multipart/form-data">', $html);
2453+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form_form" enctype="multipart/form-data">', $html);
24542454
}
24552455

24562456
public function testStartTagWithExtraAttributes()
@@ -2461,10 +2461,10 @@ public function testStartTagWithExtraAttributes()
24612461
]);
24622462

24632463
$html = $this->renderStart($form->createView(), [
2464-
'attr' => ['class' => 'foobar'],
2464+
'row_attr' => ['class' => 'foobar'],
24652465
]);
24662466

2467-
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" id="form" class="foobar">', $html);
2467+
$this->assertSame('<form name="form" method="get" action="http://example.com/directory" class="foobar" id="form_form">', $html);
24682468
}
24692469

24702470
public function testWidgetAttributes()

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,12 @@ public function testForm()
266266
]
267267
]
268268
[count(.//input)=3]
269+
[@id="my&id"]
270+
[@class="my&class"]
269271
]
270272
[@method="post"]
271273
[@action="http://example.com"]
272-
[@id="my&id"]
273-
[@class="my&class"]
274+
[@id="form_name"]
274275
'
275276
);
276277
}

src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use Symfony\Component\Form\CallbackTransformer;
1515
use Symfony\Component\Form\DataMapperInterface;
1616
use Symfony\Component\Form\Exception\InvalidArgumentException;
17-
use Symfony\Component\Form\Exception\LogicException;
1817
use Symfony\Component\Form\Exception\TransformationFailedException;
1918
use Symfony\Component\Form\Extension\Core\Type\CurrencyType;
2019
use Symfony\Component\Form\Extension\Core\Type\FormType;
@@ -349,6 +348,12 @@ public function testAttributesException()
349348
$this->factory->create(static::TESTED_TYPE, null, ['attr' => '']);
350349
}
351350

351+
public function testFormAttributeException()
352+
{
353+
$this->expectException(InvalidOptionsException::class);
354+
$this->factory->create(static::TESTED_TYPE, null, ['form_attr' => '']);
355+
}
356+
352357
public function testActionCannotBeNull()
353358
{
354359
$this->expectException(InvalidOptionsException::class);
@@ -787,8 +792,8 @@ public function testFormAttrOnRoot()
787792
->getForm()
788793
->createView();
789794
$this->assertArrayNotHasKey('form', $view->vars['attr']);
790-
$this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
791-
$this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
795+
$this->assertSame($view->vars['form_id'], $view['child1']->vars['attr']['form']);
796+
$this->assertSame($view->vars['form_id'], $view['child2']->vars['attr']['form']);
792797
}
793798

794799
public function testFormAttrOnChild()
@@ -802,41 +807,10 @@ public function testFormAttrOnChild()
802807
->getForm()
803808
->createView();
804809
$this->assertArrayNotHasKey('form', $view->vars['attr']);
805-
$this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
810+
$this->assertSame($view->vars['form_id'], $view['child1']->vars['attr']['form']);
806811
$this->assertArrayNotHasKey('form', $view['child2']->vars['attr']);
807812
}
808813

809-
public function testFormAttrAsBoolWithNoId()
810-
{
811-
$this->expectException(LogicException::class);
812-
$this->expectErrorMessage('form_attr');
813-
$this->factory
814-
->createNamedBuilder('', self::TESTED_TYPE, null, [
815-
'form_attr' => true,
816-
])
817-
->add('child1', $this->getTestedType())
818-
->add('child2', $this->getTestedType())
819-
->getForm()
820-
->createView();
821-
}
822-
823-
public function testFormAttrAsStringWithNoId()
824-
{
825-
$stringId = 'custom-identifier';
826-
$view = $this->factory
827-
->createNamedBuilder('', self::TESTED_TYPE, null, [
828-
'form_attr' => $stringId,
829-
])
830-
->add('child1', $this->getTestedType())
831-
->add('child2', $this->getTestedType())
832-
->getForm()
833-
->createView();
834-
$this->assertArrayNotHasKey('form', $view->vars['attr']);
835-
$this->assertSame($stringId, $view->vars['id']);
836-
$this->assertSame($view->vars['id'], $view['child1']->vars['attr']['form']);
837-
$this->assertSame($view->vars['id'], $view['child2']->vars['attr']['form']);
838-
}
839-
840814
public function testSortingViewChildrenBasedOnPriorityOption()
841815
{
842816
$view = $this->factory->createNamedBuilder('parent', self::TESTED_TYPE)

0 commit comments

Comments
 (0)