Issue #811542 by bojanz, sun, mlncn, chx, pillarsdotnet, loganfsmyth, xjm: Fixed Regression: Required radios throw illegal choice error when none selected.

5 files changed, 218 insertions, 2 deletions

diff --git a/includes/form.inc b/includes/form.inc
index dfac67d8b..ea9afbe15 100644
--- a/includes/form.inc
+++ b/includes/form.inc
@@ -2325,6 +2325,48 @@ function form_type_tableselect_value($element, $input = FALSE) {
 }
 
 /**
+ * Form value callback: Determines the value for a #type radios form element.
+ *
+ * @param $element
+ *   The form element whose value is being populated.
+ * @param $input
+ *   (optional) The incoming input to populate the form element. If FALSE, the
+ *   element's default value is returned. Defaults to FALSE.
+ *
+ * @return
+ *   The data that will appear in the $element_state['values'] collection for
+ *   this element.
+ */
+function form_type_radios_value(&$element, $input = FALSE) {
+  if ($input !== FALSE) {
+    // There may not be a submitted value for multiple radio buttons, if none of
+    // the options was checked by default. If there is no submitted input value
+    // for this element (NULL), _form_builder_handle_input_element()
+    // automatically attempts to use the #default_value (if set) or an empty
+    // string (''). However, an empty string would fail validation in
+    // _form_validate(), in case it is not contained in the list of allowed
+    // values in #options.
+    if (!isset($input)) {
+      // Signify a garbage value to disable the #default_value handling and take
+      // over NULL as #value.
+      $element['#has_garbage_value'] = TRUE;
+      // There was a user submission so validation is a must. If this element is
+      // #required, then an appropriate error message will be output. While an
+      // optional #type 'radios' does not necessarily make sense from a user
+      // interaction perspective, there may be use-cases for that and it is not
+      // the job of Form API to artificially limit possibilities.
+      $element['#needs_validation'] = TRUE;
+    }
+    // The value stays the same, but the flags above will ensure it is
+    // processed properly.
+    return $input;
+  }
+  elseif (isset($element['#default_value'])) {
+    return $element['#default_value'];
+  }
+}
+
+/**
  * Determines the value for a password_confirm form element.
  *
  * @param $element
@@ -2968,7 +3010,9 @@ function form_process_radios($element) {
         // The key is sanitized in drupal_attributes() during output from the
         // theme function.
         '#return_value' => $key,
-        '#default_value' => isset($element['#default_value']) ? $element['#default_value'] : NULL,
+        // Use default or FALSE. A value of FALSE means that the radio button is
+        // not 'checked'.
+        '#default_value' => isset($element['#default_value']) ? $element['#default_value'] : FALSE,
         '#attributes' => $element['#attributes'],
         '#parents' => $element['#parents'],
         '#id' => drupal_html_id('edit-' . implode('-', $parents_for_id)),
diff --git a/modules/field/modules/options/options.module b/modules/field/modules/options/options.module
index d4d05eca2..04b88d8f4 100644
--- a/modules/field/modules/options/options.module
+++ b/modules/field/modules/options/options.module
@@ -102,10 +102,18 @@ function options_field_widget_form(&$form, &$form_state, $field, $instance, $lan
         reset($options);
         $default_value = array(key($options));
       }
+
+      // If this is a single-value field, take the first default value, or
+      // default to NULL so that the form element is properly recognized as
+      // not having a default value.
+      if (!$multiple) {
+        $default_value = $default_value ? reset($default_value) : NULL;
+      }
+
       $element += array(
         '#type' => $multiple ? 'checkboxes' : 'radios',
         // Radio buttons need a scalar value.
-        '#default_value' => $multiple ? $default_value : reset($default_value),
+        '#default_value' => $default_value,
         '#options' => $options,
       );
       break;
diff --git a/modules/field/tests/field.test b/modules/field/tests/field.test
index df58ecbd9..f7d9dddb2 100644
--- a/modules/field/tests/field.test
+++ b/modules/field/tests/field.test
@@ -1482,6 +1482,51 @@ class FieldFormTestCase extends FieldTestCase {
     // Test with several multiple fields in a form
   }
 
+  /**
+   * Tests widget handling of multiple required radios.
+   */
+  function testFieldFormMultivalueWithRequiredRadio() {
+    // Create a multivalue test field.
+    $this->field = $this->field_unlimited;
+    $this->field_name = $this->field['field_name'];
+    $this->instance['field_name'] = $this->field_name;
+    field_create_field($this->field);
+    field_create_instance($this->instance);
+    $langcode = LANGUAGE_NONE;
+
+    // Add a required radio field.
+    field_create_field(array(
+      'field_name' => 'required_radio_test',
+      'type' => 'list_text',
+      'settings' => array(
+        'allowed_values' => array('yes' => 'yes', 'no' => 'no'),
+      ),
+    ));
+    field_create_instance(array(
+      'field_name' => 'required_radio_test',
+      'entity_type' => 'test_entity',
+      'bundle' => 'test_bundle',
+      'required' => TRUE,
+      'widget' => array(
+        'type' => 'options_buttons',
+      ),
+    ));
+
+    // Display creation form.
+    $this->drupalGet('test-entity/add/test-bundle');
+
+    // Press the 'Add more' button.
+    $this->drupalPost(NULL, array(), t('Add another item'));
+
+    // Verify that no error is thrown by the radio element.
+    $this->assertNoFieldByXpath('//div[contains(@class, "error")]', FALSE, 'No error message is displayed.');
+
+    // Verify that the widget is added.
+    $this->assertFieldByName("{$this->field_name}[$langcode][0][value]", '', 'Widget 1 is displayed');
+    $this->assertFieldByName("{$this->field_name}[$langcode][1][value]", '', 'New widget is displayed');
+    $this->assertNoField("{$this->field_name}[$langcode][2][value]", 'No extraneous widget is displayed');
+  }
+
   function testFieldFormJSAddMore() {
     $this->field = $this->field_unlimited;
     $this->field_name = $this->field['field_name'];
diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test
index 13fdca201..ec7df2c33 100644
--- a/modules/simpletest/tests/form.test
+++ b/modules/simpletest/tests/form.test
@@ -122,6 +122,72 @@ class FormsTestCase extends DrupalWebTestCase {
   }
 
   /**
+   * Tests validation for required checkbox, select, and radio elements.
+   *
+   * Submits a test form containing several types of form elements. The form
+   * is submitted twice, first without values for required fields and then
+   * with values. Each submission is checked for relevant error messages.
+   *
+   * @see form_test_validate_required_form()
+   */
+  function testRequiredCheckboxesRadio() {
+    $form = $form_state = array();
+    $form = form_test_validate_required_form($form, $form_state);
+
+    // Attempt to submit the form with no required fields set.
+    $edit = array();
+    $this->drupalPost('form-test/validate-required', $edit, 'Submit');
+
+    // The only error messages that should appear are the relevant 'required'
+    // messages for each field.
+    $expected = array();
+    foreach (array('textfield', 'checkboxes', 'select', 'radios') as $key) {
+      $expected[] = t('!name field is required.', array('!name' => $form[$key]['#title']));
+    }
+
+    // Check the page for error messages.
+    $errors = $this->xpath('//div[contains(@class, "error")]//li');
+    foreach ($errors as $error) {
+      $expected_key = array_search($error[0], $expected);
+      // If the error message is not one of the expected messages, fail.
+      if ($expected_key === FALSE) {
+        $this->fail(format_string("Unexpected error message: @error", array('@error' => $error[0])));
+      }
+      // Remove the expected message from the list once it is found.
+      else {
+        unset($expected[$expected_key]);
+      }
+    }
+
+    // Fail if any expected messages were not found.
+    foreach ($expected as $not_found) {
+      $this->fail(format_string("Found error message: @error", array('@error' => $not_found)));
+    }
+
+    // Verify that input elements are still empty.
+    $this->assertFieldByName('textfield', '');
+    $this->assertNoFieldChecked('edit-checkboxes-foo');
+    $this->assertNoFieldChecked('edit-checkboxes-bar');
+    $this->assertOptionSelected('edit-select', '');
+    $this->assertNoFieldChecked('edit-radios-foo');
+    $this->assertNoFieldChecked('edit-radios-bar');
+    $this->assertNoFieldChecked('edit-radios-optional-foo');
+    $this->assertNoFieldChecked('edit-radios-optional-bar');
+
+    // Submit again with required fields set and verify that there are no
+    // error messages.
+    $edit = array(
+      'textfield' => $this->randomString(),
+      'checkboxes[foo]' => TRUE,
+      'select' => 'foo',
+      'radios' => 'bar',
+    );
+    $this->drupalPost(NULL, $edit, 'Submit');
+    $this->assertNoFieldByXpath('//div[contains(@class, "error")]', FALSE, 'No error message is displayed when all required fields are filled.');
+    $this->assertRaw("The form_test_validate_required_form form was submitted successfully.", 'Validation form submitted successfully.');
+  }
+
+  /**
    * Test default value handling for checkboxes.
    *
    * @see _form_test_checkbox()
diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module
index 0a748d2df..1c16c39c7 100644
--- a/modules/simpletest/tests/form_test.module
+++ b/modules/simpletest/tests/form_test.module
@@ -23,6 +23,13 @@ function form_test_menu() {
     'access arguments' => array('access content'),
     'type' => MENU_CALLBACK,
   );
+  $items['form-test/validate-required'] = array(
+    'title' => 'Form #required validation',
+    'page callback' => 'drupal_get_form',
+    'page arguments' => array('form_test_validate_required_form'),
+    'access callback' => TRUE,
+    'type' => MENU_CALLBACK,
+  );
   $items['form-test/limit-validation-errors'] = array(
     'title' => 'Form validation with some error suppression',
     'page callback' => 'drupal_get_form',
@@ -332,6 +339,52 @@ function form_test_validate_form_validate(&$form, &$form_state) {
 }
 
 /**
+ * Form constructor to test the #required property.
+ */
+function form_test_validate_required_form($form, &$form_state) {
+  $options = drupal_map_assoc(array('foo', 'bar'));
+
+  $form['textfield'] = array(
+    '#type' => 'textfield',
+    '#title' => 'Textfield',
+    '#required' => TRUE,
+  );
+  $form['checkboxes'] = array(
+    '#type' => 'checkboxes',
+    '#title' => 'Checkboxes',
+    '#options' => $options,
+    '#required' => TRUE,
+  );
+  $form['select'] = array(
+    '#type' => 'select',
+    '#title' => 'Select',
+    '#options' => $options,
+    '#required' => TRUE,
+  );
+  $form['radios'] = array(
+    '#type' => 'radios',
+    '#title' => 'Radios',
+    '#options' => $options,
+    '#required' => TRUE,
+  );
+  $form['radios_optional'] = array(
+    '#type' => 'radios',
+    '#title' => 'Radios (optional)',
+    '#options' => $options,
+  );
+  $form['actions'] = array('#type' => 'actions');
+  $form['actions']['submit'] = array('#type' => 'submit', '#value' => 'Submit');
+  return $form;
+}
+
+/**
+ * Form submission handler for form_test_validate_required_form().
+ */
+function form_test_validate_required_form_submit($form, &$form_state) {
+  drupal_set_message('The form_test_validate_required_form form was submitted successfully.');
+}
+
+/**
  * Builds a simple form with a button triggering partial validation.
  */
 function form_test_limit_validation_errors_form($form, &$form_state) {