- Patch #140783 by sun, chx, effulgentsia, David_Rothstein, webchick: a select list without #default_value() always passes form validation.

2 files changed, 36 insertions, 62 deletions

diff --git a/includes/form.inc b/includes/form.inc
index ed5fa2850..3d468391c 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -1166,21 +1166,14 @@ function _form_validate(&$elements, &$form_state, $form_id = NULL) {
         // does not change the form, it will be the value of the first option.
         // Because of this, form validation for the field will almost always
         // pass, even if the user did not select anything. To work around this
-        // browser behavior, select fields without a #default_value get an
-        // additional, first empty option by default. In case the submitted
-        // value is identical to the empty option's value, we reset the
-        // element's value to NULL to trigger the regular #required handling
-        // below.
+        // browser behavior, required select fields without a #default_value get
+        // an additional, first empty option. In case the submitted value is
+        // identical to the empty option's value, we reset the element's value
+        // to NULL to trigger the regular #required handling below.
         // @see form_process_select()
-        elseif ($elements['#type'] == 'select' && !$elements['#multiple'] && !isset($elements['#default_value']) && $elements['#value'] === (string) $elements['#empty_value']) {
-          if ($elements['#required']) {
-            $elements['#value'] = NULL;
-            form_set_value($elements, NULL, $form_state);
-          }
-          else {
-            $elements['#value'] = $elements['#empty_value'];
-            form_set_value($elements, $elements['#empty_value'], $form_state);
-          }
+        elseif ($elements['#type'] == 'select' && !$elements['#multiple'] && $elements['#required'] && !isset($elements['#default_value']) && $elements['#value'] === $elements['#empty_value']) {
+          $elements['#value'] = NULL;
+          form_set_value($elements, NULL, $form_state);
         }
         elseif (!isset($options[$elements['#value']])) {
           form_error($elements, $t('An illegal choice has been detected. Please contact the site administrator.'));
@@ -2172,6 +2165,15 @@ function form_type_select_value($element, $input = FALSE) {
         return (isset($element['#default_value']) && is_array($element['#default_value'])) ? $element['#default_value'] : array();
       }
     }
+    // Non-multiple select elements may have an empty option preprended to them
+    // (see form_process_select()). When this occurs, usually #empty_value is
+    // an empty string, but some forms set #empty_value to integer 0 or some
+    // other non-string constant. PHP receives all submitted form input as
+    // strings, but if the empty option is selected, set the value to match the
+    // empty value exactly.
+    elseif (isset($element['#empty_value']) && $input === (string) $element['#empty_value']) {
+      return $element['#empty_value'];
+    }
     else {
       return $input;
     }
@@ -2307,14 +2309,23 @@ function _form_options_flatten($array) {
  *     Whether this first option is a valid option depends on whether the field
  *     is #required or not.
  *   - #required: (optional) Whether the user needs to select an option (TRUE)
- *     or not (FALSE). Defaults to TRUE.
+ *     or not (FALSE). Defaults to FALSE.
  *   - #empty_option: (optional) The label to show for the first default option.
  *     By default, the label is automatically set to "- Please select -" for a
  *     required field and "- None -" for an optional field.
  *   - #empty_value: (optional) The value for the first default option, which is
- *     used to determine whether the user submitted a value or not. Defaults to
- *     '' (an empty string). To be used in case the field is optional and the
- *     empty default value should have a special value (e.g., a constant).
+ *     used to determine whether the user submitted a value or not.
+ *     - If #required is TRUE, this defaults to '' (an empty string).
+ *     - If #required is not TRUE and this value isn't set, then no extra option
+ *       is added to the select control, leaving the control in a slightly
+ *       illogical state, because there's no way for the user to select nothing,
+ *       since all user agents automatically preselect the first available
+ *       option. But people are used to this being the behavior of select
+ *       controls.
+ *       @todo Address the above issue in Drupal 8.
+ *     - If #required is not TRUE and this value is set (most commonly to an
+ *       empty string), then an extra option (see #empty_option above)
+ *       representing a "non-selection" is added with this as its value.
  *
  * @see _form_validate()
  */
@@ -2323,57 +2334,21 @@ function form_process_select($element) {
   if ($element['#multiple']) {
     $element['#attributes']['multiple'] = 'multiple';
     $element['#attributes']['name'] = $element['#name'] . '[]';
-    // If not explicitly set, #required has to default to FALSE (see below).
-    if (!isset($element['#required'])) {
-      $element['#required'] = FALSE;
-    }
   }
   // A non-#multiple select needs special handling to prevent user agents from
   // preselecting the first option without intention. #multiple select lists do
   // not get an empty option, as it would not make sense, user interface-wise.
   else {
-    $add_empty_option = FALSE;
-    // Select elements always have a value in HTML, so the expectation is that
-    // the user has to choose an option. Therefore, a select element is set to
-    // #required TRUE, unless it has been explicitly set to FALSE. This differs
-    // from every other element which gets a default of #required FALSE via
-    // form_builder(). To avoid this default, system_element_info() sets
-    // #required to NULL.
-    // If #required has been explicitly set to FALSE, the user may optionally
-    // choose an option, which can be "None".
-    if (isset($element['#required']) && !$element['#required']) {
-      $element['#required'] = FALSE;
-      $element += array(
-        '#empty_value' => '',
-        '#empty_option' => t('- None -'),
-      );
-      $add_empty_option = TRUE;
-    }
-    // Otherwise, if #required is TRUE (or not set) and there is no
-    // #default_value, then the user has to choose an option, which makes this
-    // element #required.
-    elseif (!isset($element['#default_value'])) {
-      // By only conditionally setting #required to TRUE, we additionally ensure
-      // that the select element does not get a required marker if it already
-      // has a value.
-      $element['#required'] = TRUE;
+    $required = $element['#required'];
+    // If the element is required and there is no #default_value, then add an
+    // empty option that will fail validation, so that the user is required to
+    // make a choice. Also, if there's a value for #empty_value or
+    // #empty_option, then add an option that represents emptiness.
+    if (($required && !isset($element['#default_value'])) || isset($element['#empty_value']) || isset($element['#empty_option'])) {
       $element += array(
         '#empty_value' => '',
-        '#empty_option' => t('- Select -'),
+        '#empty_option' => $required ? t('- Select - ') : t('- None -'),
       );
-      $add_empty_option = TRUE;
-    }
-    // If there is a #default_value and #required is not explicitly FALSE, then
-    // there is no point in adding an empty option which is never valid, and we
-    // just retain API compatibility.
-    if (!isset($element['#required'])) {
-      $element['#required'] = FALSE;
-    }
-    // If one of the above conditions is met, add a first empty default option,
-    // which is always invalid for #required select lists that do not specify a
-    // #default_value.
-    // @see _form_validate()
-    if ($add_empty_option) {
       // The empty option is prepended to #options and purposively not merged
       // to prevent another option in #options mistakenly using the same value
       // as #empty_value.
diff --git a/includes/install.core.inc b/includes/install.core.inc
index 9cbfd059c..fcf8812e1 100644
--- a/includes/install.core.inc
+++ b/includes/install.core.inc
@@ -1759,7 +1759,6 @@ function _install_configure_form($form, &$form_state, &$install_state) {
   $form['server_settings']['site_default_country'] = array(
     '#type' => 'select',
     '#title' => t('Default country'),
-    '#required' => FALSE,
     '#default_value' => variable_get('site_default_country', NULL),
     '#options' => $countries,
     '#description' => st('Select the default country for the site.'),