diff --git a/src/Symfony/Bridge/Twig/Extension/FormExtension.php b/src/Symfony/Bridge/Twig/Extension/FormExtension.php
index 97605fdc626a..1c2fa540d301 100644
--- a/src/Symfony/Bridge/Twig/Extension/FormExtension.php
+++ b/src/Symfony/Bridge/Twig/Extension/FormExtension.php
@@ -88,12 +88,18 @@ public function getFilters()
/**
* Returns whether a choice is selected for a given form value.
*
- * This method exists for the sole purpose that Twig performs (a lot) better
- * with filters than with methods of an object.
+ * Unfortunately Twig does not support an efficient way to execute the
+ * "is_selected" closure passed to the template by ChoiceType. It is faster
+ * to implement the logic here (around 65ms for a specific form).
*
- * To give this some perspective, I'm currently testing this on a form with
- * a large list of entity fields. Using the filter is around 220ms faster than
- * accessing the method directly on the object in the Twig template.
+ * Directly implementing the logic here is also faster than doing so in
+ * ChoiceView (around 30ms).
+ *
+ * The worst option tested so far is to implement the logic in ChoiceView
+ * and access the ChoiceView method directly in the template. Doing so is
+ * around 220ms slower than doing the method call here in the filter. Twig
+ * seems to be much more efficient at executing filters than at executing
+ * methods of an object.
*
* @param ChoiceView $choice The choice to check.
* @param string|array $selectedValue The selected value to compare.
@@ -104,7 +110,11 @@ public function getFilters()
*/
public function isChoiceSelected(ChoiceView $choice, $selectedValue)
{
- return $choice->isSelected($selectedValue);
+ if (is_array($selectedValue)) {
+ return false !== array_search($choice->value, $selectedValue, true);
+ }
+
+ return $choice->value === $selectedValue;
}
/**
diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
index f057a515cc6d..90affd11b38c 100644
--- a/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
+++ b/src/Symfony/Bridge/Twig/Tests/Extension/FormExtensionDivLayoutTest.php
@@ -18,6 +18,7 @@
use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubTranslator;
use Symfony\Bridge\Twig\Tests\Extension\Fixtures\StubFilesystemLoader;
use Symfony\Component\Form\FormView;
+use Symfony\Component\Form\Extension\Core\View\ChoiceView;
use Symfony\Component\Form\Tests\AbstractDivLayoutTest;
class FormExtensionDivLayoutTest extends AbstractDivLayoutTest
@@ -105,6 +106,39 @@ public function testThemeBlockInheritanceUsingExtend()
);
}
+ public function isChoiceSelectedProvider()
+ {
+ // The commented cases should not be necessary anymore, because the
+ // choice lists should assure that both values passed here are always
+ // strings
+ return array(
+// array(true, 0, 0),
+ array(true, '0', '0'),
+ array(true, '1', '1'),
+// array(true, false, 0),
+// array(true, true, 1),
+ array(true, '', ''),
+// array(true, null, ''),
+ array(true, '1.23', '1.23'),
+ array(true, 'foo', 'foo'),
+ array(true, 'foo10', 'foo10'),
+ array(true, 'foo', array(1, 'foo', 'foo10')),
+
+ array(false, 10, array(1, 'foo', 'foo10')),
+ array(false, 0, array(1, 'foo', 'foo10')),
+ );
+ }
+
+ /**
+ * @dataProvider isChoiceSelectedProvider
+ */
+ public function testIsChoiceSelected($expected, $choice, $value)
+ {
+ $choice = new ChoiceView($choice, $choice . ' label');
+
+ $this->assertSame($expected, $this->extension->isChoiceSelected($choice, $value));
+ }
+
protected function renderEnctype(FormView $view)
{
return (string) $this->extension->renderer->renderEnctype($view);
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php
index 9d79a4b725df..39c97f091fc6 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_options.html.php
@@ -6,6 +6,6 @@
block('choice_widget_options', array('choices' => $choice)) ?>
-
+
diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
index e906f46aa8e0..b031c45a5e7d 100644
--- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
+++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
@@ -90,6 +90,20 @@ public function buildView(FormView $view, FormInterface $form, array $options)
'empty_value' => null,
));
+ // The decision, whether a choice is selected, is potentially done
+ // thousand of times during the rendering of a template. Provide a
+ // closure here that is optimized for the value of the form, to
+ // avoid making the type check inside the closure.
+ if ($options['multiple']) {
+ $view->vars['is_selected'] = function ($choice, array $values) {
+ return false !== array_search($choice, $values, true);
+ };
+ } else {
+ $view->vars['is_selected'] = function ($choice, $value) {
+ return $choice === $value;
+ };
+ }
+
// Check if the choices already contain the empty value
// Only add the empty value option if this is not the case
if (0 === count($options['choice_list']->getIndicesForValues(array('')))) {
diff --git a/src/Symfony/Component/Form/Tests/FormRendererTest.php b/src/Symfony/Component/Form/Tests/FormRendererTest.php
deleted file mode 100644
index fa36e75a59d1..000000000000
--- a/src/Symfony/Component/Form/Tests/FormRendererTest.php
+++ /dev/null
@@ -1,75 +0,0 @@
-
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Symfony\Component\Form;
-
-/**
- * @author Bernhard Schussek
- */
-use Symfony\Component\Form\Extension\Core\View\ChoiceView;
-
-class FormRendererTest extends \PHPUnit_Framework_TestCase
-{
- /**
- * @var \PHPUnit_Framework_MockObject_MockObject
- */
- private $engine;
-
- /**
- * @var \PHPUnit_Framework_MockObject_MockObject
- */
- private $csrfProvider;
-
- /**
- * @var FormRenderer
- */
- private $renderer;
-
- protected function setUp()
- {
- $this->engine = $this->getMock('Symfony\Component\Form\FormRendererEngineInterface');
- $this->csrfProvider = $this->getMock('Symfony\Component\Form\Extension\Csrf\CsrfProvider\CsrfProviderInterface');
- $this->renderer = new FormRenderer($this->engine, $this->csrfProvider);
- }
-
- public function isChoiceSelectedProvider()
- {
- // The commented cases should not be necessary anymore, because the
- // choice lists should assure that both values passed here are always
- // strings
- return array(
-// array(true, 0, 0),
- array(true, '0', '0'),
- array(true, '1', '1'),
-// array(true, false, 0),
-// array(true, true, 1),
- array(true, '', ''),
-// array(true, null, ''),
- array(true, '1.23', '1.23'),
- array(true, 'foo', 'foo'),
- array(true, 'foo10', 'foo10'),
- array(true, 'foo', array(1, 'foo', 'foo10')),
-
- array(false, 10, array(1, 'foo', 'foo10')),
- array(false, 0, array(1, 'foo', 'foo10')),
- );
- }
-
- /**
- * @dataProvider isChoiceSelectedProvider
- */
- public function testIsChoiceSelected($expected, $choice, $value)
- {
- $choice = new ChoiceView($choice, $choice . ' label');
-
- $this->assertSame($expected, $this->renderer->isChoiceSelected($choice, $value));
- }
-}