From 7fb94bda0d2c8e05edbfb02f335a5f27e0af780f Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Sun, 15 Nov 2009 21:36:06 +0000 Subject: [PATCH] #179932 by Damien Tournoud, jromine, kenorb, deviantintegral, sun: Fixed Required radios/checkboxes are not validated. --- includes/form.inc | 25 +++++-- modules/simpletest/tests/form.test | 76 +++++++++++---------- modules/simpletest/tests/form_test.module | 83 +++++++++++++++++++++++ 3 files changed, 143 insertions(+), 41 deletions(-) diff --git a/includes/form.inc b/includes/form.inc index 0e0460f2b99..6f7636c952e 100644 --- a/includes/form.inc +++ b/includes/form.inc @@ -831,7 +831,9 @@ function _form_validate($elements, &$form_state, $form_id = NULL) { // A simple call to empty() will not cut it here as some fields, like // checkboxes, can return a valid value of '0'. Instead, check the // length if it's a string, and the item count if it's an array. - if ($elements['#required'] && (!count($elements['#value']) || (is_string($elements['#value']) && strlen(trim($elements['#value'])) == 0))) { + // An unchecked checkbox has a #value of numeric 0, different than string + // '0', which could be a valid value. + if ($elements['#required'] && (!count($elements['#value']) || (is_string($elements['#value']) && strlen(trim($elements['#value'])) == 0) || $elements['#value'] === 0)) { form_error($elements, $t('!name field is required.', array('!name' => $elements['#title']))); } @@ -1408,9 +1410,15 @@ function form_type_image_button_value($form, $input, $form_state) { function form_type_checkbox_value($element, $input = FALSE) { if ($input !== FALSE) { if (empty($element['#disabled'])) { - return !empty($input) ? $element['#return_value'] : 0; + // Successful (checked) checkboxes are present with a value (possibly '0'). + // http://www.w3.org/TR/html401/interact/forms.html#successful-controls + // For an unchecked checkbox, we return numeric 0, so we can explicitly + // test for a value different than string '0'. + return isset($input) ? $element['#return_value'] : 0; } else { + // Disabled form controls are not submitted by the browser. Ignore any + // submitted value and always return default. return $element['#default_value']; } } @@ -2098,17 +2106,22 @@ function theme_text_format_wrapper($variables) { */ function theme_checkbox($variables) { $element = $variables['element']; + $t = get_t(); _form_set_class($element, array('form-checkbox')); $checkbox = ''; if (!is_null($element['#title'])) { - $checkbox = ''; + $required = !empty($element['#required']) ? ' *' : ''; + $checkbox = ''; } return $checkbox; @@ -2165,7 +2178,7 @@ function form_process_checkboxes($element) { '#processed' => TRUE, '#title' => $choice, '#return_value' => $key, - '#default_value' => isset($value[$key]), + '#default_value' => isset($value[$key]) ? $key : NULL, '#attributes' => $element['#attributes'], '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, ); @@ -2319,7 +2332,7 @@ function form_process_tableselect($element) { '#type' => 'checkbox', '#title' => '', '#return_value' => $key, - '#default_value' => isset($value[$key]), + '#default_value' => isset($value[$key]) ? $key : NULL, '#attributes' => $element['#attributes'], '#ajax' => isset($element['#ajax']) ? $element['#ajax'] : NULL, ); diff --git a/modules/simpletest/tests/form.test b/modules/simpletest/tests/form.test index c1271c45961..895084f6b3f 100644 --- a/modules/simpletest/tests/form.test +++ b/modules/simpletest/tests/form.test @@ -10,22 +10,30 @@ class FormsTestCase extends DrupalWebTestCase { public static function getInfo() { return array( - 'name' => 'Required field validation', - 'description' => 'Carriage returns, tabs, and spaces are not valid content for a required field.', + 'name' => 'Form element validation', + 'description' => 'Tests various form element validation mechanisms.', 'group' => 'Form API', ); } + function setUp() { + parent::setUp('form_test'); + } + /** * Check several empty values for required forms elements. * + * Carriage returns, tabs, spaces, and unchecked checkbox elements are not + * valid content for a required field. + * * If the form field is found in form_get_errors() then the test pass. */ function testRequiredFields() { // Originates from http://drupal.org/node/117748 - // Sets of empty strings and arrays + // Sets of empty strings and arrays. $empty_strings = array('""' => "", '"\n"' => "\n", '" "' => " ", '"\t"' => "\t", '" \n\t "' => " \n\t ", '"\n\n\n\n\n"' => "\n\n\n\n\n"); $empty_arrays = array('array()' => array()); + $empty_checkbox = array(NULL); $elements['textfield']['element'] = array('#title' => $this->randomName(), '#type' => 'textfield'); $elements['textfield']['empty_values'] = $empty_strings; @@ -42,6 +50,9 @@ class FormsTestCase extends DrupalWebTestCase { $elements['radios']['element'] = array('#title' => $this->randomName(), '#type' => 'radios', '#options' => array($this->randomName(), $this->randomName(), $this->randomName())); $elements['radios']['empty_values'] = $empty_arrays; + $elements['checkbox']['element'] = array('#title' => $this->randomName(), '#type' => 'checkbox', '#required' => TRUE, '#title' => $this->randomName()); + $elements['checkbox']['empty_values'] = $empty_checkbox; + $elements['checkboxes']['element'] = array('#title' => $this->randomName(), '#type' => 'checkboxes', '#options' => array($this->randomName(), $this->randomName(), $this->randomName())); $elements['checkboxes']['empty_values'] = $empty_arrays; @@ -54,7 +65,7 @@ class FormsTestCase extends DrupalWebTestCase { // Regular expression to find the expected marker on required elements. $required_marker_preg = '@\*@'; - // Go through all the elements and all the empty values for them + // Go through all the elements and all the empty values for them. foreach ($elements as $type => $data) { foreach ($data['empty_values'] as $key => $empty) { foreach (array(TRUE, FALSE) as $required) { @@ -107,42 +118,37 @@ class FormsTestCase extends DrupalWebTestCase { // Clear the expected form error messages so they don't appear as exceptions. drupal_get_messages(); } -} - -/** - * Test form type functions for expected behavior. - */ -class FormsTestTypeCase extends DrupalUnitTestCase { - public static function getInfo() { - return array( - 'name' => 'Form type-specific tests', - 'description' => 'Test form type functions for expected behavior.', - 'group' => 'Form API', - ); - } /** - * Test form_type_checkbox_value() function for expected behavior. + * Test default value handling for checkboxes. + * + * @see _form_test_checkbox(). */ - function testFormCheckboxValue() { - $form['#return_value'] = $return_value = $this->randomName(); - $form['#default_value'] = $default_value = $this->randomName(); - // Element is disabled , and $edit is not empty. - $form['#disabled'] = TRUE; - $edit = array(1); - $this->assertEqual(form_type_checkbox_value($form, $edit), $default_value, t('form_type_checkbox_value() returns the default value when #disabled is set.')); - - // Element is not disabled, $edit is not empty. - unset($form['#disabled']); - $this->assertEqual(form_type_checkbox_value($form, $edit), $return_value, t('form_type_checkbox_value() returns the return value when #disabled is not set.')); - - // Element is not disabled, $edit is empty. + function testCheckboxProcessing() { + // First, try to submit without the required checkbox. $edit = array(); - $this->assertIdentical(form_type_checkbox_value($form, $edit), 0, t('form_type_checkbox_value() returns 0 when #disabled is not set, and $edit is empty.')); + $this->drupalPost('form-test/checkbox', $edit, t('Submit')); + $this->assertRaw(t('!name field is required.', array('!name' => 'required_checkbox')), t('A required checkbox is actually mandatory')); - // $edit is FALSE. - $edit = FALSE; - $this->assertNull(form_type_checkbox_value($form, $edit), t('form_type_checkbox_value() returns NULL when $edit is FALSE')); + // Now try to submit the form correctly. + $this->drupalPost(NULL, array('required_checkbox' => 1), t('Submit')); + + $values = json_decode($this->drupalGetContent(), TRUE); + $expected_values = array( + 'disabled_checkbox_on' => 'disabled_checkbox_on', + 'disabled_checkbox_off' => '', + 'checkbox_on' => 'checkbox_on', + 'checkbox_off' => '', + 'zero_checkbox_on' => '0', + 'zero_checkbox_off' => '', + ); + foreach ($expected_values as $widget => $expected_value) { + $this->assertEqual($values[$widget], $expected_value, t('Checkbox %widget returns expected value (expected: %expected, got: %value)', array( + '%widget' => var_export($widget, TRUE), + '%expected' => var_export($expected_value, TRUE), + '%value' => var_export($values[$widget], TRUE), + ))); + } } } diff --git a/modules/simpletest/tests/form_test.module b/modules/simpletest/tests/form_test.module index ffc777c421c..efd26b77340 100644 --- a/modules/simpletest/tests/form_test.module +++ b/modules/simpletest/tests/form_test.module @@ -70,6 +70,14 @@ function form_test_menu() { 'type' => MENU_CALLBACK, ); + $items['form-test/checkbox'] = array( + 'title' => t('Form test'), + 'page callback' => 'drupal_get_form', + 'page arguments' => array('_form_test_checkbox'), + 'access callback' => TRUE, + 'type' => MENU_CALLBACK, + ); + return $items; } @@ -393,3 +401,78 @@ function form_test_form_state_values_clean_form_submit($form, &$form_state) { drupal_json_output($form_state['values']); exit; } + +/** + * Build a form to test a checkbox. + */ +function _form_test_checkbox($form, &$form_state) { + // A required checkbox. + $form['required_checkbox'] = array( + '#type' => 'checkbox', + '#required' => TRUE, + '#title' => 'required_checkbox', + ); + + // A disabled checkbox should get its default value back. + $form['disabled_checkbox_on'] = array( + '#type' => 'checkbox', + '#disabled' => TRUE, + '#return_value' => 'disabled_checkbox_on', + '#default_value' => 'disabled_checkbox_on', + '#title' => 'disabled_checkbox_on', + ); + $form['disabled_checkbox_off'] = array( + '#type' => 'checkbox', + '#disabled' => TRUE, + '#return_value' => 'disabled_checkbox_off', + '#default_value' => NULL, + '#title' => 'disabled_checkbox_off', + ); + + // A checkbox is active when #default_value == #return_value. + $form['checkbox_on'] = array( + '#type' => 'checkbox', + '#return_value' => 'checkbox_on', + '#default_value' => 'checkbox_on', + '#title' => 'checkbox_on', + ); + + // But inactive in any other case. + $form['checkbox_off'] = array( + '#type' => 'checkbox', + '#return_value' => 'checkbox_off', + '#default_value' => 'checkbox_on', + '#title' => 'checkbox_off', + ); + + // Checkboxes with a #return_value of '0' are supported. + $form['zero_checkbox_on'] = array( + '#type' => 'checkbox', + '#return_value' => '0', + '#default_value' => '0', + '#title' => 'zero_checkbox_on', + ); + + // In that case, passing a #default_value != '0' means that the checkbox is off. + $form['zero_checkbox_off'] = array( + '#type' => 'checkbox', + '#return_value' => '0', + '#default_value' => '1', + '#title' => 'zero_checkbox_off', + ); + + $form['submit'] = array( + '#type' => 'submit', + '#value' => t('Submit') + ); + + return $form; +} + +/** + * Return the form values via JSON. + */ +function _form_test_checkbox_submit($form, &$form_state) { + drupal_json_output($form_state['values']); + exit(); +}