From 2e59d8000e79b2783573509d14b62ef0cbeee71c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Par=C3=A1da=20J=C3=B3zsef?= Date: Sun, 17 Jan 2016 01:29:59 +0100 Subject: [PATCH 1/6] bug #17270 [Form] Reset choiceLabels labels value after it's been copied --- src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index ef9a4b5baed00..23c3aa62f77f0 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -291,6 +291,9 @@ public function configureOptions(OptionsResolver $resolver) // forms) $labels = $choiceLabels->labels; + // Reset labels to prevent previous invocation + $choiceLabels->labels = array(); + return function ($choice, $key) use ($labels) { return $labels[$key]; }; From 7940fcbcbb8fd9e3082eecafc217f42074a97428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Par=C3=A1da=20J=C3=B3zsef?= Date: Sun, 17 Jan 2016 18:29:05 +0100 Subject: [PATCH 2/6] Cover special case with test --- .../Extension/Core/Type/ChoiceTypeTest.php | 37 +++++++++++++++++ .../Form/Tests/Fixtures/ChoiceSubType.php | 41 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index d7536f7640d3d..f6aa49bd98c99 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -14,6 +14,7 @@ use Symfony\Component\Form\ChoiceList\View\ChoiceGroupView; use Symfony\Component\Form\ChoiceList\View\ChoiceView; use Symfony\Component\Form\Extension\Core\ChoiceList\ObjectChoiceList; +use Symfony\Component\Form\Tests\Fixtures\ChoiceSubType; class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase { @@ -1887,4 +1888,40 @@ public function testInitializeWithDefaultObjectChoice() // Trigger data initialization $form->getViewData(); } + + /** + * This covers the case when: + * - Custom choice type added after a choice type. + * - Custom type is expanded. + * - Custom type replaces 'choices' normalizer with a custom one. + * In this case, custom type should not inherit labels from the first added choice type. + */ + public function testCustomChoiceTypeDoesNotInheritChoiceLabels() + { + $builder = $this->factory->createBuilder(); + $builder->add('choice', 'choice', array( + 'choices' => array( + '1' => '1', + '2' => '2' + ) + ) + ); + $builder->add('subChoice', new ChoiceSubType()); + $builder->add('choiceTwo', 'choice', array( + 'choices' => array( + '1' => '1', + '2' => '2', + '3' => '3', + ) + ) + ); + $form = $builder->getForm(); + + $this->assertInstanceOf('\Closure', $form->get('choice')->getConfig()->getOption('choice_label')); + // Since a custom 'choices' normalizer is set in ChoiceSubType, the $choicesNormalizer closure + // in ChoiceType::configureOptions() is not resolved and the $choiceLabels->labels will be an empty array. + // In this case the $choiceLabel closure in ChoiceType::configureOptions() will return null. + $this->assertNull($form->get('subChoice')->getConfig()->getOption('choice_label')); + $this->assertInstanceOf('\Closure', $form->get('choiceTwo')->getConfig()->getOption('choice_label')); + } } diff --git a/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php new file mode 100644 index 0000000000000..c1a8c16421785 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php @@ -0,0 +1,41 @@ + + */ +class ChoiceSubType extends AbstractType +{ + /** + * {@inheritdoc} + */ + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefaults(array('expanded' => true)); + $resolver->setNormalizer('choices', function () { + return array( + 'attr1' => 'Attribute 1', + 'attr2' => 'Attribute 2', + ); + }); + } + + /** + * {@inheritdoc} + */ + public function getName() + { + return 'sub_choice'; + } + + /** + * {@inheritdoc} + */ + public function getParent() + { + return 'choice'; + } +} \ No newline at end of file From 9d1babbd7f5a9f977d1c71027dad4413096c3fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Par=C3=A1da=20J=C3=B3zsef?= Date: Sun, 17 Jan 2016 18:36:01 +0100 Subject: [PATCH 3/6] cs fix by fabbot.io --- .../Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 6 +++--- src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index f6aa49bd98c99..7077d9f01fadd 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1902,8 +1902,8 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() $builder->add('choice', 'choice', array( 'choices' => array( '1' => '1', - '2' => '2' - ) + '2' => '2', + ), ) ); $builder->add('subChoice', new ChoiceSubType()); @@ -1912,7 +1912,7 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() '1' => '1', '2' => '2', '3' => '3', - ) + ), ) ); $form = $builder->getForm(); diff --git a/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php index c1a8c16421785..d1eeb9ec99405 100644 --- a/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php +++ b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php @@ -1,4 +1,5 @@ Date: Mon, 18 Jan 2016 00:18:37 +0100 Subject: [PATCH 4/6] Clarify comment --- .../Form/Extension/Core/Type/ChoiceType.php | 3 ++- .../Tests/Extension/Core/Type/ChoiceTypeTest.php | 16 +++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index 23c3aa62f77f0..515f104af692c 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -291,7 +291,8 @@ public function configureOptions(OptionsResolver $resolver) // forms) $labels = $choiceLabels->labels; - // Reset labels to prevent previous invocation + // The $choiceLabels object is shared with the 'choices' closure. + // Since that normalizer can be replaced, labels have to be cleared here. $choiceLabels->labels = array(); return function ($choice, $key) use ($labels) { diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index 7077d9f01fadd..e76b480fd4e0a 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1907,21 +1907,11 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() ) ); $builder->add('subChoice', new ChoiceSubType()); - $builder->add('choiceTwo', 'choice', array( - 'choices' => array( - '1' => '1', - '2' => '2', - '3' => '3', - ), - ) - ); $form = $builder->getForm(); - $this->assertInstanceOf('\Closure', $form->get('choice')->getConfig()->getOption('choice_label')); - // Since a custom 'choices' normalizer is set in ChoiceSubType, the $choicesNormalizer closure - // in ChoiceType::configureOptions() is not resolved and the $choiceLabels->labels will be an empty array. - // In this case the $choiceLabel closure in ChoiceType::configureOptions() will return null. + // The default 'choices' normalizer would fill the $choiceLabels, but it has been replaced + // in the custom choice type, so $choiceLabels->labels remains empty array. + // In this case the 'choice_label' closure returns null. $this->assertNull($form->get('subChoice')->getConfig()->getOption('choice_label')); - $this->assertInstanceOf('\Closure', $form->get('choiceTwo')->getConfig()->getOption('choice_label')); } } From 8f85eb2b18ba9e58496691541f62f7426c0d4851 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Par=C3=A1da=20J=C3=B3zsef?= Date: Mon, 18 Jan 2016 00:20:58 +0100 Subject: [PATCH 5/6] Clarify comment --- .../Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php index e76b480fd4e0a..cf74fec212ec5 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1911,7 +1911,7 @@ public function testCustomChoiceTypeDoesNotInheritChoiceLabels() // The default 'choices' normalizer would fill the $choiceLabels, but it has been replaced // in the custom choice type, so $choiceLabels->labels remains empty array. - // In this case the 'choice_label' closure returns null. + // In this case the 'choice_label' closure returns null and not the closure from the first choice type. $this->assertNull($form->get('subChoice')->getConfig()->getOption('choice_label')); } } From eab6ef5ab570d3f931b5a0887887abbc293502b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Par=C3=A1da=20J=C3=B3zsef?= Date: Mon, 18 Jan 2016 13:24:19 +0100 Subject: [PATCH 6/6] License header --- .../Component/Form/Tests/Fixtures/ChoiceSubType.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php index d1eeb9ec99405..857dd5d5a77eb 100644 --- a/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php +++ b/src/Symfony/Component/Form/Tests/Fixtures/ChoiceSubType.php @@ -1,5 +1,14 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Symfony\Component\Form\Tests\Fixtures; use Symfony\Component\Form\AbstractType;