Wanted my users to be forced to choose any taxonomy term from the handy dandy sliding list. But selecting none as the default value doesn't work. Instead the first taxonomy term listed is used as the default value. Check the number of Drupal users listed as residing in Afghanistan for an illustration of the problem at hand.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mlncn’s picture

Tagging... though this goes beyond usability to be a straight functionality bug.

mr.baileys’s picture

Status: Active » Postponed (maintainer needs more info)

I'm unable to reproduce this with a current version of HEAD, so this might have been fixed recently.

I tried with setting the "Term reference" field to required, used the "select list" widget and set the default to "- none -". When I go to enter content, no value is selected by default in the select list, the option "- none -" does not appear, and I cannot save the node without selecting at least one of the valid options.

Please re-open with steps to reproduce if you are still seeing this issue on a clean and recent install…

mlncn’s picture

Status: Postponed (maintainer needs more info) » Active

This bug still appears in the latest D7 head, maybe we did not communicate well.

If the option "- none -" does not appear in the select list, what does appear? This is the problem. If i create a taxonomy with First, Second, Third as terms, add that to a content type as a term reference field and mark it required, it should show me the "- none -" but give an error when i try to save it. Instead, the First is what is visible in the select field (not highlighted, true, but) -- when i save the node, even though i have never touched this term field, the node saves with a taxonomy value of "First". This should not happen when the default is set to none.

mr.baileys’s picture

Component: taxonomy.module » field system
Status: Active » Needs review
FileSize
1.21 KB

Seems the confusion stems from the fact that the bug only occurs when the "select list" widget is rendered as a drop-down list. When I tried earlier, my field was set to "Unlimited" number of values, causing the list to be rendered as a combobox instead, and validation correctly prevented me from submitting the node.

This problem does not affect just taxonomy.module, but rather all field types using the "select list" widget exposed by the options.module provided a) Number of values is set to 1 and b) the field is required.

To reproduce:

  • Create a content type
  • Add a field of type "List" (my_field)
    • Enter some values in the "allowed values" list
    • Make it a required field with "Number of values" set to 1
  • Create a new node but leave the drop-down untouched.

Expected result: a validation error telling you that field my_field is a required field
Actual result: the node is saved using the first (unselected) list item as value for field my_field

Patch attached that alters the select list widget in the options.module so it adds the "none" value for required single-value fields, and validates when "none" is selected.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

looks like the right fix to me.

mlncn’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +dgd7

Awesome! Sorry i missed a crucial step in the bug report.

Looks like a very nice fix codewise and it works... almost.

With radio buttons, i'm getting properly none of them selected, and "An illegal choice has been detected. Please contact the site administrator." rather than an error message and red box identifying the problem and the field. With the select list widget, it's still not adding the none option and it is using the unselected first value.

mlncn’s picture

Status: Needs work » Needs review

Cross-posted with Moshe and i'm going to assume i have something messed up locally (though i've tested on two installs), because it sure looks like that code should work.

Could someone else review though?

mlncn’s picture

Status: Needs review » Needs work

Brand new CVS HEAD download, patch in #4 applied, install Drupal with the default installation profile, if i create a Term reference or a List field, make it required, allow only one value– the behavior is the same as described in #6. Select lists, without being touched, will save to the first value, and radio buttons will trigger the non-specific error.

mr.baileys’s picture

Yup, looks like I screwed up the empty-field condition *after* I tested it and before rolling the patch. Probably means tests are in order, so attached is a correct patch with tests.

The only change when compared to current HEAD is that required single-value select lists get the "none" option. The optional select lists already have this option, and optional multiple select lists are not affected by the "first-value-selected-when-no-selection-made" bug.

...and radio buttons will trigger the non-specific error.

Not sure, but I think this is a form API bug rather than a bug in the options widget: looks like FAPI throws this error when a required radio button list is posted without (default) value selected. Same behavior occurs without applying this patch. Since this is different from the "wrong-value-saved-bug" reported in the OP (validation kicks in, even though with the wrong message) and since this is FAPI, this should probably be addressed in a separate issue, right?

mr.baileys’s picture

Status: Needs work » Needs review

status change...

mlncn’s picture

Status: Needs review » Reviewed & tested by the community

Fantastic! Select dropdowns with a single required value work fine with this patch, both for List fields and Taxonomy term fields.

Found the issue addressing radio buttons weirdness: #811542: Regression: Required radios throw illegal choice error when none selected

yched’s picture

Looks like the right approach to me as well.

sun’s picture

yched’s picture

Assigned: Unassigned » yched
Status: Reviewed & tested by the community » Needs review
FileSize
4.83 KB

Patch #9 works, and is an improvement on the current HEAD.

However,
- In the case targeted here (required select dropdown), the option selected by default should be labeled 'Select a value' rather than 'None', which reads like a valid value, while it's not. A 'required' asterisk mark and a preselected 'None' option next to each other give contradictory signals.
- We should only add this option when the element comes with no preselected value. If the select already has a value that I want to change, seeing the '_none' option (whether 'None' or 'Select a value') in the list looks sloppy.

Patch attached, no additional test for now, help welcome for those.

yched’s picture

Title: Setting default to none not respected; first field is selected by default » Fields with select widget: first option is selected by default even if no 'default value' set for the field

Clearer title (hopefully) ?

yched’s picture

Status: Needs review » Needs work

needs some more work

yched’s picture

Status: Needs work » Needs review
FileSize
9.63 KB

Here's a patch as per #14, with full tests :

Non-required select list :
- None -
option 1
option 2

Required select list with no pre-filled value (e.g node creation form) :
- Select a value - (selected by default - form validation fails submitted with this option selected)
option 1
option 2

Required select list with an existing value (e.g node edit form, or node creation form if the field has been set up with a 'default value') :
option 1
option 2 (selected by default if the current node value is '2')

Status: Needs review » Needs work

The last submitted patch, 735426-options-required-default-17.patch, failed testing.

yched’s picture

Status: Needs work » Needs review
FileSize
9.63 KB

Stupid typo. Updated patch, should fix the tests.

mlncn’s picture

Status: Needs review » Reviewed & tested by the community

Outlier test – attaching a vocabulary that at first had no terms – and this patch works beautifully.

Without patch:

  1. Created Vocabulary.
  2. Attached it to a content type as a required select field, the "None" option was the only one for default (naturally, as the vocabulary has no terms). At this point, can't save a node of this type, needs a valid term (all good and fair).
  3. Added terms to Vocabulary.
  4. Create node form defaults the field to the first taxonomy term in the select field.

With patch:

  1. Create node form defaults to " - Select a value - " and helpfully names the field when no value is selected.

And for the required, empty vocabulary it is just a field title and an asterisk. Good enough for a situation that's just not viable.

Tried out other more normal scenarios and everything works well, with the exception of the very unhelpful error for unselected required radio buttons that needs to be handled here: #811542: Regression: Required radios throw illegal choice error when none selected

Code looks good. Ready to grace D7 before the beta (or next alpha)!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

This looks good. Committed to CVS HEAD.

yched’s picture

Status: Fixed » Reviewed & tested by the community

Didn't get committed :-)

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. For real now. :)

Status: Fixed » Closed (fixed)
Issue tags: -Fields in Core, -#d7ux, -D7UX usability, -dgd7

Automatically closed -- issue fixed for 2 weeks with no activity.