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 239780f3c6..e91d44e155 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 @@ -67,6 +67,9 @@ {% block choice_widget_collapsed %} {% spaceless %} + {% if required and empty_value is none and not empty_value_in_choices %} + {% set required = false %} + {% endif %} block($form, 'widget_attributes') ?> + block($form, 'widget_attributes', array( + 'required' => $required && (null !== $empty_value || $empty_value_in_choices) + )) ?> multiple="multiple" > diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index d678a6854a..692f91e95c 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -119,8 +119,10 @@ class ChoiceType extends AbstractType } // Check if the choices already contain the empty value + $view->vars['empty_value_in_choices'] = 0 !== count($options['choice_list']->getChoicesForValues(array(''))); + // Only add the empty value option if this is not the case - if (null !== $options['empty_value'] && 0 === count($options['choice_list']->getChoicesForValues(array('')))) { + if (null !== $options['empty_value'] && !$view->vars['empty_value_in_choices']) { $view->vars['empty_value'] = $options['empty_value']; } diff --git a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php index 5e7d3599bc..4e922caf0b 100644 --- a/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractLayoutTest.php @@ -369,10 +369,22 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg 'expanded' => false, )); + // If the field is collapsed, has no "multiple" attribute, is required but + // has *no* empty value, the "required" must not be added, otherwise + // the resulting HTML is invalid. + // https://github.com/symfony/symfony/issues/8942 + + // HTML 5 spec + // http://www.w3.org/html/wg/drafts/html/master/forms.html#placeholder-label-option + + // "If a select element has a required attribute specified, does not + // have a multiple attribute specified, and has a display size of 1, + // then the select element must have a placeholder label option." + $this->assertWidgetMatchesXpath($form->createView(), array(), '/select [@name="name"] - [@required="required"] + [not(@required)] [ ./option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"] /following-sibling::option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"] @@ -394,7 +406,7 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg $this->assertWidgetMatchesXpath($form->createView(), array('separator' => '-- sep --'), '/select [@name="name"] - [@required="required"] + [not(@required)] [ ./option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"] /following-sibling::option[@disabled="disabled"][not(@selected)][.="-- sep --"] @@ -417,7 +429,7 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg $this->assertWidgetMatchesXpath($form->createView(), array('separator' => null), '/select [@name="name"] - [@required="required"] + [not(@required)] [ ./option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"] /following-sibling::option[@value="&a"][@selected="selected"][.="[trans]Choice&A[/trans]"] @@ -439,7 +451,7 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg $this->assertWidgetMatchesXpath($form->createView(), array('separator' => ''), '/select [@name="name"] - [@required="required"] + [not(@required)] [ ./option[@value="&b"][not(@selected)][.="[trans]Choice&B[/trans]"] /following-sibling::option[@disabled="disabled"][not(@selected)][.=""] @@ -1704,7 +1716,7 @@ abstract class AbstractLayoutTest extends \Symfony\Component\Form\Test\FormInteg $this->assertWidgetMatchesXpath($form->createView(), array(), '/select [@name="name"] - [@required="required"] + [not(@required)] [./optgroup [@label="[trans]Europe[/trans]"] [./option[@value="Europe/Vienna"][@selected="selected"][.="[trans]Vienna[/trans]"]] 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 ee40e74268..3da49dfa6a 100644 --- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php +++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/ChoiceTypeTest.php @@ -1050,6 +1050,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase $view = $form->createView(); $this->assertEquals($viewValue, $view->vars['empty_value']); + $this->assertFalse($view->vars['empty_value_in_choices']); } /** @@ -1067,6 +1068,7 @@ class ChoiceTypeTest extends \Symfony\Component\Form\Test\TypeTestCase $view = $form->createView(); $this->assertNull($view->vars['empty_value']); + $this->assertTrue($view->vars['empty_value_in_choices']); } public function getOptionsWithEmptyValue()