Download & Extend

Select elements which are required can be submitted without any user input

Project:Drupal core
Version:6.x-dev
Component:forms system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (won't fix)

Issue Summary

If there is a select element on the page which does not have the default value set and the element is set to required, the form will default the the first item in the #options array. This means that the user can skip over the "required" field and submit the field without ever selecting a value, and the result will be whatever value is first in the #options array.

I have created a fix for this. It seems to work great but I am not too fluent with the FAPI yet to know if my fix is the best way of going about it.

Here is what I did:

<?php
function form_builder($form_id, $form) {
  global
$form_values, $form_submitted, $form_button_counter;

 
// Initialize as unprocessed.
 
$form['#processed'] = FALSE;

 
/* Use element defaults */
 
if ((!empty($form['#type'])) && ($info = _element_info($form['#type']))) {
   
// Overlay $info onto $form, retaining preexisting keys in $form.
   
$form += $info;
  }

// This is the part I added 
 
if ($form['#type'] == 'select') {
    if (
$form['#required'] && !$form['#default_value']) {
    
$form['#options'] = array_reverse($form['#options'], true);
    
$form['#options'][''] = '';
    
$form['#options'] = array_reverse($form['#options'], true);
    
$form['#default_value'] = '';
    }
  }
?>

Does this look correct?

Comments

#1

Version:5.2» 7.x-dev
Priority:critical» normal

These sorts of changes need to happen in the current development version, not a stable release.

Moving to normal priority since nothing is fundamentally broken.

#2

Assigned to:Anonymous» sobi3ch
Status:active» reviewed & tested by the community

I made small code lifting with testing and then I roll a patch.

AttachmentSizeStatusTest resultOperations
form.inc_.patch1023 bytesIdleFailed: Failed to apply patch.View details | Re-test

#3

Status:reviewed & tested by the community» needs review

Thanks for re-rolling and improving. Don't mark your own patches as RTBC -- I'd like to get another person to review this as well.

#4

Status:needs review» needs work

The last submitted patch failed testing.

#5

For those that need this in Drupal 6.x I made a patch on 6.10 (form.inc version 1.265.2.19).

AttachmentSizeStatusTest resultOperations
form.inc-D6.patch851 bytesIgnored: Check issue status.NoneNone

#6

I tested it.
it works fine with ( | ). But I would expect not to need to use it :(

It also works fine if I get my values by a code in allowed values - PHP code. The only problem is that the value is not showed in field settings default value select list. On the form is it than available.

It does not work with a node reference. There is still no default value available :(

any ideas to get it work with node reference?

I would expect to have the value at each time as an available option that cant be submited if the field is required.

Once again, thx for your contribution, it brought me forward.

#7

Hi,
please can you tell me why the patch is not in 6.12?
thx

#8

I wish someone could tell me that too. It's weird that if the element is set to not being required it starts with a - None - option but as soon as you select required it sets the first item in the list as default.

Please fix this for 7.x, if I need to do more legwork to make it happen I am definitely willing as this is a big issue IMHO.

I will create a Drupal 7 patch this evening.

#9

I'm not sure why this patch is failing testing, can anyone shine some light?

Also I made a small change to the 6.x patch, changed the default option from "" to "- None -" to keep it consistent with the core and CCK.

AttachmentSizeStatusTest resultOperations
form.inc-D6.patch851 bytesIgnored: Check issue status.NoneNone

#10

Lets get this in 7!!!!

What do I need to do to make it happen?????

#11

Patch in #9 has been tested on the latest Drupal 6.14 and confirmed working.

Still no word on how I can push this into D7 before launch?

#12

Is there anything further to do on this, it is marked as needs work, but the patch in #9 works. Presumably too late to get it in 7, but has it made it into 6 yet??

#13

@DeeLay: #9 was ignored by simpletest so it can't go forward..

#14

Assigned to:sobi3ch» Anonymous
Status:needs work» needs review

Ok so it's done for d7dev, please check..

AttachmentSizeStatusTest resultOperations
form.inc_.d7dev.patch775 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form.inc_.d7dev.patch.View details | Re-test

#15

Status:needs review» needs work

The last submitted patch, form.inc_.d7dev.patch, failed testing.

#16

Status:needs work» needs review

new approach

WOW! it's seems it's bigger issue then I thought in the begging :)
40 fail(s), and 36,738 exception(es).

AttachmentSizeStatusTest resultOperations
form.inc_.patch804 bytesIdleFAILED: [[SimpleTest]]: [MySQL] 18,570 pass(es), 40 fail(s), and 36,738 exception(es).View details | Re-test

#17

.. . Don't mark your own patches as RTBC -- I'd like to get another person to review this as well.

sure

#18

Status:needs review» needs work

The last submitted patch, form.inc_.patch, failed testing.

#19

Status:needs work» needs review

2nd approach

AttachmentSizeStatusTest resultOperations
form.inc_.v2.patch1.08 KBIdleFAILED: [[SimpleTest]]: [MySQL] 18,566 pass(es), 28 fail(s), and 26,593 exception(es).View details | Re-test

#20

Status:needs review» needs work

The last submitted patch, form.inc_.v2.patch, failed testing.

#21

Status:needs work» needs review

I'm proposing new sub-select-#options element
#unselected

Example:

  $form['foo'] = array(
    '#type' => 'select',
    '#title' => t('Foo title'),
    '#required' => TRUE,
    '#options' => array(
      '#unselected' => TRUE,
      'title' => t('Titles only'),
      'teaser' => t('Titles plus teaser'),
      'fulltext' => t('Full text'),
    ),
  );
AttachmentSizeStatusTest resultOperations
form.inc_.v3.patch1.12 KBIdlePASSED: [[SimpleTest]]: [MySQL] 18,701 pass(es).View details | Re-test

#22

Very nice! Probably too late to make it into 7.x?

#23

someone can make a review?! please. I think it's worth...

#24

Status:needs review» needs work

+++ includes/form.patch-please-select.inc 2010-03-11 13:42:42.507418314 +0100
@@ -1201,6 +1201,25 @@ function form_builder($form_id, $element
+  /**
+   * If element #type exist and #type == 'select' ¶
+   * and '#select > #unselected' == TRUE // mean that developer ask drupal form api to use '<Please select>' message

trailing white space

Powered by Dreditor.

#25

Status:needs work» postponed (maintainer needs more info)

1) I'm relatively sure that this bug does not exist in Drupal 7.

2) The patch implements something entirely different than this issue is about.

Most likely won't fix.

#26

#9 was ignored because:

Drupal core patches: Run the diff command in the root Drupal directory where you're making a patch, the same directory that contains the index.php, cron.php, etc. files. Core patches should always be made from Drupal root.

see more: http://drupal.org/patch/create

so I create this again from drupal root folder

@sun:

1) I'm relatively sure that this bug does not exist in Drupal 7.

I agree > changing

2) The patch implements something entirely different than this issue is about.

yes to much coffee :)

AttachmentSizeStatusTest resultOperations
form.inc_.requreid-select.patch881 bytesIdleFAILED: [[SimpleTest]]: [MySQL] Unable to apply patch form.inc_.requreid-select.patch.View details | Re-test

#27

Status:postponed (maintainer needs more info)» needs review

#28

Version:7.x-dev» 6.x-dev

#29

Status:needs review» needs work

The last submitted patch, form.inc_.requreid-select.patch, failed testing.

#30

Status:needs work» closed (won't fix)

This is technically a duplicate of #140783: A select list without #default_value always passes form validation and cannot be fixed for D6, as it has a potential impact on every select element in all forms throughout Drupal. To understand what I'm saying, make sure to read the entire aforementioned issue.

For D6, you need to implement a custom workaround for this problem in your module; i.e., manually validate that a user selected something on your own.

Tricky hint for how to achieve this in a simple way:

  '#required' => TRUE,
  '#options' => array(
    NULL => t('- Select -'),
  ),

#31

@sun: ok I understand now. Sorry for hassle...

nobody click here