I was looking through the source code of one of my forms today and noticed that each individual checkbox in a checkboxes field has the html class "error" set when the checkboxes field has an error flagged. It would
make more sense to set the error class for the parent div which contains all the checkboxes. In either case, the checkboxes field didn't get a "red highlight" like text fields do when they have an error flagged.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | theme-form-checkboxes-error-bug44933.patch | 545 bytes | Wesley Tanaka |
| #6 | theme-form-checkboxes-error-bug44933_0.patch | 634 bytes | Wesley Tanaka |
| #2 | theme-form-checkboxes-error-bug44933-alt2.patch | 388 bytes | Wesley Tanaka |
| #1 | theme-form-checkboxes-error-bug44933.patch | 538 bytes | Wesley Tanaka |
Comments
Comment #1
Wesley Tanaka commentedthis patch does exactly as the title says. it leaves the per-checkbox error class, since that's shared with a single checkbox field.
Comment #2
Wesley Tanaka commentedthis patch adds a span surrounding form elements (compound or not), but not including the name or description of the form element.
Comment #3
chx commentedhardly a bug.
Comment #4
webchickpatching file includes/theme.inc
Hunk #1 FAILED at 617.
1 out of 1 hunk FAILED -- saving rejects to file includes/theme.inc.rej
Comment #5
Wesley Tanaka commentedfixing version to the branch the bug was reported against.
i filed it as a bug because all other form elements get some kind of indication that they're the one causing the error, but composite form elements don't seem to.
this seems to still exist in 5.1 -- rerolling the patch at the moment
Comment #6
Wesley Tanaka commentedfirefox (v2) doesn't seem to honor the red border style or the red color style on checkboxes, by the way -- perhaps this is only a bug when using firefox?
Attached is the patch from #1 rerolled against drupal 5.1. Unfortunately, due to a promiscuous div.error style in system.css, this produces visually unsatisfactory results. It appears a better change might be to attach the error class to the div following the label element which, if I remember right, is new since 4.7.
Comment #7
Wesley Tanaka commentedpatch in last comment is against 5.1. changing version. setting to "needs review" because a question needs to be resolved:
Is it better to
1. set the error class in the .form-item divs as in the patch
or
2. change theme_checkboxes and theme_radios to put the error class on div.form-checkboxes and div.form-radios, since I think those are the only two core form elements that are affected by this?
I think #1 is a better solution because non-core custom form elements that used checkboxes and/or radio buttons would get the fix automatically without needing to change their theme_ function. In either case, system.css would need changes too, so this patch isn't complete.
Comment #8
drummHTML markup changes are generally not accepted for the stable 5.x branch as they might break themes. There is still time to get these changes in 6.x.
Comment #9
birdmanx35 commentedThis fails against the latest version of HEAD (I know, I know, of course it would).
$ patch -p0 < theme-form-checkboxes-error-bug44933_0.patch
patching file includes/form.inc
Hunk #1 FAILED at 1531.
1 out of 1 hunk FAILED -- saving rejects to file includes/form.inc.rej
Comment #10
keith.smith commented(changing status to cnw since patch does not apply)
Comment #11
Wesley Tanaka commentedRerolled for Drupal 5.11
Comment #12
Anonymous (not verified) commentedSee comment #8.
Comment #13
lcasassa commentedVersoin: Drupal 6.14.
Just add after line 1906 in form.inc:
_form_set_class($element, array($class)); // Linus
So then the parend div will have the error class.
This is how it should look:
function theme_checkboxes($element) {
$class = 'form-checkboxes';
_form_set_class($element, array($class)); // Linus
if (isset($element['#attributes']['class'])) {
$class .= ' '. $element['#attributes']['class'];
}
$element['#children'] = '
';
if ($element['#title'] || $element['#description']) {
unset($element['#id']);
return theme('form_element', $element, $element['#children']);
}
else {
return $element['#children'];
}
}
Comment #14
travismiller commented+1 for #13. thanks Linus
Comment #15
Berliner-dupe commented#13 has no effect to set an error-class to parent-div by checkboxes.
Anyone an idea?
Comment #16
Berliner-dupe commented....
Comment #17
chinita7 commented#13 works on my site for single check box and multiple radio button. But not for multiple check box.
another solution here by @Berliner but it also doesn't work for multi check box for me.