From a273e798985f69ecfeadee5fc3bcbf050acf6e02 Mon Sep 17 00:00:00 2001
From: Bernhard Schussek <bschussek@gmail.com>
Date: Fri, 13 Sep 2013 16:55:54 +0200
Subject: [PATCH] [Form] Fixed: "required" attribute is not added to <select>
 tag if no empty value

---
 .../views/Form/form_div_layout.html.twig      |  3 +++
 .../Form/choice_widget_collapsed.html.php     |  4 +++-
 .../Form/Extension/Core/Type/ChoiceType.php   |  4 +++-
 .../Form/Tests/AbstractLayoutTest.php         | 22 ++++++++++++++-----
 .../Extension/Core/Type/ChoiceTypeTest.php    |  2 ++
 5 files changed, 28 insertions(+), 7 deletions(-)

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 %}
     <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>
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php
index 8ffa584354..701ef07e66 100644
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/choice_widget_collapsed.html.php
+++ b/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; ?>
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()