- Patch #735426 by yched, mr.baileys: fields with select widget: first option is selected by default even if no 'default value' set for the field.

2 files changed, 66 insertions, 27 deletions

diff --git a/modules/field/modules/options/options.module b/modules/field/modules/options/options.module
index 24ab04758..75683a9fb 100644
--- a/modules/field/modules/options/options.module
+++ b/modules/field/modules/options/options.module
@@ -25,7 +25,7 @@ function options_help($path, $arg) {
 function options_theme() {
   return array(
     'options_none' => array(
-      'variables' => array('instance' => NULL),
+      'variables' => array('instance' => NULL, 'option' => NULL),
     ),
   );
 }
@@ -76,7 +76,8 @@ function options_field_widget_form(&$form, &$form_state, $field, $instance, $lan
   $type = str_replace('options_', '', $instance['widget']['type']);
   $multiple = $field['cardinality'] > 1 || $field['cardinality'] == FIELD_CARDINALITY_UNLIMITED;
   $required = $element['#required'];
-  $properties = _options_properties($type, $multiple, $required);
+  $has_value = isset($items[0][$value_key]);
+  $properties = _options_properties($type, $multiple, $required, $has_value);
 
   // Prepare the list of options.
   $options = _options_get_options($field, $instance, $properties);
@@ -137,6 +138,9 @@ function options_field_widget_form(&$form, &$form_state, $field, $instance, $lan
  * Form element validation handler for options element.
  */
 function options_field_widget_validate($element, &$form_state) {
+  if ($element['#required'] && $element['#value'] == '_none') {
+    form_error($element, t('!name field is required.', array('!name' => $element['#title'])));
+  }
   // Transpose selections from field => delta to delta => field, turning
   // multiple selected options into multiple parent elements.
   $items = _options_form_to_storage($element);
@@ -146,12 +150,12 @@ function options_field_widget_validate($element, &$form_state) {
 /**
  * Describes the preparation steps required by each widget.
  */
-function _options_properties($type, $multiple, $required) {
+function _options_properties($type, $multiple, $required, $has_value) {
   $base = array(
     'zero_placeholder' => FALSE,
     'filter_xss' => FALSE,
     'strip_tags' => FALSE,
-    'empty_value' => FALSE,
+    'empty_option' => FALSE,
     'optgroups' => FALSE,
   );
 
@@ -162,9 +166,25 @@ function _options_properties($type, $multiple, $required) {
       $properties = array(
         // Select boxes do not support any HTML tag.
         'strip_tags' => TRUE,
-        'empty_value' => !$required,
         'optgroups' => TRUE,
       );
+      if ($multiple) {
+        // Multiple select: add a 'none' option for non-required fields.
+        if (!$required) {
+          $properties['empty_option'] = 'option_none';
+        }
+      }
+      else {
+        // Single select: add a 'none' option for non-required fields,
+        // and a 'select a value' option for required fields that do not come
+        // with a value selected.
+        if (!$required) {
+          $properties['empty_option'] = 'option_none';
+        }
+        else if (!$has_value) {
+          $properties['empty_option'] = 'option_select';
+        }
+      }
       break;
 
     case 'buttons':
@@ -173,9 +193,11 @@ function _options_properties($type, $multiple, $required) {
         // Form API 'checkboxes' do not suport 0 as an option, so we replace it with
         // a placeholder within the form workflow.
         'zero_placeholder' => $multiple,
-        // Checkboxes do not need a 'none' choice.
-        'empty_value' => !$required && !$multiple,
       );
+      // Add a 'none' option for non-required radio buttons.
+      if (!$required && !$multiple) {
+        $properties['empty_option'] = 'option_none';
+      }
       break;
 
     case 'onoff':
@@ -202,8 +224,9 @@ function _options_get_options($field, $instance, $properties) {
     $options = options_array_flatten($options);
   }
 
-  if ($properties['empty_value']) {
-    $options = array('_none' => theme('options_none', array('instance' => $instance))) + $options;
+  if ($properties['empty_option']) {
+    $label = theme('options_none', array('instance' => $instance, 'option' => $properties['empty_option']));
+    $options = array('_none' => $label) + $options;
   }
 
   return $options;
@@ -289,7 +312,7 @@ function _options_form_to_storage($element) {
 
   // Filter out the 'none' option. Use a strict comparison, because
   // 0 == 'any string'.
-  if ($properties['empty_value']) {
+  if ($properties['empty_option']) {
     $index = array_search('_none', $values, TRUE);
     if ($index !== FALSE) {
       unset($values[$index]);
@@ -368,7 +391,7 @@ function options_field_widget_error($element, $error, $form, &$form_state) {
 /**
  * Returns HTML for the label for the empty value for options that are not required.
  *
- * The default theme will display N/A for a radio list and blank for a select.
+ * The default theme will display N/A for a radio list and '- None -' for a select.
  *
  * @param $variables
  *   An associative array containing:
@@ -378,12 +401,18 @@ function options_field_widget_error($element, $error, $form, &$form_state) {
  */
 function theme_options_none($variables) {
   $instance = $variables['instance'];
+  $option = $variables['option'];
+
+  $output = '';
   switch ($instance['widget']['type']) {
     case 'options_buttons':
-      return t('N/A');
+      $output = t('N/A');
+      break;
+
     case 'options_select':
-      return t('- None -');
-    default :
-      return '';
+      $output = ($option == 'option_none' ? t('- None -') : t('- Select a value -'));
+      break;
   }
+
+  return $output;
 }
diff --git a/modules/field/modules/options/options.test b/modules/field/modules/options/options.test
index 8cbfd5bc7..5fa444b66 100644
--- a/modules/field/modules/options/options.test
+++ b/modules/field/modules/options/options.test
@@ -207,6 +207,7 @@ class OptionsWidgetsTestCase extends FieldTestCase {
       'field_name' => $this->card_1['field_name'],
       'entity_type' => 'test_entity',
       'bundle' => 'test_bundle',
+      'required' => TRUE,
       'widget' => array(
         'type' => 'options_select',
       ),
@@ -220,13 +221,23 @@ class OptionsWidgetsTestCase extends FieldTestCase {
     $entity->is_new = TRUE;
     field_test_entity_save($entity);
 
-    // Display form: with no field data, nothing is selected.
+    // Display form.
     $this->drupalGet('test-entity/' . $entity->ftid .'/edit');
+    // A required field without any value has a "none" option.
+    $this->assertTrue($this->xpath('//select[@id=:id]//option[@value="_none" and text()=:label]', array(':id' => 'edit-card-1-' . $langcode, ':label' => t('- Select a value -'))), t('A required select list has a "Select a value" choice.'));
+
+    // With no field data, nothing is selected.
+    $this->assertNoOptionSelected("edit-card-1-$langcode", '_none');
     $this->assertNoOptionSelected("edit-card-1-$langcode", 0);
     $this->assertNoOptionSelected("edit-card-1-$langcode", 1);
     $this->assertNoOptionSelected("edit-card-1-$langcode", 2);
     $this->assertRaw('Some dangerous & unescaped markup', t('Option text was properly filtered.'));
 
+    // Submit form: select invalid 'none' option.
+    $edit = array("card_1[$langcode]" => '_none');
+    $this->drupalPost(NULL, $edit, t('Save'));
+    $this->assertRaw(t('!title field is required.', array('!title' => $instance['field_name'])), t('Cannot save a required field when selecting "none" from the select list.'));
+
     // Submit form: select first option.
     $edit = array("card_1[$langcode]" => 0);
     $this->drupalPost(NULL, $edit, t('Save'));
@@ -234,31 +245,30 @@ class OptionsWidgetsTestCase extends FieldTestCase {
 
     // Display form: check that the right options are selected.
     $this->drupalGet('test-entity/' . $entity->ftid .'/edit');
+    // A required field with a value has no 'none' option.
+    $this->assertFalse($this->xpath('//select[@id=:id]//option[@value="_none"]', array(':id' => 'edit-card-1-' . $langcode)), t('A required select list with an actual value has no "none" choice.'));
     $this->assertOptionSelected("edit-card-1-$langcode", 0);
     $this->assertNoOptionSelected("edit-card-1-$langcode", 1);
     $this->assertNoOptionSelected("edit-card-1-$langcode", 2);
 
+    // Make the field non required.
+    $instance['required'] = FALSE;
+    field_update_instance($instance);
+
+    // Display form.
+    $this->drupalGet('test-entity/' . $entity->ftid .'/edit');
+    // A non-required field has a 'none' option.
+    $this->assertTrue($this->xpath('//select[@id=:id]//option[@value="_none" and text()=:label]', array(':id' => 'edit-card-1-' . $langcode, ':label' => t('- None -'))), t('A non-required select list has a "None" choice.'));
     // Submit form: Unselect the option.
     $edit = array("card_1[$langcode]" => '_none');
     $this->drupalPost('test-entity/' . $entity->ftid .'/edit', $edit, t('Save'));
     $this->assertFieldValues($entity_init, 'card_1', $langcode, array());
 
-    // A required select list does not have an empty key.
-    $instance['required'] = TRUE;
-    field_update_instance($instance);
-    $this->drupalGet('test-entity/' . $entity->ftid .'/edit');
-    $this->assertFalse($this->xpath('//select[@id=:id]//option[@value=""]', array(':id' => 'edit-card-1-' . $langcode)), t('A required select list does not have an empty key.'));
-
-    // We do not have to test that a required select list with one option is
-    // auto-selected because the browser does it for us.
-
     // Test optgroups.
 
     $this->card_1['settings']['allowed_values'] = NULL;
     $this->card_1['settings']['allowed_values_function'] = 'list_test_allowed_values_callback';
     field_update_field($this->card_1);
-    $instance['required'] = FALSE;
-    field_update_instance($instance);
 
     // Display form: with no field data, nothing is selected
     $this->drupalGet('test-entity/' . $entity->ftid .'/edit');