Skip to content

Commit

Permalink
[Form] Fixed: Duplicate choice labels are remembered when using "choi…
Browse files Browse the repository at this point in the history
…ces_as_values" = false
  • Loading branch information
webmozart committed Nov 27, 2015
1 parent d1a50a2 commit 1179f07
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
72 changes: 71 additions & 1 deletion src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php
Expand Up @@ -238,6 +238,7 @@ public function finishView(FormView $view, FormInterface $form, array $options)
*/
public function configureOptions(OptionsResolver $resolver)
{
$choiceLabels = array();
$choiceListFactory = $this->choiceListFactory;

$emptyData = function (Options $options) {
Expand All @@ -252,6 +253,44 @@ public function configureOptions(OptionsResolver $resolver)
return $options['required'] ? null : '';
};

// BC closure, to be removed in 3.0
$choicesNormalizer = function (Options $options, $choices) use (&$choiceLabels) {
// Unset labels from previous invocations
$choiceLabels = array();

// This closure is irrelevant when "choices_as_values" is set to true
if ($options['choices_as_values']) {
return $choices;
}

ChoiceType::normalizeLegacyChoices($choices, $choiceLabels);

return $choices;
};

// BC closure, to be removed in 3.0
$choiceLabel = function (Options $options) use (&$choiceLabels) {
// If the choices contain duplicate labels, the normalizer of the
// "choices" option stores them in the $choiceLabels variable

// Trigger the normalizer
$options->offsetGet('choices');

// Pick labels from $choiceLabels if available
// Don't invoke count() to avoid creating a copy of the array (yet)
if ($choiceLabels) {
// Don't pass the labels by reference. We do want to create a
// copy here so that every form has an own version of that
// variable (contrary to the global reference shared by all
// forms)
return function ($choice, $key) use ($choiceLabels) {
return $choiceLabels[$key];
};
}

return;
};

$choiceListNormalizer = function (Options $options, $choiceList) use ($choiceListFactory) {
if ($choiceList) {
@trigger_error('The "choice_list" option is deprecated since version 2.7 and will be removed in 3.0. Use "choice_loader" instead.', E_USER_DEPRECATED);
Expand Down Expand Up @@ -322,7 +361,7 @@ public function configureOptions(OptionsResolver $resolver)
'choices' => array(),
'choices_as_values' => false,
'choice_loader' => null,
'choice_label' => null,
'choice_label' => $choiceLabel,
'choice_name' => null,
'choice_value' => null,
'choice_attr' => null,
Expand All @@ -340,6 +379,7 @@ public function configureOptions(OptionsResolver $resolver)
'choice_translation_domain' => true,
));

$resolver->setNormalizer('choices', $choicesNormalizer);
$resolver->setNormalizer('choice_list', $choiceListNormalizer);
$resolver->setNormalizer('placeholder', $placeholderNormalizer);
$resolver->setNormalizer('choice_translation_domain', $choiceTranslationDomainNormalizer);
Expand Down Expand Up @@ -454,4 +494,34 @@ private function createChoiceListView(ChoiceListInterface $choiceList, array $op
$options['choice_attr']
);
}

/**
* When "choices_as_values" is set to false, the choices are in the keys and
* their labels in the values. Labels may occur twice. The form component
* flips the choices array in the new implementation, so duplicate labels
* are lost. Store them in a utility array that is used from the
* "choice_label" closure by default.
*
* @param array $choices The choice labels indexed by choices.
* Labels are replaced by generated keys.
* @param array $choiceLabels The array that receives the choice labels
* indexed by generated keys.
* @param int|null $nextKey The next generated key.
*
* @internal Public only to be accessible from closures on PHP 5.3. Don't
* use this method, as it may be removed without notice.
*/
public static function normalizeLegacyChoices(array &$choices, array &$choiceLabels, &$nextKey = 0)
{
foreach ($choices as $choice => &$choiceLabel) {
if (is_array($choiceLabel)) {
self::normalizeLegacyChoices($choiceLabel, $choiceLabels, $nextKey);
continue;
}

$choiceLabels[$nextKey] = $choiceLabel;
$choices[$choice] = $nextKey;
++$nextKey;
}
}
}
Expand Up @@ -1694,6 +1694,23 @@ public function testPassChoiceDataToView()
), $view->vars['choices']);
}

/**
* @group legacy
*/
public function testDuplicateChoiceLabels()
{
$form = $this->factory->create('choice', null, array(
'choices' => array('a' => 'A', 'b' => 'B', 'c' => 'A'),
));
$view = $form->createView();

$this->assertEquals(array(
new ChoiceView('a', 'a', 'A'),
new ChoiceView('b', 'b', 'B'),
new ChoiceView('c', 'c', 'A'),
), $view->vars['choices']);
}

public function testAdjustFullNameForMultipleNonExpanded()
{
$form = $this->factory->createNamed('name', 'choice', null, array(
Expand Down

0 comments on commit 1179f07

Please sign in to comment.