Index: includes/common.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/common.inc,v retrieving revision 1.1052 diff -u -p -r1.1052 common.inc --- includes/common.inc 26 Nov 2009 05:54:48 -0000 1.1052 +++ includes/common.inc 1 Dec 2009 04:49:16 -0000 @@ -5557,6 +5557,9 @@ function drupal_common_theme() { 'form_required_marker' => array( 'arguments' => array('element' => NULL), ), + 'form_element_label' => array( + 'render element' => 'element', + ), 'text_format_wrapper' => array( 'render element' => 'element', ), Index: includes/form.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/form.inc,v retrieving revision 1.410 diff -u -p -r1.410 form.inc --- includes/form.inc 1 Dec 2009 03:07:33 -0000 1.410 +++ includes/form.inc 1 Dec 2009 04:49:16 -0000 @@ -1688,7 +1688,6 @@ function form_options_flatten($array, $r */ function theme_select($variables) { $element = $variables['element']; - $select = ''; $size = $element['#size'] ? ' size="' . $element['#size'] . '"' : ''; _form_set_class($element, array('form-select')); $multiple = $element['#multiple']; @@ -1845,10 +1844,6 @@ function theme_radio($variables) { $output .= (check_plain($element['#value']) == $element['#return_value']) ? ' checked="checked" ' : ' '; $output .= drupal_attributes($element['#attributes']) . ' />'; - if (isset($element['#title'])) { - $output = ''; - } - return $output; } @@ -2198,11 +2193,6 @@ function theme_checkbox($variables) { } $checkbox .= drupal_attributes($element['#attributes']) . ' />'; - if (isset($element['#title'])) { - $required = !empty($element['#required']) ? ' *' : ''; - $checkbox = ''; - } - return $checkbox; } @@ -2833,10 +2823,32 @@ function theme_file($variables) { /** * Theme a form element. * + * Each form element is wrapped in a div with #type and #name classes. In + * addition to the element itself, the div contains a label before or after + * the element based on the optional #title_display property: + * - before: The label is output before the element. This is the default. + * The label includes the #title and the required marker, if #required. + * - after: The label is output after the element. For example, this is used + * for radio and checkbox #type elements as set in system_element_info(). + * If the #title is empty but the field is #required, the label will + * contain only the required marker. + * - attribute: Set the title attribute on the element to create a tooltip + * but output no label element. This is used where a visual label is not + * needed, such as a table of checkboxes where the row and column provide + * the context. The tooltip will include the title and required marker. + * - none: Suppress output of any label or required marker. For example, this + * is used by the password_confirm #type as set in system_element_info(). + * The password_confirm field creates children elements that have their own + * labels and required markers, but the parent element should have neither. + * The label or attribute includes a required marker for required fields for + * all cases except 'none'. After the label and fields, the last thing this + * function outputs is the optional element #description. + * * @param $variables * An associative array containing: * - element: An associative array containing the properties of the element. - * Properties used: #title, #description, #id, #required, #children + * Properties used: #title, #description, #id, #required, #children, #type, + * #name, #title_display. * * @return * A string representing the form element. @@ -2858,19 +2870,49 @@ function theme_form_element($variables) } $output = '
' . "\n"; - $required = !empty($element['#required']) ? theme('form_required_marker', array('element' => $element)) : ''; - if (!empty($element['#title']) && empty($element['#form_element_skip_title'])) { - $title = $element['#title']; - if (!empty($element['#id'])) { - $output .= ' \n"; - } - else { - $output .= ' \n"; - } + // Determine the position of the label and required marker from #title_display. + $title_display = ''; + if (isset($element['#title_display'])) { + $title_display = $element['#title_display']; + } + elseif (empty($element['#title'])) { + // If #title is empty and #title_display is not set, we still use a label + // to contain the required marker for required fields. The label element + // will use the #id to associate the required marker with the field that is + // required. This is especially important for screenreader users to know + // which field is required. The default position for a required marker with + // no title is after the field. + $title_display = 'after'; + } + else { + // Otherwise, the default position for elements with a title is before. + $title_display = 'before'; } - $output .= " " . $element['#children'] . "\n"; + switch ($title_display) { + case 'before': + $output .= ' ' . theme('form_element_label', $variables); + $output .= ' ' . $element['#children'] . "\n"; + break; + + case 'after': + $output .= ' ' . $element['#children']; + $output .= ' ' . theme('form_element_label', $variables) . "\n"; + break; + + case 'none': + case 'attribute': + // Output no label and no required marker, only the children. + $output .= ' ' . $element['#children'] . "\n"; + // Set the element's title attribute to show #title as a tooltip. + $element['#attributes']['title'] = $element['#title']; + if (!empty($element['#required'])) { + // Append an indication that this field is required into the tooltip. + $element['#attributes']['title'] .= ' (' . $t('Required') . ')'; + } + break; + } if (!empty($element['#description'])) { $output .= '
' . $element['#description'] . "
\n"; @@ -2895,6 +2937,11 @@ function theme_form_element($variables) function theme_form_required_marker($variables) { // This is also used in the installer, pre-database setup. $t = get_t(); + + if (empty($variables['element']['#required'])) { + return ''; + } + $attributes = array( 'class' => 'form-required', 'title' => $t('This field is required.'), @@ -2903,6 +2950,60 @@ function theme_form_required_marker($var } /** + * Theme a form element label and required marker. + * + * Form element labels include the #title and a #required marker. The label is + * associated with the element itself by the element #id. Labels may appear + * before or after elements, depending on theme_form_element() and #title_display. + * This function will not be called for elements with no labels, depending on + * #title_display. For elements that have an empty #title and are not required, + * this function will output no label (''). For required elements that have an + * empty #title, this will output the required marker alone within the label. + * The label will use the #id to associate the marker with the field that is + * required. That is especially important for screenreader users to know + * which field is required. + * + * @param $variables + * An associative array containing: + * - element: An associative array containing the properties of the element. + * Properties used: #required, #title, #id, #value, #description. + * @return + * A string representing the form element label. + * + * @ingroup themeable + */ +function theme_form_element_label($variables) { + $element = $variables['element']; + // This is also used in the installer, pre-database setup. + $t = get_t(); + + // If the element is required, append a required marker to the label. + $required = !empty($element['#required']) ? theme('form_required_marker', array('element' => $element)) : ''; + + $title = ''; + if (empty($element['#title'])) { + if (empty($required)) { + // If title and required marker are both empty, output no label. + return ''; + } + } + else { + $title = filter_xss_admin($element['#title']); + } + + $attributes = array(); + if (isset($element['#title_display']) && $element['#title_display'] == 'after') { + // Style the label as class option to display inline with the element. + $attributes['class'] = 'option'; + } + if (!empty($element['#id'])) { + $attributes['for'] = $element['#id']; + } + + return ' ' . $t('!title !required', array('!title' => $title, '!required' => $required)) . "\n"; +} + +/** * Sets a form element's class attribute. * * Adds 'required' and 'error' classes as needed. @@ -3150,7 +3251,7 @@ function batch_process($redirect = NULL, $batch =& batch_get(); drupal_theme_initialize(); - + if (isset($batch)) { // Add process information $process_info = array( @@ -3165,7 +3266,7 @@ function batch_process($redirect = NULL, ); $batch += $process_info; - // The batch is now completely built. Allow other modules to make changes to the + // The batch is now completely built. Allow other modules to make changes to the // batch so that it is easier to reuse batch processes in other enviroments. drupal_alter('batch', $batch); Index: modules/comment/comment-node-form.js =================================================================== RCS file: /cvs/drupal/drupal/modules/comment/comment-node-form.js,v retrieving revision 1.3 diff -u -p -r1.3 comment-node-form.js --- modules/comment/comment-node-form.js 21 Aug 2009 00:21:48 -0000 1.3 +++ modules/comment/comment-node-form.js 1 Dec 2009 04:49:16 -0000 @@ -5,7 +5,7 @@ Drupal.behaviors.commentFieldsetSummaries = { attach: function (context) { $('fieldset#edit-comment-settings', context).setSummary(function (context) { - return Drupal.checkPlain($('input:checked', context).parent().text()); + return Drupal.checkPlain($('input:checked', context).next('label').text()); }); // Provide the summary for the node type form. $('fieldset#edit-comment', context).setSummary(function(context) { @@ -15,7 +15,7 @@ Drupal.behaviors.commentFieldsetSummarie vals.push($("select[name='comment'] option:selected", context).text()); // Threading. - var threading = $("input[name='comment_default_mode']:checked", context).parent().text(); + var threading = $("input[name='comment_default_mode']:checked", context).next('label').text(); if (threading) { vals.push(threading); } Index: modules/node/content_types.js =================================================================== RCS file: /cvs/drupal/drupal/modules/node/content_types.js,v retrieving revision 1.7 diff -u -p -r1.7 content_types.js --- modules/node/content_types.js 22 Aug 2009 23:18:28 -0000 1.7 +++ modules/node/content_types.js 1 Dec 2009 04:49:17 -0000 @@ -22,7 +22,7 @@ Drupal.behaviors.contentTypes = { }); $('fieldset#edit-display', context).setSummary(function(context) { var vals = []; - $('input:checked', context).parent().each(function() { + $('input:checked', context).next('label').each(function() { vals.push(Drupal.checkPlain($(this).text())); }); if (!$('#edit-node-submitted', context).is(':checked')) { Index: modules/shortcut/shortcut.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/shortcut/shortcut.admin.inc,v retrieving revision 1.2 diff -u -p -r1.2 shortcut.admin.inc --- modules/shortcut/shortcut.admin.inc 8 Nov 2009 13:23:41 -0000 1.2 +++ modules/shortcut/shortcut.admin.inc 1 Dec 2009 04:49:17 -0000 @@ -133,25 +133,6 @@ function shortcut_set_switch_submit($for } /** - * Theme function for the form that switches shortcut sets. - * - * @param $variables - * An associative array containing: - * - form: An array representing the form. - * @return - * A themed HTML string representing the content of the form. - * - * @ingroup themeable - * @see shortcut_set_switch() - */ -function theme_shortcut_set_switch($variables) { - $form = $variables['form']; - // Render the textfield for adding a new set inline with the radio button. - $form['set']['new']['#title'] = t('New set: !textfield', array('!textfield' => drupal_render($form['new']))); - return drupal_render_children($form); -} - -/** * Menu callback; Build the form for customizing shortcut sets. * * @param $form Index: modules/shortcut/shortcut.css =================================================================== RCS file: /cvs/drupal/drupal/modules/shortcut/shortcut.css,v retrieving revision 1.2 diff -u -p -r1.2 shortcut.css --- modules/shortcut/shortcut.css 11 Nov 2009 06:56:52 -0000 1.2 +++ modules/shortcut/shortcut.css 1 Dec 2009 04:49:17 -0000 @@ -83,3 +83,12 @@ div.add-or-remove-shortcuts a:hover span -webkit-border-bottom-right-radius: 5px; } +#shortcut-set-switch .form-type-radios { + padding-bottom: 0; + margin-bottom: 0; +} + +#shortcut-set-switch .form-item-new { + padding-top: 0; + padding-left: 17px; +} Index: modules/shortcut/shortcut.module =================================================================== RCS file: /cvs/drupal/drupal/modules/shortcut/shortcut.module,v retrieving revision 1.6 diff -u -p -r1.6 shortcut.module --- modules/shortcut/shortcut.module 19 Nov 2009 03:57:15 -0000 1.6 +++ modules/shortcut/shortcut.module 1 Dec 2009 04:49:17 -0000 @@ -108,10 +108,6 @@ function shortcut_menu() { */ function shortcut_theme() { return array( - 'shortcut_set_switch' => array( - 'render element' => 'form', - 'file' => 'shortcut.admin.inc', - ), 'shortcut_set_customize' => array( 'render element' => 'form', 'file' => 'shortcut.admin.inc', Index: modules/simpletest/tests/form.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form.test,v retrieving revision 1.27 diff -u -p -r1.27 form.test --- modules/simpletest/tests/form.test 1 Dec 2009 03:07:34 -0000 1.27 +++ modules/simpletest/tests/form.test 1 Dec 2009 04:49:17 -0000 @@ -207,6 +207,74 @@ class FormValidationTestCase extends Dru } /** + * Test form element labels, required markers and associated output. + */ +class FormsElementsLabelsTestCase extends DrupalWebTestCase { + + public static function getInfo() { + return array( + 'name' => 'Form element and label output test', + 'description' => 'Test form element labels, required markers and associated output.', + 'group' => 'Form API', + ); + } + + function setUp() { + parent::setUp('form_test'); + } + + /** + * Test form elements, labels, title attibutes and required marks output + * correctly and have the correct label option class if needed. + */ + function testFormLabels() { + $this->drupalGet('form_test/form-labels'); + + // Check that the checkbox/radio processing is not interfering with + // basic placement. + $elements = $this->xpath('//input[@id="edit-form-checkboxes-test-third-checkbox"]/following-sibling::label[@for="edit-form-checkboxes-test-third-checkbox" and @class="option"]'); + $this->assertTrue(isset($elements[0]), t("Label follows field and label option class correct for regular checkboxes.")); + + $elements = $this->xpath('//input[@id="edit-form-radios-test-second-radio"]/following-sibling::label[@for="edit-form-radios-test-second-radio" and @class="option"]'); + $this->assertTrue(isset($elements[0]), t("Label follows field and label option class correct for regular radios.")); + + // Exercise various defaults for checkboxes and modifications to ensure + // appropriate override and correct behaviour. + $elements = $this->xpath('//input[@id="edit-form-checkbox-test"]/following-sibling::label[@for="edit-form-checkbox-test" and @class="option"]'); + $this->assertTrue(isset($elements[0]), t("Label follows field and label option class correct for a checkbox by default.")); + + $elements = $this->xpath('//input[@id="edit-form-checkbox-test-attribute" and @title="Checkbox test with label as attribute"]'); + $this->assertTrue(isset($elements[0]), t("Title as attribute for a checkbox is correct.")); + + $elements = $this->xpath('//input[@id="edit-form-checkbox-test-attribute"]/following-sibling::label'); + $this->assertFalse(isset($elements[0]), t("Label does not follow title as attribute for a checkbox.")); + + $elements = $this->xpath('//input[@id="edit-form-checkbox-test-attribute"]/following-sibling::label[@for="edit-form-checkbox-test-attribute" and @class="option"]'); + $this->assertFalse(isset($elements[0]), t("No label option class found for checkbox with title as attribute.")); + + // Exercise various defaults for textboxes and modifications to ensure + // appropriate override and correct behaviour. + $elements = $this->xpath('//label[@for="edit-form-textfield-test-title-and-required"]/child::span[@class="form-required"]/parent::*/following-sibling::input[@id="edit-form-textfield-test-title-and-required"]'); + $this->assertTrue(isset($elements[0]), t("Label preceeds textfield, with required marker inside label.")); + + $elements = $this->xpath('//input[@id="edit-form-textfield-test-no-title-required"]/following-sibling::label[@for="edit-form-textfield-test-no-title-required"]/span[@class="form-required"]'); + $this->assertTrue(isset($elements[0]), t("Label tag with required marker follows required textfield with no title.")); + + $elements = $this->xpath('//input[@id="edit-form-textfield-test-title"]/preceding-sibling::span[@class="form-required"]'); + $this->assertFalse(isset($elements[0]), t("No required marker on non-required field.")); + + $elements = $this->xpath('//input[@id="edit-form-textfield-test-title-attribute" and @title="Textfield test for title as attribute"]'); + $this->assertTrue(isset($elements[0]), t("Title as attribute for a textfield is correct.")); + + $elements = $this->xpath('//input[@id="edit-form-textfield-test-title-after"]/following-sibling::label[@for="edit-form-textfield-test-title-after" and @class="option"]'); + $this->assertTrue(isset($elements[0]), t("Label after field and label option class correct for text field.")); + + $elements = $this->xpath('//label[@for="edit-form-textfield-test-title-no-show"]'); + $this->assertFalse(isset($elements[0]), t("No label tag when title set not to display.")); + } +} + +/** * Test the tableselect form element for expected behavior. */ class FormsElementsTableSelectFunctionalTest extends DrupalWebTestCase { Index: modules/simpletest/tests/form_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/form_test.module,v retrieving revision 1.19 diff -u -p -r1.19 form_test.module --- modules/simpletest/tests/form_test.module 1 Dec 2009 03:07:34 -0000 1.19 +++ modules/simpletest/tests/form_test.module 1 Dec 2009 04:49:17 -0000 @@ -94,6 +94,14 @@ function form_test_menu() { 'type' => MENU_CALLBACK, ); + $items['form_test/form-labels'] = array( + 'title' => 'Form label test', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('form_label_test_form'), + 'access arguments' => array('access content'), + 'type' => MENU_CALLBACK, + ); + return $items; } @@ -474,6 +482,71 @@ function form_test_storage_form_submit($ drupal_set_message("Form constructions: ". $_SESSION['constructions']); } + /** + * A form for testing form labels and required marks. + */ +function form_label_test_form(&$form_state) { + $form['form_checkboxes_test'] = array( + '#type' => 'checkboxes', + '#title' => t('Checkboxes test'), + '#options' => array( + 'first-checkbox' => t('First checkbox'), + 'second-checkbox' => t('Second checkbox'), + 'third-checkbox' => t('Third checkbox'), + ), + ); + $form['form_radios_test'] = array( + '#type' => 'radios', + '#title' => t('Radios test'), + '#options' => array( + 'first-radio' => t('First radio'), + 'second-radio' => t('Second radio'), + 'third-radio' => t('Third radio'), + ), + ); + $form['form_checkbox_test'] = array( + '#type' => 'checkbox', + '#title' => t('Checkbox test'), + ); + $form['form_checkbox_test_attribute'] = array( + '#type' => 'checkbox', + '#title' => t('Checkbox test with label as attribute'), + '#title_display' => 'attribute', + ); + $form['form_textfield_test_title_and_required'] = array( + '#type' => 'textfield', + '#title' => t('Textfield test for required with title'), + '#required' => TRUE, + ); + $form['form_textfield_test_no_title_required'] = array( + '#type' => 'textfield', + // No title. + '#required' => TRUE, + ); + $form['form_textfield_test_title'] = array( + '#type' => 'textfield', + '#title' => t('Textfield test for title only'), + // Not required. + ); + $form['form_textfield_test_title_attribute'] = array( + '#type' => 'textfield', + '#title' => t('Textfield test for title as attribute'), + '#title_display' => 'attribute', + ); + $form['form_textfield_test_title_after'] = array( + '#type' => 'textfield', + '#title' => t('Textfield test for title after element'), + '#title_display' => 'after', + ); + $form['form_textfield_test_title_no_show'] = array( + '#type' => 'textfield', + '#title' => t('Textfield test for title set not to display'), + '#title_display' => 'none', + ); + + return $form; +} + /** * Menu callback; Invokes a form builder function with a wrapper callback. */ Index: modules/system/system.api.php =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.api.php,v retrieving revision 1.104 diff -u -p -r1.104 system.api.php --- modules/system/system.api.php 1 Dec 2009 00:39:34 -0000 1.104 +++ modules/system/system.api.php 1 Dec 2009 04:49:17 -0000 @@ -274,6 +274,8 @@ function hook_cron_queue_info() { * - "#pre_render": array of callback functions taking $element and $form_state. * - "#post_render": array of callback functions taking $element and $form_state. * - "#submit": array of callback functions taking $form and $form_state. + * - "#title_display": optional string indicating if and how #title should be + * displayed, see theme_form_element() and theme_form_element_label(). * * @see hook_element_info_alter() * @see system_element_info() Index: modules/system/system.module =================================================================== RCS file: /cvs/drupal/drupal/modules/system/system.module,v retrieving revision 1.846 diff -u -p -r1.846 system.module --- modules/system/system.module 1 Dec 2009 00:39:34 -0000 1.846 +++ modules/system/system.module 1 Dec 2009 04:49:17 -0000 @@ -56,7 +56,6 @@ define('REGIONS_VISIBLE', 'visible'); */ define('REGIONS_ALL', 'all'); - /** * Implement hook_help(). */ @@ -369,6 +368,9 @@ function system_element_info() { '#input' => TRUE, '#process' => array('form_process_password_confirm', 'user_form_process_password_confirm'), '#theme_wrappers' => array('form_element'), + // Do not output a title or required marker for password_confirm. + // The children elements will output their own titles as usual. + '#title_display' => 'none', ); $types['textarea'] = array( '#input' => TRUE, @@ -391,7 +393,7 @@ function system_element_info() { '#process' => array('ajax_process_form'), '#theme' => 'radio', '#theme_wrappers' => array('form_element'), - '#form_element_skip_title' => TRUE, + '#title_display' => 'after', ); $types['checkboxes'] = array( '#input' => TRUE, @@ -406,7 +408,7 @@ function system_element_info() { '#process' => array('ajax_process_form'), '#theme' => 'checkbox', '#theme_wrappers' => array('form_element'), - '#form_element_skip_title' => TRUE, + '#title_display' => 'after', ); $types['select'] = array( '#input' => TRUE,