Missing #empty_option on a couple selects

I must admit I kind of lost track on what #140783: A select list without #default_value always passes form validation does exactly. However, #934110: Regression: Installer defaults country to 'Afghanistan' not '- None -' received a similar fix and got signed off by sun, so this one should be OK.

+ bumping priority.

I just went through the originally committed patch to extract all locations that we should double-check here -- basically, all select lists that previously had a manually inserted "none" option need to set either #empty_value or #empty_option now. To use the default none option, only #empty_value should be set. Only set #empty_option, if we explicitly need a different empty option text than "- None -".

That said, I think we should go with the default #empty_option for Image field's link target select list, too.

+++ modules/field_ui/field_ui.admin.inc	24 Sep 2010 17:59:23 -0000
@@ -304,7 +304,7 @@ function field_ui_field_overview_form($f
-    '#parent_options' => array('' => t('<none>')),
+    '#parent_options' => array(),

@@ -335,6 +335,7 @@ function field_ui_field_overview_form($f
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,

@@ -401,6 +402,7 @@ function field_ui_field_overview_form($f
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,

@@ -458,6 +458,7 @@ function field_ui_field_overview_form($f
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,

@@ -526,6 +530,7 @@ function field_ui_field_overview_form($f
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,

@@ -844,7 +853,7 @@ function field_ui_display_overview_form(
-    '#parent_options' => array('' => t('<none>')),
+    '#parent_options' => array(),

@@ -888,8 +897,8 @@ function field_ui_display_overview_form(
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,
           '#options' => $table['#parent_options'],
-          '#default_value' => '',

@@ -1042,8 +1051,8 @@ function field_ui_display_overview_form(
       'parent_wrapper' => array(
         'parent' => array(
           '#type' => 'select',
+          '#required' => FALSE,
           '#options' => $table['#parent_options'],
-          '#default_value' => '',

+++ modules/image/image.field.inc	24 Sep 2010 17:59:23 -0000
@@ -269,7 +269,8 @@ function image_field_widget_settings_for
   $form['preview_image_style'] = array(
     '#title' => t('Preview image style'),
     '#type' => 'select',
-    '#options' => array('' => '<' . t('no preview') . '>') + image_style_options(FALSE),
+    '#required' => FALSE,
+    '#options' => image_style_options(FALSE),

@@ -418,22 +419,24 @@ function image_field_formatter_settings_
   $link_types = array(
-    '' => t('<none>'),
     'content' => t('Content'),
     'file' => t('File'),
   );
   $form['image_link'] = array(
     '#title' => t('Link image to'),
     '#type' => 'select',
+    '#required' => FALSE,

+++ modules/system/system.admin.inc	24 Sep 2010 17:59:23 -0000
@@ -2941,7 +2941,7 @@ function system_actions_manage() {
-  $options = array(t('Choose an advanced action'));
+  $options = array();

@@ -3013,9 +3013,7 @@ function system_actions_manage_form($for
   $form['parent']['action'] = array(
     '#type' => 'select',
-    '#default_value' => '',
     '#options' => $options,
-    '#description' => '',

+++ modules/trigger/trigger.admin.inc	24 Sep 2010 17:59:23 -0000
@@ -177,7 +177,6 @@ function trigger_assign_form($form, $for
   // List possible actions that may be assigned.
   if (count($options) != 0) {
-    array_unshift($options, t('Choose an action'));
     $form[$hook]['parent']['aid'] = array(
       '#type' => 'select',
       '#options' => $options,

Correct. If #empty_value is set, a non-required select list gets the additional empty option.

Either #empty_value or #empty_option can be set to trigger the empty option. Of course, you can also set both.

#empty_value defaults to an empty string (''), #empty_option defaults to '- None -'.

For this issue, we are only interested in select lists that

- are not #required

- and should have an empty option,

- but do not define #empty_value or #empty_option.

I think that some of the select lists I pasted above were already fixed in the follow-up patch, but we should double-check that to be sure.

re #2 :

- hunks in field_ui.admin.inc :
Right - we need to add '#empty_value' => '' to all the ['parent_wrapper']['parent'] elements - both in field_ui_field_overview_form() and field_ui_display_overview_form().

- image.field.inc :
That looks like a missing '#empty_option' => '<' . t('no preview') . '>'

Did not look into the other hunks.

Subscribe

Let's fix those - the ['parent_wrapper']['parent'] one on Field UI is messing with people testing fieldgroups.

Attached patch fixes all the hunks sun identified in #2.

Subscribe.

Eh. Not really sure what the right component for this issue is, so calling it "forms system" for now.

Committed #7 to HEAD. That might not be everything, but it's at least better than what we have.