Index: includes/form.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/form.inc,v retrieving revision 1.506 diff -u -p -r1.506 form.inc --- includes/form.inc 21 Oct 2010 20:46:58 -0000 1.506 +++ includes/form.inc 27 Oct 2010 23:42:23 -0000 @@ -2073,11 +2073,29 @@ function form_type_image_button_value($f * for this element. Return nothing to use the default. */ function form_type_checkbox_value($element, $input = FALSE) { - if ($input !== FALSE) { - // 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 integer 0, so we can explicitly - // test for a value different than string '0'. + if ($input === FALSE) { + // Use #default_value as the default value of a checkbox, except change + // NULL to 0, because _form_builder_handle_input_element() would otherwise + // replace NULL with empty string, but an empty string is a potentially + // valid value for a checked checkbox. + return isset($element['#default_value']) ? $element['#default_value'] : 0; + } + else { + // Checked checkboxes are submitted with a value (possibly '0' or ''): + // http://www.w3.org/TR/html401/interact/forms.html#successful-controls. + // For checked checkboxes, browsers submit the string version of + // #return_value, but we return the original #return_value. For unchecked + // checkboxes, browsers submit nothing at all, but + // _form_builder_handle_input_element() detects this, and calls this + // function with $input=NULL. Returning NULL from a value callback means to + // use the default value, which is not what is wanted when an unchecked + // checkbox is submitted, so we use integer 0 as the value indicating an + // unchecked checkbox. Therefore, modules must not use integer 0 as a + // #return_value, as doing so results in the checkbox always being treated + // as unchecked. The string '0' is allowed for #return_value. The most + // common use-case for setting #return_value to either 0 or '0' is for the + // first option within a 0-indexed array of checkboxes, and for this, + // form_process_checkboxes() uses the string rather than the integer. return isset($input) ? $element['#return_value'] : 0; } } @@ -2109,7 +2127,7 @@ function form_type_checkboxes_value($ele // NULL elements from the array before constructing the return value, to // simulate the behavior of web browsers (which do not send unchecked // checkboxes to the server at all). This will not affect non-programmatic - // form submissions, since a checkbox can never legitimately be NULL. + // form submissions, since all values in $_POST are strings. foreach ($input as $key => $value) { if (!isset($value)) { unset($input[$key]); @@ -2807,7 +2825,7 @@ function theme_checkbox($variables) { element_set_attributes($element, array('id', 'name', '#return_value' => 'value')); // Unchecked checkbox has #value of integer 0. - if (isset($element['#return_value']) && isset($element['#value']) && $element['#value'] !== 0 && $element['#value'] == $element['#return_value']) { + if (!empty($element['#checked'])) { $element['#attributes']['checked'] = 'checked'; } _form_set_class($element, array('form-checkbox')); @@ -2859,6 +2877,33 @@ function form_pre_render_conditional_for return $element; } +/** + * Sets the #checked property of a checkbox element. + */ +function form_process_checkbox($element, $form_state) { + $value = $element['#value']; + $return_value = $element['#return_value']; + // On form submission, the #value of an available and enabled checked + // checkbox is #return_value, and the #value of an available and enabled + // unchecked checkbox is integer 0. On not submitted forms, and for + // checkboxes with #access=FALSE or #disabled=TRUE, the #value is + // #default_value (integer 0 if #default_value is NULL). Most of the time, + // a string comparison of #value and #return_value is sufficient for + // determining the "checked" state, but a value of TRUE always means checked + // (even if #return_value is 'foo'), and a value of FALSE or integer 0 always + // means unchecked (even if #return_value is '' or '0'). + if ($value === TRUE || $value === FALSE || $value === 0) { + $element['#checked'] = (bool) $value; + } + else { + // Compare as strings, so that 15 is not considered equal to '15foo', but 1 + // is considered equal to '1'. This cast does not imply that either #value + // or #return_value is expected to be a string. + $element['#checked'] = ((string) $value === (string) $return_value); + } + return $element; +} + function form_process_checkboxes($element) { $value = is_array($element['#value']) ? $element['#value'] : array(); $element['#tree'] = TRUE; @@ -2868,6 +2913,13 @@ function form_process_checkboxes($elemen } foreach ($element['#options'] as $key => $choice) { if (!isset($element[$key])) { + // Integer 0 is not a valid #return_value, so use '0' instead. + // @see form_type_checkbox_value(). + // @todo For Drupal 8, cast all integer keys to strings for consistency + // with form_process_radios(). + if ($key === 0) { + $key = '0'; + } $element[$key] = array( '#type' => 'checkbox', '#processed' => TRUE, Index: modules/field/modules/options/options.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/options/options.module,v retrieving revision 1.27 diff -u -p -r1.27 options.module --- modules/field/modules/options/options.module 17 Aug 2010 21:48:39 -0000 1.27 +++ modules/field/modules/options/options.module 27 Oct 2010 23:42:24 -0000 @@ -152,7 +152,6 @@ function options_field_widget_validate($ */ function _options_properties($type, $multiple, $required, $has_value) { $base = array( - 'zero_placeholder' => FALSE, 'filter_xss' => FALSE, 'strip_tags' => FALSE, 'empty_option' => FALSE, @@ -190,9 +189,6 @@ function _options_properties($type, $mul case 'buttons': $properties = array( 'filter_xss' => TRUE, - // Form API 'checkboxes' do not suport 0 as an option, so we replace it with - // a placeholder within the form workflow. - 'zero_placeholder' => $multiple, ); // Add a 'none' option for non-required radio buttons. if (!$required && !$multiple) { @@ -238,18 +234,6 @@ function _options_get_options($field, $i * The function is recursive to support optgroups. */ function _options_prepare_options(&$options, $properties) { - // Substitute the '_0' placeholder. - if ($properties['zero_placeholder']) { - $values = array_keys($options); - $labels = array_values($options); - // Use a strict comparison, because 0 == 'any string'. - $index = array_search(0, $values, TRUE); - if ($index !== FALSE && !is_array($options[$index])) { - $values[$index] = '_0'; - $options = array_combine($values, $labels); - } - } - foreach ($options as $value => $label) { // Recurse for optgroups. if (is_array($label)) { @@ -273,14 +257,6 @@ function _options_storage_to_form($items $items_transposed = options_array_transpose($items); $values = (isset($items_transposed[$column]) && is_array($items_transposed[$column])) ? $items_transposed[$column] : array(); - // Substitute the '_0' placeholder. - if ($properties['zero_placeholder']) { - $index = array_search('0', $values); - if ($index !== FALSE) { - $values[$index] = '_0'; - } - } - // Discard values that are not in the current list of options. Flatten the // array if needed. if ($properties['optgroups']) { @@ -302,14 +278,6 @@ function _options_form_to_storage($eleme $values = array($values[0] ? $element['#on_value'] : $element['#off_value']); } - // Substitute the '_0' placeholder. - if ($properties['zero_placeholder']) { - $index = array_search('_0', $values); - if ($index !== FALSE) { - $values[$index] = 0; - } - } - // Filter out the 'none' option. Use a strict comparison, because // 0 == 'any string'. if ($properties['empty_option']) { Index: modules/field/modules/options/options.test =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/options/options.test,v retrieving revision 1.17 diff -u -p -r1.17 options.test --- modules/field/modules/options/options.test 24 Sep 2010 00:37:42 -0000 1.17 +++ modules/field/modules/options/options.test 27 Oct 2010 23:42:24 -0000 @@ -112,9 +112,6 @@ class OptionsWidgetsTestCase extends Fie * Tests the 'options_buttons' widget (multiple select). */ function testCheckBoxes() { - // Checkboxes do not support '0' as an option, the widget internally - // replaces it with '_0'. - // Create an instance of the 'multiple values' field. $instance = array( 'field_name' => $this->card_2['field_name'], @@ -142,7 +139,7 @@ class OptionsWidgetsTestCase extends Fie // Submit form: select first and third options. $edit = array( - "card_2[$langcode][_0]" => TRUE, + "card_2[$langcode][0]" => TRUE, "card_2[$langcode][1]" => FALSE, "card_2[$langcode][2]" => TRUE, ); @@ -157,7 +154,7 @@ class OptionsWidgetsTestCase extends Fie // Submit form: select only first option. $edit = array( - "card_2[$langcode][_0]" => TRUE, + "card_2[$langcode][0]" => TRUE, "card_2[$langcode][1]" => FALSE, "card_2[$langcode][2]" => FALSE, ); @@ -172,7 +169,7 @@ class OptionsWidgetsTestCase extends Fie // Submit form: select the three options while the field accepts only 2. $edit = array( - "card_2[$langcode][_0]" => TRUE, + "card_2[$langcode][0]" => TRUE, "card_2[$langcode][1]" => TRUE, "card_2[$langcode][2]" => TRUE, ); @@ -181,7 +178,7 @@ class OptionsWidgetsTestCase extends Fie // Submit form: uncheck all options. $edit = array( - "card_2[$langcode][_0]" => FALSE, + "card_2[$langcode][0]" => FALSE, "card_2[$langcode][1]" => FALSE, "card_2[$langcode][2]" => FALSE, ); Index: modules/simpletest/tests/form.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form.test,v retrieving revision 1.72 diff -u -p -r1.72 form.test --- modules/simpletest/tests/form.test 4 Oct 2010 18:00:46 -0000 1.72 +++ modules/simpletest/tests/form.test 27 Oct 2010 23:42:26 -0000 @@ -1352,3 +1352,84 @@ class FormsFileInclusionTestCase extends $this->assertText('Submit callback called.'); } } + +/** + * Tests checkbox element. + */ +class FormCheckboxTestCase extends DrupalWebTestCase { + + public static function getInfo() { + return array( + 'name' => 'Form API checkbox', + 'description' => 'Tests form API checkbox handling of various combinations of #default_value and #return_value.', + 'group' => 'Form API', + ); + } + + function setUp() { + parent::setUp('form_test'); + } + + function testFormCheckbox() { + // Ensure that the checked state is determined and rendered correctly for + // tricky combinations of default and return values. + foreach (array(FALSE, NULL, TRUE, 0, '0', '', 1, '1', 'foobar', '1foobar') as $default_value) { + // Only values that can be used for array indeces are supported for + // #return_value, with the exception of integer 0, which is not supported. + // @see form_process_checkbox(). + foreach (array('0', '', 1, '1', 'foobar', '1foobar') as $return_value) { + $form_array = drupal_get_form('form_test_checkbox_type_juggling', $default_value, $return_value); + $form = drupal_render($form_array); + if ($default_value === TRUE) { + $checked = TRUE; + } + elseif ($return_value === '0') { + $checked = ($default_value === '0'); + } + elseif ($return_value === '') { + $checked = ($default_value === ''); + } + elseif ($return_value === 1 || $return_value === '1') { + $checked = ($default_value === 1 || $default_value === '1'); + } + elseif ($return_value === 'foobar') { + $checked = ($default_value === 'foobar'); + } + elseif ($return_value === '1foobar') { + $checked = ($default_value === '1foobar'); + } + $checked_in_html = strpos($form, 'checked') !== FALSE; + $message = t('#default_value is %default_value #return_value is %return_value.', array('%default_value' => var_export($default_value, TRUE), '%return_value' => var_export($return_value, TRUE))); + $this->assertIdentical($checked, $checked_in_html, $message); + } + } + + // Ensure that $form_state['values'] is populated correctly for a checkboxes + // group that includes a 0-indexed array of options. + $results = json_decode($this->drupalPost('form-test/checkboxes-zero', array(), 'Save')); + $this->assertIdentical($results->checkbox_off, array(0, 0, 0), t('All three in checkbox_off are zeroes: off.')); + $this->assertIdentical($results->checkbox_zero_default, array('0', 0, 0), t('The first choice is on in checkbox_zero_default')); + $this->assertIdentical($results->checkbox_string_zero_default, array('0', 0, 0), t('The first choice is on in checkbox_string_zero_default')); + $edit = array('checkbox_off[0]' => '0'); + $results = json_decode($this->drupalPost('form-test/checkboxes-zero', $edit, 'Save')); + $this->assertIdentical($results->checkbox_off, array('0', 0, 0), t('The first choice is on in checkbox_off but the rest is not')); + + // Ensure that each checkbox is rendered correctly for a checkboxes group + // that includes a 0-indexed array of options. + $this->drupalPost('form-test/checkboxes-zero/0', array(), 'Save'); + $checkboxes = $this->xpath('//input[@type="checkbox"]'); + foreach ($checkboxes as $checkbox) { + $checked = isset($checkbox['checked']); + $name = (string) $checkbox['name']; + $this->assertIdentical($checked, $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]', t('Checkbox %name correctly checked', array('%name' => $name))); + } + $edit = array('checkbox_off[0]' => '0'); + $this->drupalPost('form-test/checkboxes-zero/0', $edit, 'Save'); + $checkboxes = $this->xpath('//input[@type="checkbox"]'); + foreach ($checkboxes as $checkbox) { + $checked = isset($checkbox['checked']); + $name = (string) $checkbox['name']; + $this->assertIdentical($checked, $name == 'checkbox_off[0]' || $name == 'checkbox_zero_default[0]' || $name == 'checkbox_string_zero_default[0]', t('Checkbox %name correctly checked', array('%name' => $name))); + } + } +} Index: modules/simpletest/tests/form_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form_test.module,v retrieving revision 1.52 diff -u -p -r1.52 form_test.module --- modules/simpletest/tests/form_test.module 20 Oct 2010 01:15:58 -0000 1.52 +++ modules/simpletest/tests/form_test.module 27 Oct 2010 23:42:26 -0000 @@ -175,6 +175,13 @@ function form_test_menu() { 'access callback' => TRUE, 'type' => MENU_CALLBACK, ); + $items['form-test/checkboxes-zero'] = array( + 'title' => 'FAPI test involving checkboxes and zero', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('form_test_checkboxes_zero'), + 'access callback' => TRUE, + 'type' => MENU_CALLBACK, + ); return $items; } @@ -1395,3 +1402,44 @@ function form_test_load_include_custom($ $form_state['cache'] = TRUE; return $form; } + +function form_test_checkbox_type_juggling($form, $form_state, $default_value, $return_value) { + $form['checkbox'] = array( + '#type' => 'checkbox', + '#return_value' => $return_value, + '#default_value' => $default_value, + ); + return $form; +} + +function form_test_checkboxes_zero($form, &$form_state, $json = TRUE) { + $form['checkbox_off'] = array( + '#type' => 'checkboxes', + '#options' => array('foo', 'bar', 'baz'), + ); + $form['checkbox_zero_default'] = array( + '#type' => 'checkboxes', + '#options' => array('foo', 'bar', 'baz'), + '#default_value' => array(0), + ); + $form['checkbox_string_zero_default'] = array( + '#type' => 'checkboxes', + '#options' => array('foo', 'bar', 'baz'), + '#default_value' => array('0'), + ); + $form['submit'] = array( + '#type' => 'submit', + '#value' => 'Save', + ); + if ($json) { + $form['#submit'][] = '_form_test_checkbox_submit'; + } + else { + $form['#submit'][] = '_form_test_checkboxes_zero_no_redirect'; + } + return $form; +} + +function _form_test_checkboxes_zero_no_redirect($form, &$form_state) { + $form_state['redirect'] = FALSE; +} Index: modules/system/system.module =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.module,v retrieving revision 1.983 diff -u -p -r1.983 system.module --- modules/system/system.module 21 Oct 2010 12:09:41 -0000 1.983 +++ modules/system/system.module 27 Oct 2010 23:42:28 -0000 @@ -408,8 +408,8 @@ function system_element_info() { $types['checkbox'] = array( '#input' => TRUE, '#return_value' => 1, - '#process' => array('ajax_process_form'), '#theme' => 'checkbox', + '#process' => array('form_process_checkbox', 'ajax_process_form'), '#theme_wrappers' => array('form_element'), '#title_display' => 'after', );