Skip to content

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from
Open

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}) %}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have to make this change here, it seems to me that we need to adapt for custom form types that do not provide the row_attr variable (and that we need to fall back to attr then).

{{- parent() -}}
{%- endblock form_start %}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,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 -%}
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap3HorizontalLayoutTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Extension/AbstractBootstrap4HorizontalLayoutTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Tests/Extension/AbstractDivLayoutTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ public function testForm()
]
[@method="post"]
[@action="http://example.com"]
[@class="my&class"]
[@id="form_name"]
'
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Extension/AbstractLayoutTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2374,7 +2374,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()
Expand Down Expand Up @@ -2406,7 +2406,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()
Expand All @@ -2420,7 +2420,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()
Expand All @@ -2431,7 +2431,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);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Tests/Extension/AbstractTableLayoutTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ public function testForm()
]
[@method="post"]
[@action="http://example.com"]
[@class="my&class"]
[@id="form_name"]
'
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap3LayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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()
Expand All @@ -43,7 +43,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()
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap4LayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,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()
Expand All @@ -48,7 +48,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()
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionBootstrap5LayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,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()
Expand All @@ -50,7 +50,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()
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,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()
Expand All @@ -115,7 +115,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(): array
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionTableLayoutTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,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()
Expand All @@ -38,7 +38,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()
Expand Down
10 changes: 7 additions & 3 deletions src/Symfony/Component/Form/Extension/Core/Type/BaseType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,21 +65,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 = [];
Expand Down
12 changes: 6 additions & 6 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/ButtonTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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']);
}

Expand Down Expand Up @@ -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']);
}
}
12 changes: 6 additions & 6 deletions src/Symfony/Component/Form/Tests/Extension/Core/Type/FormTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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']);
}

Expand Down Expand Up @@ -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()
Expand Down