From fb002d89ea33a62e026fcc800c2ddb3bdf0c3e74 Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 25 Jul 2012 13:20:23 +0200 Subject: [PATCH] [Form] Fixed variable passing from outer to inner blocks of the same FormView instance --- .../Twig/Node/SearchAndRenderBlockNode.php | 44 +++-- .../views/Form/form_div_layout.html.twig | 2 +- .../views/Form/form_table_layout.html.twig | 2 +- .../Node/SearchAndRenderBlockNodeTest.php | 187 ++++++++++++++++++ .../Resources/views/Form/form_row.html.php | 2 +- .../views/FormTable/form_row.html.php | 2 +- .../Templating/Helper/FormHelper.php | 2 +- src/Symfony/Component/Form/FormRenderer.php | 76 ++++--- .../Form/Tests/AbstractDivLayoutTest.php | 10 +- .../Form/Tests/AbstractLayoutTest.php | 25 ++- 10 files changed, 299 insertions(+), 53 deletions(-) create mode 100644 src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php diff --git a/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php b/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php index 8eec7e8c11b2..b93ebaafcafa 100644 --- a/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php +++ b/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php @@ -32,22 +32,44 @@ public function compile(\Twig_Compiler $compiler) $compiler->raw(', \'' . $blockNameSuffix . '\''); if (isset($arguments[1])) { - $compiler->raw(', '); - - // The "label" function allows one extra argument here, the label if ('label' === $blockNameSuffix) { - if (isset($arguments[2])) { - $compiler->subcompile($arguments[2]); - $compiler->raw(' + '); + // The "label" function expects the label in the second argument. + // The array of variables is given in the third argument + $lineno = $arguments[1]->getLine(); + $variables = new \Twig_Node_Expression_Array(array(), $lineno); + $givenVariables = isset($arguments[2]) ? $arguments[2] : $variables; + $labelKey = new \Twig_Node_Expression_Constant('label', $lineno); + $found = false; + + // If the label is listed in the variables, the label given + // in the arguments should take precedence in the following form: + // labelInArgs|default(labelInAttr) + foreach ($givenVariables->getKeyValuePairs() as $pair) { + if ((string) $labelKey === (string) $pair['key']) { + $pair['value'] = new \Twig_Node_Expression_Filter_Default( + $arguments[1], + new \Twig_Node_Expression_Constant('default', $lineno), + new \Twig_Node(array($pair['value']), array(), $lineno), + $lineno + ); + $found = true; + } + + $variables->addElement($pair['value'], $pair['key']); } - // Add the label to the variable array - $compiler->raw('array(\'label\' => '); - $compiler->subcompile($arguments[1]); - $compiler->raw(')'); + // If the label does not exist in the variables, simply add it + if (!$found) { + $variables->addElement($arguments[1], $labelKey); + } } else { - $compiler->subcompile($arguments[1]); + // All other functions than "label" expect the variables + // in the second argument + $variables = $arguments[1]; } + + $compiler->raw(', '); + $compiler->subcompile($variables); } } 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 index 3c10d53d3701..0e562e414273 100644 --- 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 @@ -249,7 +249,7 @@ {% block form_row %} {% spaceless %}
- {{ form_label(form, label|default(null)) }} + {{ form_label(form) }} {{ form_errors(form) }} {{ form_widget(form) }}
diff --git a/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig b/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig index 27f830fe2d4f..63bd7d278713 100644 --- a/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig +++ b/src/Symfony/Bridge/Twig/Resources/views/Form/form_table_layout.html.twig @@ -4,7 +4,7 @@ {% spaceless %} - {{ form_label(form, label|default(null)) }} + {{ form_label(form) }} {{ form_errors(form) }} diff --git a/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php b/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php new file mode 100644 index 000000000000..23f3cc38b4d8 --- /dev/null +++ b/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php @@ -0,0 +1,187 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bridge\Twig\Tests\Node; + +use Symfony\Bridge\Twig\Tests\TestCase; +use Symfony\Bridge\Twig\Node\SearchAndRenderBlockNode; + +class SearchAndRenderBlockNodeTest extends TestCase +{ + protected function setUp() + { + parent::setUp(); + + if (version_compare(\Twig_Environment::VERSION, '1.5.0', '<')) { + $this->markTestSkipped('Requires Twig version to be at least 1.5.0.'); + } + } + + public function testCompileWidget() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + )); + + $node = new SearchAndRenderBlockNode('form_widget', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'widget\')', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileWidgetWithVariables() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Array(array( + new \Twig_Node_Expression_Constant('foo', 0), + new \Twig_Node_Expression_Constant('bar', 0), + ), 0), + )); + + $node = new SearchAndRenderBlockNode('form_widget', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'widget\', array("foo" => "bar"))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithLabel() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Constant('my label', 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => "my label"))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithNullLabel() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Constant(null, 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => null))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithDefaultLabel() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithAttributes() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Constant(null, 0), + new \Twig_Node_Expression_Array(array( + new \Twig_Node_Expression_Constant('foo', 0), + new \Twig_Node_Expression_Constant('bar', 0), + ), 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => null))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithLabelAndAttributes() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Constant('value in argument', 0), + new \Twig_Node_Expression_Array(array( + new \Twig_Node_Expression_Constant('foo', 0), + new \Twig_Node_Expression_Constant('bar', 0), + new \Twig_Node_Expression_Constant('label', 0), + new \Twig_Node_Expression_Constant('value in attributes', 0), + ), 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => _twig_default_filter("value in argument", "value in attributes")))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + protected function getVariableGetter($name) + { + if (version_compare(phpversion(), '5.4.0RC1', '>=')) { + return sprintf('(isset($context["%s"]) ? $context["%s"] : null)', $name, $name); + } + + return sprintf('$this->getContext($context, "%s")', $name); + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php index 091807020d23..a4f86d022318 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/form_row.html.php @@ -1,5 +1,5 @@
- label($form, isset($label) ? $label : null) ?> + label($form) ?> errors($form) ?> widget($form) ?>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php index b9e5c5639ce4..7e1f2f5d28db 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/FormTable/form_row.html.php @@ -1,6 +1,6 @@ - label($form, isset($label) ? $label : null) ?> + label($form) ?> errors($form) ?> diff --git a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php index b8c6a0256aa4..e271bc56c3a9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php +++ b/src/Symfony/Bundle/FrameworkBundle/Templating/Helper/FormHelper.php @@ -125,7 +125,7 @@ public function row(FormView $view, array $variables = array()) */ public function label(FormView $view, $label = null, array $variables = array()) { - if ($label !== null) { + if (null !== $label) { $variables += array('label' => $label); } diff --git a/src/Symfony/Component/Form/FormRenderer.php b/src/Symfony/Component/Form/FormRenderer.php index 4d5538f10612..d464d314ee35 100644 --- a/src/Symfony/Component/Form/FormRenderer.php +++ b/src/Symfony/Component/Form/FormRenderer.php @@ -22,6 +22,8 @@ */ class FormRenderer implements FormRendererInterface { + const CACHE_KEY_VAR = 'full_block_name'; + /** * @var FormRendererEngineInterface */ @@ -42,11 +44,6 @@ class FormRenderer implements FormRendererInterface */ private $hierarchyLevelMap = array(); - /** - * @var array - */ - private $variableMap = array(); - /** * @var array */ @@ -95,7 +92,8 @@ public function renderBlock(FormView $view, $blockName, array $variables = array throw new FormException('This method should only be called while rendering a form element.'); } - $scopeVariables = end($this->variableStack); + $viewCacheKey = $view->vars[self::CACHE_KEY_VAR]; + $scopeVariables = end($this->variableStack[$viewCacheKey]); $resource = $this->engine->getResourceForBlockName($view, $blockName); @@ -117,13 +115,13 @@ public function renderBlock(FormView $view, $blockName, array $variables = array // cannot be overwritten $variables = array_replace($scopeVariables, $variables); - $this->variableStack[] = $variables; + $this->variableStack[$viewCacheKey][] = $variables; // Do the rendering $html = $this->engine->renderBlock($view, $resource, $blockName, $variables); // Clear the stack - array_pop($this->variableStack); + array_pop($this->variableStack[$viewCacheKey]); return $html; } @@ -140,7 +138,8 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va } // The cache key for storing the variables and types - $mapKey = $uniqueBlockName = $view->vars['full_block_name'] . '_' . $blockNameSuffix; + $viewCacheKey = $view->vars[self::CACHE_KEY_VAR]; + $viewAndSuffixCacheKey = $viewCacheKey . $blockNameSuffix; // In templates, we have to deal with two kinds of block hierarchies: // @@ -169,7 +168,7 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va // widget() function again to render the block for the parent type. // // The second kind is implemented in the following blocks. - if (!isset($this->blockNameHierarchyMap[$mapKey])) { + if (!isset($this->blockNameHierarchyMap[$viewAndSuffixCacheKey])) { // INITIAL CALL // Calculate the hierarchy of template blocks and start on // the bottom level of the hierarchy (= "__
" block) @@ -177,21 +176,33 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va foreach ($view->vars['types'] as $type) { $blockNameHierarchy[] = $type . '_' . $blockNameSuffix; } - $blockNameHierarchy[] = $uniqueBlockName; + $blockNameHierarchy[] = $view->vars['full_block_name'] . '_' . $blockNameSuffix; $hierarchyLevel = count($blockNameHierarchy) - 1; - // The default variable scope contains all view variables, merged with - // the variables passed explicitly to the helper - $scopeVariables = $view->vars; + $hierarchyInit = true; } else { // RECURSIVE CALL // If a block recursively calls renderSection() again, resume rendering // using the parent type in the hierarchy. - $blockNameHierarchy = $this->blockNameHierarchyMap[$mapKey]; - $hierarchyLevel = $this->hierarchyLevelMap[$mapKey] - 1; + $blockNameHierarchy = $this->blockNameHierarchyMap[$viewAndSuffixCacheKey]; + $hierarchyLevel = $this->hierarchyLevelMap[$viewAndSuffixCacheKey] - 1; + + $hierarchyInit = false; + } + // The variables are cached globally for a view (instead of for the + // current suffix) + if (!isset($this->variableStack[$viewCacheKey])) { + // The default variable scope contains all view variables, merged with + // the variables passed explicitly to the helper + $scopeVariables = $view->vars; + + $varInit = true; + } else { // Reuse the current scope and merge it with the explicitly passed variables - $scopeVariables = $this->variableMap[$mapKey]; + $scopeVariables = end($this->variableStack[$viewCacheKey]); + + $varInit = false; } // Load the resource where this block can be found @@ -235,28 +246,29 @@ public function searchAndRenderBlock(FormView $view, $blockNameSuffix, array $va // We need to store these values in maps (associative arrays) because within a // call to widget() another call to widget() can be made, but for a different view // object. These nested calls should not override each other. - $this->blockNameHierarchyMap[$mapKey] = $blockNameHierarchy; - $this->hierarchyLevelMap[$mapKey] = $hierarchyLevel; - $this->variableMap[$mapKey] = $variables; + $this->blockNameHierarchyMap[$viewAndSuffixCacheKey] = $blockNameHierarchy; + $this->hierarchyLevelMap[$viewAndSuffixCacheKey] = $hierarchyLevel; - // We also need to store the view and the variables so that we can render custom - // blocks with renderBlock() using the same themes and variables as in the outer - // block. - // - // A stack is sufficient for this purpose, because renderBlock() always accesses - // the immediate next outer scope, which is always stored at the end of the stack. - $this->variableStack[] = $variables; + // We also need to store the variables for the view so that we can render other + // blocks for the same view using the same variables as in the outer block. + $this->variableStack[$viewCacheKey][] = $variables; // Do the rendering $html = $this->engine->renderBlock($view, $resource, $blockName, $variables); // Clear the stack - array_pop($this->variableStack); + array_pop($this->variableStack[$viewCacheKey]); - // Clear the maps - unset($this->blockNameHierarchyMap[$mapKey]); - unset($this->hierarchyLevelMap[$mapKey]); - unset($this->variableMap[$mapKey]); + // Clear the caches if they were filled for the first time within + // this function call + if ($hierarchyInit) { + unset($this->blockNameHierarchyMap[$viewAndSuffixCacheKey]); + unset($this->hierarchyLevelMap[$viewAndSuffixCacheKey]); + } + + if ($varInit) { + unset($this->variableStack[$viewCacheKey]); + } if ($renderOnlyOnce) { $view->setRendered(); diff --git a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php index bb97cd64a67f..75e8e193e4eb 100644 --- a/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractDivLayoutTest.php @@ -38,13 +38,17 @@ public function testRow() public function testRowOverrideVariables() { $view = $this->factory->createNamed('name', 'text')->createView(); - $html = $this->renderRow($view, array('label' => 'foo&bar')); + $html = $this->renderRow($view, array( + 'attr' => array('class' => 'my&class'), + 'label' => 'foo&bar', + 'label_attr' => array('class' => 'my&label&class'), + )); $this->assertMatchesXpath($html, '/div [ - ./label[@for="name"][.="[trans]foo&bar[/trans]"] - /following-sibling::input[@id="name"] + ./label[@for="name"][@class="my&label&class required"][.="[trans]foo&bar[/trans]"] + /following-sibling::input[@id="name"][@class="my&class"] ] ' ); diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 40195c2f3db6..b414c364c404 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -225,7 +225,7 @@ public function testLabelDoesNotRenderFieldAttributes() ); } - public function testLabelWithCustomOptionsPassedDirectly() + public function testLabelWithCustomAttributesPassedDirectly() { $form = $this->factory->createNamed('name', 'text'); $html = $this->renderLabel($form->createView(), null, array( @@ -242,7 +242,7 @@ public function testLabelWithCustomOptionsPassedDirectly() ); } - public function testLabelWithCustomTextAndCustomOptionsPassedDirectly() + public function testLabelWithCustomTextAndCustomAttributesPassedDirectly() { $form = $this->factory->createNamed('name', 'text'); $html = $this->renderLabel($form->createView(), 'Custom label', array( @@ -260,6 +260,27 @@ public function testLabelWithCustomTextAndCustomOptionsPassedDirectly() ); } + // https://github.com/symfony/symfony/issues/5029 + public function testLabelWithCustomTextAsOptionAndCustomAttributesPassedDirectly() + { + $form = $this->factory->createNamed('name', 'text', null, array( + 'label' => 'Custom label', + )); + $html = $this->renderLabel($form->createView(), null, array( + 'label_attr' => array( + 'class' => 'my&class' + ), + )); + + $this->assertMatchesXpath($html, + '/label + [@for="name"] + [@class="my&class required"] + [.="[trans]Custom label[/trans]"] +' + ); + } + public function testErrors() { $form = $this->factory->createNamed('name', 'text');