merged branch bschussek/issue8942 (PR #9030)

This PR was merged into the 2.3 branch.

Discussion
----------

[Form] Fixed: "required" attribute is not added to <select> tag if no empty value

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #8942
| License       | MIT
| Doc PR        | -

Commits
-------

a273e79 [Form] Fixed: "required" attribute is not added to <select> tag if no empty value

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 %}
     <select {{ block('widget_attributes') }}{% if multiple %} multiple="multiple"{% endif %}>
         {% if empty_value is not none %}
             <option value=""{% if required and value is empty %} selected="selected"{% endif %}>{{ empty_value|trans({}, translation_domain) }}</option>

src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php

@@ -1,5 +1,7 @@
 <select
-    <?php echo $view['form']->block($form, 'widget_attributes') ?>
+    <?php echo $view['form']->block($form, 'widget_attributes', array(
+        'required' => $required && (null !== $empty_value || $empty_value_in_choices)
+    )) ?>
     <?php if ($multiple): ?> multiple="multiple"<?php endif ?>
 >
     <?php if (null !== $empty_value): ?><option value=""<?php if ($required and empty($value) && "0" !== $value): ?> selected="selected"<?php endif?>><?php echo $view->escape($view['translator']->trans($empty_value, array(), $translation_domain)) ?></option><?php endif; ?>

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'];
         }
 

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]"]]

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()