From 260739030964d1d0e69047c882737431397ce29d Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Wed, 25 Jul 2012 19:03:07 +0200 Subject: [PATCH] [Form] Fixed SearchAndRenderBlockNode to really ignore empty labels --- .../Twig/Node/SearchAndRenderBlockNode.php | 84 ++++++++++----- .../Node/SearchAndRenderBlockNodeTest.php | 101 +++++++++++++++++- 2 files changed, 154 insertions(+), 31 deletions(-) diff --git a/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php b/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php index b93ebaafca..dca1686098 100644 --- a/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php +++ b/src/Symfony/Bridge/Twig/Node/SearchAndRenderBlockNode.php @@ -33,43 +33,71 @@ class SearchAndRenderBlockNode extends \Twig_Node_Expression_Function if (isset($arguments[1])) { if ('label' === $blockNameSuffix) { - // 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; + // The "label" function expects the label in the second and + // the variables in the third argument + $label = $arguments[1]; + $variables = isset($arguments[2]) ? $arguments[2] : null; + $lineno = $label->getLine(); - // 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; + if ($label instanceof \Twig_Node_Expression_Constant) { + // If the label argument is given as a constant, we can either + // strip it away if it is empty, or integrate it into the array + // of variables at compile time. + $labelIsExpression = false; + + // Only insert the label into the array if it is not empty + if (!twig_test_empty($label->getAttribute('value'))) { + $originalVariables = $variables; + $variables = new \Twig_Node_Expression_Array(array(), $lineno); + $labelKey = new \Twig_Node_Expression_Constant('label', $lineno); + + if (null !== $originalVariables) { + foreach ($originalVariables->getKeyValuePairs() as $pair) { + // Don't copy the original label attribute over if it exists + if ((string) $labelKey !== (string) $pair['key']) { + $variables->addElement($pair['value'], $pair['key']); + } + } + } + + // Insert the label argument into the array + $variables->addElement($label, $labelKey); } - - $variables->addElement($pair['value'], $pair['key']); - } - - // If the label does not exist in the variables, simply add it - if (!$found) { - $variables->addElement($arguments[1], $labelKey); + } else { + // The label argument is not a constant, but some kind of + // expression. This expression needs to be evaluated at runtime. + // Depending on the result (whether it is null or not), the + // label in the arguments should take precedence over the label + // in the attributes or not. + $labelIsExpression = true; } } else { // All other functions than "label" expect the variables // in the second argument + $label = null; $variables = $arguments[1]; + $labelIsExpression = false; } - $compiler->raw(', '); - $compiler->subcompile($variables); + if (null !== $variables || $labelIsExpression) { + $compiler->raw(', '); + + if (null !== $variables) { + $compiler->subcompile($variables); + } + + if ($labelIsExpression) { + if (null !== $variables) { + $compiler->raw(' + '); + } + + // Check at runtime whether the label is empty. + // If not, add it to the array at runtime. + $compiler->raw('(twig_test_empty($_label_ = '); + $compiler->subcompile($label); + $compiler->raw(') ? array() : array("label" => $_label_))'); + } + } } } diff --git a/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php b/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php index 23f3cc38b4..c1f247caab 100644 --- a/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php +++ b/src/Symfony/Bridge/Twig/Tests/Node/SearchAndRenderBlockNodeTest.php @@ -98,9 +98,33 @@ class SearchAndRenderBlockNodeTest extends TestCase $compiler = new \Twig_Compiler(new \Twig_Environment()); + // "label" => null must not be included in the output! + // Otherwise the default label is overwritten with null. $this->assertEquals( sprintf( - '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("label" => null))', + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithEmptyStringLabel() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Constant('', 0), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + // "label" => null must not be included in the output! + // Otherwise the default label is overwritten with null. + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\')', $this->getVariableGetter('form') ), trim($compiler->compile($node)->getSource()) @@ -141,9 +165,12 @@ class SearchAndRenderBlockNodeTest extends TestCase $compiler = new \Twig_Compiler(new \Twig_Environment()); + // "label" => null must not be included in the output! + // Otherwise the default label is overwritten with null. + // https://github.com/symfony/symfony/issues/5029 $this->assertEquals( sprintf( - '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => null))', + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar"))', $this->getVariableGetter('form') ), trim($compiler->compile($node)->getSource()) @@ -169,7 +196,75 @@ class SearchAndRenderBlockNodeTest extends TestCase $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->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => "value in argument"))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithLabelThatEvaluatesToNull() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Conditional( + // if + new \Twig_Node_Expression_Constant(true, 0), + // then + new \Twig_Node_Expression_Constant(null, 0), + // else + new \Twig_Node_Expression_Constant(null, 0), + 0 + ), + )); + + $node = new SearchAndRenderBlockNode('form_label', $arguments, 0); + + $compiler = new \Twig_Compiler(new \Twig_Environment()); + + // "label" => null must not be included in the output! + // Otherwise the default label is overwritten with null. + // https://github.com/symfony/symfony/issues/5029 + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', (twig_test_empty($_label_ = ((true) ? (null) : (null))) ? array() : array("label" => $_label_)))', + $this->getVariableGetter('form') + ), + trim($compiler->compile($node)->getSource()) + ); + } + + public function testCompileLabelWithLabelThatEvaluatesToNullAndAttributes() + { + $arguments = new \Twig_Node(array( + new \Twig_Node_Expression_Name('form', 0), + new \Twig_Node_Expression_Conditional( + // if + new \Twig_Node_Expression_Constant(true, 0), + // then + new \Twig_Node_Expression_Constant(null, 0), + // else + new \Twig_Node_Expression_Constant(null, 0), + 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()); + + // "label" => null must not be included in the output! + // Otherwise the default label is overwritten with null. + // https://github.com/symfony/symfony/issues/5029 + $this->assertEquals( + sprintf( + '$this->env->getExtension(\'form\')->renderer->searchAndRenderBlock(%s, \'label\', array("foo" => "bar", "label" => "value in attributes") + (twig_test_empty($_label_ = ((true) ? (null) : (null))) ? array() : array("label" => $_label_)))', $this->getVariableGetter('form') ), trim($compiler->compile($node)->getSource())