Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#19 | 735426-options-required-default-19.patch | 9.63 KB | yched |
#17 | 735426-options-required-default-17.patch | 9.63 KB | yched |
#14 | 735426-options-required-default-14.patch | 4.83 KB | yched |
#9 | 735426-options-required-default-2.patch | 3.52 KB | mr.baileys |
#4 | 735426-options-required-default.patch | 1.21 KB | mr.baileys |
Comments
Comment #1
mlncn CreditAttribution: mlncn commentedTagging... though this goes beyond usability to be a straight functionality bug.
Comment #2
mr.baileysI'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…
Comment #3
mlncn CreditAttribution: mlncn commentedThis 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.
Comment #4
mr.baileysSeems 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:
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.
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedlooks like the right fix to me.
Comment #6
mlncn CreditAttribution: mlncn commentedAwesome! 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.
Comment #7
mlncn CreditAttribution: mlncn commentedCross-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?
Comment #8
mlncn CreditAttribution: mlncn commentedBrand 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.
Comment #9
mr.baileysYup, 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.
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?
Comment #10
mr.baileysstatus change...
Comment #11
mlncn CreditAttribution: mlncn commentedFantastic! 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
Comment #12
yched CreditAttribution: yched commentedLooks like the right approach to me as well.
Comment #13
sun#9: 735426-options-required-default-2.patch queued for re-testing.
Comment #14
yched CreditAttribution: yched commentedPatch #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.
Comment #15
yched CreditAttribution: yched commentedClearer title (hopefully) ?
Comment #16
yched CreditAttribution: yched commentedneeds some more work
Comment #17
yched CreditAttribution: yched commentedHere'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')
Comment #19
yched CreditAttribution: yched commentedStupid typo. Updated patch, should fix the tests.
Comment #20
mlncn CreditAttribution: mlncn commentedOutlier test – attaching a vocabulary that at first had no terms – and this patch works beautifully.
Without patch:
With patch:
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)!
Comment #21
Dries CreditAttribution: Dries commentedThis looks good. Committed to CVS HEAD.
Comment #22
yched CreditAttribution: yched commentedDidn't get committed :-)
Comment #23
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. For real now. :)