From 16c888a917c889faf6d29fa6162201c9c8810f2f Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Tue, 17 Aug 2010 21:48:39 +0000 Subject: [PATCH] - Patch #735426 by yched, mr.baileys: fields with select widget: first option is selected by default even if no 'default value' set for the field. --- modules/field/modules/options/options.module | 59 +++++++++++++++----- modules/field/modules/options/options.test | 34 +++++++---- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/modules/field/modules/options/options.module b/modules/field/modules/options/options.module index 24ab04758d17..75683a9fbce2 100644 --- a/modules/field/modules/options/options.module +++ b/modules/field/modules/options/options.module @@ -25,7 +25,7 @@ function options_help($path, $arg) { function options_theme() { return array( 'options_none' => array( - 'variables' => array('instance' => NULL), + 'variables' => array('instance' => NULL, 'option' => NULL), ), ); } @@ -76,7 +76,8 @@ function options_field_widget_form(&$form, &$form_state, $field, $instance, $lan $type = str_replace('options_', '', $instance['widget']['type']); $multiple = $field['cardinality'] > 1 || $field['cardinality'] == FIELD_CARDINALITY_UNLIMITED; $required = $element['#required']; - $properties = _options_properties($type, $multiple, $required); + $has_value = isset($items[0][$value_key]); + $properties = _options_properties($type, $multiple, $required, $has_value); // Prepare the list of options. $options = _options_get_options($field, $instance, $properties); @@ -137,6 +138,9 @@ function options_field_widget_form(&$form, &$form_state, $field, $instance, $lan * Form element validation handler for options element. */ function options_field_widget_validate($element, &$form_state) { + if ($element['#required'] && $element['#value'] == '_none') { + form_error($element, t('!name field is required.', array('!name' => $element['#title']))); + } // Transpose selections from field => delta to delta => field, turning // multiple selected options into multiple parent elements. $items = _options_form_to_storage($element); @@ -146,12 +150,12 @@ function options_field_widget_validate($element, &$form_state) { /** * Describes the preparation steps required by each widget. */ -function _options_properties($type, $multiple, $required) { +function _options_properties($type, $multiple, $required, $has_value) { $base = array( 'zero_placeholder' => FALSE, 'filter_xss' => FALSE, 'strip_tags' => FALSE, - 'empty_value' => FALSE, + 'empty_option' => FALSE, 'optgroups' => FALSE, ); @@ -162,9 +166,25 @@ function _options_properties($type, $multiple, $required) { $properties = array( // Select boxes do not support any HTML tag. 'strip_tags' => TRUE, - 'empty_value' => !$required, 'optgroups' => TRUE, ); + if ($multiple) { + // Multiple select: add a 'none' option for non-required fields. + if (!$required) { + $properties['empty_option'] = 'option_none'; + } + } + else { + // Single select: add a 'none' option for non-required fields, + // and a 'select a value' option for required fields that do not come + // with a value selected. + if (!$required) { + $properties['empty_option'] = 'option_none'; + } + else if (!$has_value) { + $properties['empty_option'] = 'option_select'; + } + } break; case 'buttons': @@ -173,9 +193,11 @@ function _options_properties($type, $multiple, $required) { // Form API 'checkboxes' do not suport 0 as an option, so we replace it with // a placeholder within the form workflow. 'zero_placeholder' => $multiple, - // Checkboxes do not need a 'none' choice. - 'empty_value' => !$required && !$multiple, ); + // Add a 'none' option for non-required radio buttons. + if (!$required && !$multiple) { + $properties['empty_option'] = 'option_none'; + } break; case 'onoff': @@ -202,8 +224,9 @@ function _options_get_options($field, $instance, $properties) { $options = options_array_flatten($options); } - if ($properties['empty_value']) { - $options = array('_none' => theme('options_none', array('instance' => $instance))) + $options; + if ($properties['empty_option']) { + $label = theme('options_none', array('instance' => $instance, 'option' => $properties['empty_option'])); + $options = array('_none' => $label) + $options; } return $options; @@ -289,7 +312,7 @@ function _options_form_to_storage($element) { // Filter out the 'none' option. Use a strict comparison, because // 0 == 'any string'. - if ($properties['empty_value']) { + if ($properties['empty_option']) { $index = array_search('_none', $values, TRUE); if ($index !== FALSE) { unset($values[$index]); @@ -368,7 +391,7 @@ function options_field_widget_error($element, $error, $form, &$form_state) { /** * Returns HTML for the label for the empty value for options that are not required. * - * The default theme will display N/A for a radio list and blank for a select. + * The default theme will display N/A for a radio list and '- None -' for a select. * * @param $variables * An associative array containing: @@ -378,12 +401,18 @@ function options_field_widget_error($element, $error, $form, &$form_state) { */ function theme_options_none($variables) { $instance = $variables['instance']; + $option = $variables['option']; + + $output = ''; switch ($instance['widget']['type']) { case 'options_buttons': - return t('N/A'); + $output = t('N/A'); + break; + case 'options_select': - return t('- None -'); - default : - return ''; + $output = ($option == 'option_none' ? t('- None -') : t('- Select a value -')); + break; } + + return $output; } diff --git a/modules/field/modules/options/options.test b/modules/field/modules/options/options.test index 8cbfd5bc7c25..5fa444b66961 100644 --- a/modules/field/modules/options/options.test +++ b/modules/field/modules/options/options.test @@ -207,6 +207,7 @@ class OptionsWidgetsTestCase extends FieldTestCase { 'field_name' => $this->card_1['field_name'], 'entity_type' => 'test_entity', 'bundle' => 'test_bundle', + 'required' => TRUE, 'widget' => array( 'type' => 'options_select', ), @@ -220,13 +221,23 @@ class OptionsWidgetsTestCase extends FieldTestCase { $entity->is_new = TRUE; field_test_entity_save($entity); - // Display form: with no field data, nothing is selected. + // Display form. $this->drupalGet('test-entity/' . $entity->ftid .'/edit'); + // A required field without any value has a "none" option. + $this->assertTrue($this->xpath('//select[@id=:id]//option[@value="_none" and text()=:label]', array(':id' => 'edit-card-1-' . $langcode, ':label' => t('- Select a value -'))), t('A required select list has a "Select a value" choice.')); + + // With no field data, nothing is selected. + $this->assertNoOptionSelected("edit-card-1-$langcode", '_none'); $this->assertNoOptionSelected("edit-card-1-$langcode", 0); $this->assertNoOptionSelected("edit-card-1-$langcode", 1); $this->assertNoOptionSelected("edit-card-1-$langcode", 2); $this->assertRaw('Some dangerous & unescaped markup', t('Option text was properly filtered.')); + // Submit form: select invalid 'none' option. + $edit = array("card_1[$langcode]" => '_none'); + $this->drupalPost(NULL, $edit, t('Save')); + $this->assertRaw(t('!title field is required.', array('!title' => $instance['field_name'])), t('Cannot save a required field when selecting "none" from the select list.')); + // Submit form: select first option. $edit = array("card_1[$langcode]" => 0); $this->drupalPost(NULL, $edit, t('Save')); @@ -234,31 +245,30 @@ class OptionsWidgetsTestCase extends FieldTestCase { // Display form: check that the right options are selected. $this->drupalGet('test-entity/' . $entity->ftid .'/edit'); + // A required field with a value has no 'none' option. + $this->assertFalse($this->xpath('//select[@id=:id]//option[@value="_none"]', array(':id' => 'edit-card-1-' . $langcode)), t('A required select list with an actual value has no "none" choice.')); $this->assertOptionSelected("edit-card-1-$langcode", 0); $this->assertNoOptionSelected("edit-card-1-$langcode", 1); $this->assertNoOptionSelected("edit-card-1-$langcode", 2); + // Make the field non required. + $instance['required'] = FALSE; + field_update_instance($instance); + + // Display form. + $this->drupalGet('test-entity/' . $entity->ftid .'/edit'); + // A non-required field has a 'none' option. + $this->assertTrue($this->xpath('//select[@id=:id]//option[@value="_none" and text()=:label]', array(':id' => 'edit-card-1-' . $langcode, ':label' => t('- None -'))), t('A non-required select list has a "None" choice.')); // Submit form: Unselect the option. $edit = array("card_1[$langcode]" => '_none'); $this->drupalPost('test-entity/' . $entity->ftid .'/edit', $edit, t('Save')); $this->assertFieldValues($entity_init, 'card_1', $langcode, array()); - // A required select list does not have an empty key. - $instance['required'] = TRUE; - field_update_instance($instance); - $this->drupalGet('test-entity/' . $entity->ftid .'/edit'); - $this->assertFalse($this->xpath('//select[@id=:id]//option[@value=""]', array(':id' => 'edit-card-1-' . $langcode)), t('A required select list does not have an empty key.')); - - // We do not have to test that a required select list with one option is - // auto-selected because the browser does it for us. - // Test optgroups. $this->card_1['settings']['allowed_values'] = NULL; $this->card_1['settings']['allowed_values_function'] = 'list_test_allowed_values_callback'; field_update_field($this->card_1); - $instance['required'] = FALSE; - field_update_instance($instance); // Display form: with no field data, nothing is selected $this->drupalGet('test-entity/' . $entity->ftid .'/edit'); -- GitLab