Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #25236 [Form][TwigBridge] Fix collision between view properties a…
…nd form fields (yceruto)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form][TwigBridge] Fix collision between view properties and form fields

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #18882
| License       | MIT
| Doc PR        | TODO

This introduce a new Twig test function `rootform` that guarantee the right access to the `parent` property of the form view. The rest of the properties (`vars` and `children`) are not used at least inside Symfony repo.

I've chosen this solution because it doesn't [affect the design of the form view class/interface](https://github.com/symfony/symfony/pull/19492/files#diff-f60b55ea46e40b9c4475a1bd361f6940R168) and because [the problem happen only on Twig](https://github.com/twigphp/Twig/blob/fd98722d15996561f598f9181dbcef8432e9ff85/lib/Twig/Extension/Core.php#L1439-L1447).

More details about the problem here:
* #24892
* #19492
* #23649 (comment)

_if this is approved_ we should update also:
* [`foundation_5_layout.html.twig`](https://github.com/symfony/symfony/blob/336600857b8bb47d5a7ba3d1924a0e7a3e2af7a8/src/Symfony/Bridge/Twig/Resources/views/Form/foundation_5_layout.html.twig#L321-L326) in `3.3` (done in #25305)
* [`bootstrap_4_layout.html.twig`](https://github.com/symfony/symfony/blob/76d356f36a692dd8ad796de363484c97d6731d1f/src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_4_layout.html.twig#L176) in `3.4` (done in #25306)

Commits
-------

8505894 Fix collision between view properties and form fields
  • Loading branch information
fabpot committed Dec 4, 2017
2 parents 6eedbb5 + 8505894 commit 9524396
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 5 deletions.
7 changes: 7 additions & 0 deletions src/Symfony/Bridge/Twig/Extension/FormExtension.php
Expand Up @@ -14,6 +14,7 @@
use Symfony\Bridge\Twig\TokenParser\FormThemeTokenParser;
use Symfony\Bridge\Twig\Form\TwigRendererInterface;
use Symfony\Component\Form\Extension\Core\View\ChoiceView;
use Symfony\Component\Form\FormView;
use Twig\Environment;
use Twig\Extension\AbstractExtension;
use Twig\Extension\InitRuntimeInterface;
Expand Down Expand Up @@ -97,6 +98,7 @@ public function getTests()
{
return array(
new TwigTest('selectedchoice', array($this, 'isSelectedChoice')),
new TwigTest('rootform', array($this, 'isRootForm')),
);
}

Expand Down Expand Up @@ -156,6 +158,11 @@ public function isSelectedChoice(ChoiceView $choice, $selectedValue)
return $choice->value === $selectedValue;
}

public function isRootForm(FormView $formView)
{
return null === $formView->parent;
}

/**
* {@inheritdoc}
*/
Expand Down
Expand Up @@ -238,12 +238,12 @@

{% block form_errors -%}
{% if errors|length > 0 -%}
{% if form.parent %}<span class="help-block">{% else %}<div class="alert alert-danger">{% endif %}
{% if form is not rootform %}<span class="help-block">{% else %}<div class="alert alert-danger">{% endif %}
<ul class="list-unstyled">
{%- for error in errors -%}
<li><span class="glyphicon glyphicon-exclamation-sign"></span> {{ error.message }}</li>
{%- endfor -%}
</ul>
{% if form.parent %}</span>{% else %}</div>{% endif %}
{% if form is not rootform %}</span>{% else %}</div>{% endif %}
{%- endif %}
{%- endblock form_errors %}
Expand Up @@ -15,7 +15,7 @@

{%- block form_widget_compound -%}
<div {{ block('widget_container_attributes') }}>
{%- if form.parent is empty -%}
{%- if form is rootform -%}
{{ form_errors(form) }}
{%- endif -%}
{{- block('form_rows') -}}
Expand Down Expand Up @@ -303,7 +303,7 @@
{% endif %}
{%- endfor %}

{% if not form.methodRendered and form.parent is null %}
{% if not form.methodRendered and form is rootform %}
{%- do form.setMethodRendered() -%}
{% set method = method|upper %}
{%- if method in ["GET", "POST"] -%}
Expand Down
Expand Up @@ -31,7 +31,7 @@

{%- block form_widget_compound -%}
<table {{ block('widget_container_attributes') }}>
{%- if form.parent is empty and errors|length > 0 -%}
{%- if form is rootform and errors|length > 0 -%}
<tr>
<td colspan="2">
{{- form_errors(form) -}}
Expand Down
Expand Up @@ -146,6 +146,22 @@ public function testIsChoiceSelected($expected, $choice, $value)
$this->assertSame($expected, $this->extension->isSelectedChoice($choice, $value));
}

public function isRootFormProvider()
{
return array(
array(true, new FormView()),
array(false, new FormView(new FormView())),
);
}

/**
* @dataProvider isRootFormProvider
*/
public function testIsRootForm($expected, FormView $formView)
{
$this->assertSame($expected, $this->extension->isRootForm($formView));
}

protected function renderForm(FormView $view, array $vars = array())
{
return (string) $this->extension->renderer->renderBlock($view, 'form', $vars);
Expand Down

0 comments on commit 9524396

Please sign in to comment.