=== modified file 'includes/form.inc' --- includes/form.inc 2010-10-21 20:46:58 +0000 +++ includes/form.inc 2010-10-25 17:58:37 +0000 @@ -163,7 +163,7 @@ * Any additional arguments are passed on to the functions called by * drupal_get_form(), including the unique form constructor function. For * example, the node_edit form requires that a node object is passed in here - * when it is called. These are available to implementations of + * when it is called. These are available to implementations of * hook_form_alter() and hook_form_FORM_ID_alter() as the array * $form_state['build_info']['args']. * @@ -2109,7 +2109,8 @@ 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 so they can + // never legitimately be NULL. foreach ($input as $key => $value) { if (!isset($value)) { unset($input[$key]); @@ -2790,6 +2791,33 @@ function form_process_radios($element) { } /** + * Initialize #checked on checkbox elements and XSS fix #return_value. + */ +function form_pre_render_checkbox($element) { + // On form submission, the #value of a checked checkbox element is + // #return_value. On form submission, the #value of an unchecked checkbox + // element is 0. On not submitted forms, the #value of a checkbox is + // #default_value. + $value = $element['#value']; + // Most of the time, a relaxed equals check is adequate so that a #value of + // '1' matches #return_value 1. The isset in form_process_checkboxes() is + // type agnostic too so that won't interfere with this. There are a few + // exemptions: TRUE and FALSE simply means that the checkbox should be + // checked / unchecked, regardless of return value. The number zero also + // means unchecked. + if (is_bool($value) || $value === 0) { + $element['#checked'] = (bool) $value; + } + else { + $element['#checked'] = ($value == $element['#return_value']); + } + // As this ends up as the value of the HTML attribute 'value' it needs to be + // HTML escaped. + $element['#return_value'] = check_plain($element['#return_value']); + return $element; +} + +/** * Returns HTML for a checkbox form element. * * @param $variables @@ -2807,7 +2835,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 ($element['#checked']) { $element['#attributes']['checked'] = 'checked'; } _form_set_class($element, array('form-checkbox')); === modified file 'modules/simpletest/tests/common.test' --- modules/simpletest/tests/common.test 2010-10-08 15:36:12 +0000 +++ modules/simpletest/tests/common.test 2010-10-25 09:25:13 +0000 @@ -1533,6 +1533,7 @@ class DrupalRenderTestCase extends Drupa $element = array( '#type' => 'checkbox', + '#value' => TRUE, '#title' => $this->randomName(), ); $this->assertRenderedElement($element, '//input[@type=:type]', array(':type' => 'checkbox')); === modified file 'modules/simpletest/tests/form.test' --- modules/simpletest/tests/form.test 2010-10-04 18:00:45 +0000 +++ modules/simpletest/tests/form.test 2010-10-25 06:32:52 +0000 @@ -1352,3 +1352,53 @@ class FormsFileInclusionTestCase extends $this->assertText('Submit callback called.'); } } + +/** + * Test checkbox element. + */ +class FormCheckboxTestCase extends DrupalWebTestCase { + + public static function getInfo() { + return array( + 'name' => 'Form API checkbox', + 'description' => 'Tests form API checkbox handling especially 0, empty string etc.', + 'group' => 'Form API', + ); + } + + function setUp() { + parent::setUp('form_test'); + } + + function testFormCheckbox() { + $default_value_array = array(0, FALSE, NULL, TRUE, '0', '', 1, '1', 'foobar'); + $return_value_array = array('0', '', 1, '1', 'foobar'); + foreach ($default_value_array as $default_value_key => $default_value) { + foreach ($return_value_array as $return_value) { + // This case is broken in Drupal 7. TODO: fix in Drupal 8. + if ($default_value === NULL && $return_value === '') { + continue; + } + $form_array = drupal_get_form('form_test_checkbox_type_juggling', $default_value, $return_value); + $form = drupal_render($form_array); + if ($default_value_key < 3) { + $checked = FALSE; + } + elseif ($default_value_key == 3) { + $checked = TRUE; + } + else { + if ($return_value === '0' || $return_value === '') { + $checked = $default_value === $return_value; + } + else { + $checked = $default_value == $return_value; + } + } + $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); + } + } + } +} === modified file 'modules/simpletest/tests/form_test.module' --- modules/simpletest/tests/form_test.module 2010-10-20 01:15:58 +0000 +++ modules/simpletest/tests/form_test.module 2010-10-25 06:39:59 +0000 @@ -1395,3 +1395,12 @@ 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; +} === modified file 'modules/system/system.module' --- modules/system/system.module 2010-10-21 12:09:41 +0000 +++ modules/system/system.module 2010-10-25 06:35:26 +0000 @@ -407,9 +407,12 @@ function system_element_info() { ); $types['checkbox'] = array( '#input' => TRUE, + // Unchecked checkbox has a #default_value 0. + '#default_value' => 0, '#return_value' => 1, '#process' => array('ajax_process_form'), '#theme' => 'checkbox', + '#pre_render' => array('form_pre_render_checkbox'), '#theme_wrappers' => array('form_element'), '#title_display' => 'after', ); @@ -2883,7 +2886,7 @@ function system_get_module_admin_tasks($ } if ($parent = menu_link_load($admin_tasks[$duplicate_path]['plid'])) { // Append the parent item's title to the duplicated task's title. - // We use $links[$duplicate_path] in case there are triplicates. + // We use $links[$duplicate_path] in case there are triplicates. $admin_tasks[$duplicate_path]['title'] = t('@original_title (@parent_title)', array('@original_title' => $links[$duplicate_path]['title'], '@parent_title' => $parent['title'])); } }