Posted by asimmonds on October 30, 2005 at 11:54am
| Project: | Drupal core |
| Version: | x.y.z |
| Component: | base system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | chx |
| Status: | closed (fixed) |
Issue Summary
In the process of investigating http://drupal.org/node/35467, I found that blank required form fields are not detected anymore.
I think the problem is in form.inc:
@@ -138,7 +138,7 @@ function _form_validate($elements) {
/* Validate the current input */
if (!$elements['#validated'] && $elements['#input']) {
* if ($elements['#required'] && !isset($elements['#value'])) {
form_error($elements, t('%name field is required', array('%name' => $elements['#title'])));
}
if ($elements['#valid']) {The !isset does not work for a field that is blank. I tried replacing it with empty(), which works most of the time but empty() also returns TRUE for a string "0" and a integer 0.
I created a new function drupal_empty() to replace the !isset
<?php
function drupal_empty($value = NULL) {
if (isset($value)) {
if (empty($value) && ($value !== 0) && ($value !== '0'))
return TRUE;
else
return FALSE;
}
else
return TRUE;
}
?>(it's not optimised or anything, just copied from the php help)
And that seems to work for the required form fields and '0' string fields are handled properly.
Is this the best course of action? Are there other places in the code which don't work quite right because of isset()/empty() which could also use a function like this?
Comments
#1
I've in-lined these extra conditions into _form_validate()
#2
If you thought that 'POST values are strings, why === 0?' then think of empty checkboxes.
That said, this applies and is a must.
#3
Aren't we doing the same check
($elements['#value'] !== 0)twice?That doesn't look right to me ...
#4
we are checking first for 0 and then for '0'
#5
Dries is somewhat right, the '0' check is now not needed, that would kill a literal 0 in a textfield for example. Empty checkbox is 0 not '0'.
#6
With patch #5, a textfield with a literal '0' in it is still seen as required.
Changing it to:
<?phpif ($elements['#required'] && empty($elements['#value']) && $elements['#value'] !== '0') {
?>
allows the '0' in a textfield as well as still detecting a unchecked set of checkboxes.
(Sorry, can't roll a patch at the moment)
#7
And here's the patch with that change.
Tested against a form with:
single checkbox, radios set, checkboxes set, textfield, select, multiple-select, weight
all controls with #required => TRUE (I wanted to see if did anything weird with the other controls)
textfield errored on blank, but not '0', 0.0, or -0
checkboxes set errored on all unchecked, but OK on at least one checkbox checked
single checkbox errored on unchecked, OK on checked (but why have a required single checkbox?)
all other controls seemed to behave properly
#8
this patch works for me, thanks
#9
Looks good, still applies.
#10
Committed to HEAD. Thanks.
#11