To reproduce:
1) Create a new field in the profile: a "checkbox" with "The user must enter a value." "Visible in user registration form."
2) Register a user, without filling in the checkbox.

If reproduced properly despite the required field not being filled in the user is registered.

Reletad to http://drupal.org/node/179932 ?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Chill35’s picture

Component: profile.module » forms system
Priority: Normal » Critical

I noticed the problem as well.

I was using a theming function to add a required checkbox to the 'user_register' form, it wasn't marked as required.
Then I used hook_form_alter to do the same. Still, the checkbox was not marked as required.

So I added a checkbox through the profile.module, using "The user must enter a value." and "Visible in user registration form."

Still the checkbox is not marked as required.

When using the profile.module and adding other types of fields, such as 'text field', with "The user must enter a value." and "Visible in user registration form.", fields are marked as required.

This is a problem with checkbox ONLY, as far as I can tell.

Since this problem can be reproduced beyond the profile.module, I am changing the 'component' of this issue to 'forms system'.

Since this seems to affect the Forms API generally, I am marking this as 'critical'.

Chill35’s picture

Title: checkboxes set to "required" and in registration form doesn't work as expected » checkboxes set to "required" and in registration form don't work as expected
Priority: Critical » Normal

Maybe this isn't critical...

Chill35’s picture

Title: checkboxes set to "required" and in registration form don't work as expected » checkbox set to "required" in any form doesn't work as expected
Priority: Normal » Critical

I just made different tests... adding required checkbox in different forms.

Doesn't work for any form.

I checked the Drupal Form API, there's no trick about it, setting the '#required' property to TRUE (or 1) *should* work.

Chill35’s picture

The problem is in form.inc I think.

Line 666, number of the beast! (evil code)

if (isset($elements['#needs_validation'])) {
  // Make sure a value is passed when the field is required.
  // A simple call to empty() will not cut it here as some fields, like
  // checkboxes, can return a valid value of '0'. Instead, check the
  // length if it's a string, and the item count if it's an array.
  if ($elements['#required'] && (!count($elements['#value']) || (is_string($elements['#value']) && strlen(trim($elements['#value'])) == 0))) {
    form_error($elements, $t('!name field is required.', array('!name' => $elements['#title'])));
  }
Chill35’s picture

I am not familiar enough with the form API at this point to come up with proper conditions.

Chill35’s picture

Title: unckecked checkbox set to "required" bypasses validation » checkbox set to "required" in any form doesn't work as expected

This is a serious problem.

You can't launch a commercial site without

[x] I agree.

Or

[x] I CERTIFY THAT I AM OVER THE AGE OF 18 YEARS OLD.

Chill35’s picture

Title: checkbox set to "required" in any form doesn't work as expected » unckecked checkbox set to "required" bypasses validation
hellsnail’s picture

Title: checkbox set to "required" in any form doesn't work as expected » unckecked checkbox set to "required" bypasses validation

A special case could be made for checkboxes in _form_validate, for the rare REQUIRED checkbox, I think.

// Make sure a value is passed when the field is required.
      // A simple call to empty() will not cut it here as some fields, like
      // checkboxes, can return a valid value of '0'. Instead, check the
      // length if it's a string, and the item count if it's an array.
      if ($elements['#required'] && (!count($elements['#value']) || (is_string($elements['#value']) && strlen(trim($elements['#value'])) == 0))) {
        form_error($elements, $t('!name field is required.', array('!name' => $elements['#title'])));
      }

one might add another clause to the IF, something like this pseudo code:
|| (TYPE == 'checkbox' && ( VALUE == 0 || VALUE == "0" || VALUE == false || strtoupper(VALUE) == 'FALSE'))

Chill35’s picture

I created a module to 'circumvent' the bug for the registeration form: http://drupal.org/project/terms_of_use

I am using the property '#element_validate' on the checkbox, hence I do my own validation through a function of my own.

Robert Castelo’s picture

I've created a generic solution here:

http://drupal.org/project/checkbox_validate

Just install the module and all checkboxes in every form display and behave the way they should.

I like this solution because it means developers don't need to add any extra code to their modules to fix the bug in core, and once core is fixed this module can be uninstalled.

If your module relies on required checkbox fields be sure to make this module a dependency.

webchick’s picture

Status: Active » Closed (duplicate)

Either this or #179932: Required radios/checkboxes are not validated is a duplicate. I've marked this one a dupe since it is newer, although the other was originally applied against another project, which is probably why it was missed.

RaRi’s picture

subscribe ..

ginc’s picture

Version: 6.2 » 6.16
Status: Closed (duplicate) » Needs work

not a duplicate of #179932: Required radios/checkboxes are not validated anymore as that bug fixed only in 7.x

geerlingguy’s picture

Version: 6.16 » 6.x-dev

Subscribe. Switched to 6.x dev (patches should be made against HEAD).

Cyberwolf’s picture

Subscribing.

StevenPatz’s picture

Title: unckecked checkbox set to "required" bypasses validation » unchecked checkbox set to "required" bypasses validation
YK85’s picture

subscribing

grendzy’s picture

Title: unchecked checkbox set to "required" bypasses validation » Required radios/checkboxes are not validated

syncing tag / title with D7. (sun.core asked for a separate D6 issue).

izmeez’s picture

subscribing

steveh7’s picture

subscribing

achton’s picture

subscribing

Max K.’s picture

Status: Needs work » Needs review
FileSize
939 bytes

I found the problem -- the conditional for validating required fields was only testing for zero-length values or values that weren't set. In the case of an unchecked checkbox, the value will be set to the integer zero, so I added handling for that specific case. Give it a shot!

jcmarco’s picture

As this is the same issue than in http://drupal.org/node/179932, I am backporting the D7 solution for this problem:
required checkboxes are not validated and there is no required warning in the title.

For the validation issue, this is a back porting from D7.

For the required warning, obviously the Form API in D6 is different from D7, but this patch add exactly the same behavior than in D7 and for the rest of required form elements.
In theme_checkbox(), we add the same warning elements that are added in theme_form_element() function for the other required form elements.
It adds as well the $t=get_t() used in theme_form_element() for the rest of the form elements and as in D7.

It works great for CCK required checkboxes, and forms using this elements, as in legal module (with this core patch the checkbox_validate module wouldn't be necessary any more)

klonos’s picture

subscribing...

YK85’s picture

#23 did the trick, thanks!
Is there a chance of this getting committed into the Drupal 6 version?

sun’s picture

Title: Required radios/checkboxes are not validated » Required radios/checkboxes are not validated (D6)
YK85’s picture

Status: Needs review » Reviewed & tested by the community

The patch makes it so I no longer need to use the contrib module http://drupal.org/project/checkbox_validate
Thanks jcmarco for the D6 patch!

sun’s picture

Status: Reviewed & tested by the community » Needs review

Due to the follow-up issues/problems identified with the patch that went into D7 (#179932: Required radios/checkboxes are not validated), I'm not sure whether it is a wise idea to apply this change to D6.

I'd actually opt for a won't fix here. People can use checkbox_validate module for D6 and below. For D7, you won't, but we still have to figure out some major problems related to required/disabled checkboxes.

klonos’s picture

@sun: to which issues are you referring to? (issue numbers or post links please?)

If this (latest patch in #23) raises issues, but the Checkbox validate module on the other hand fixes them, why not merge this module in d6 core? At least in this case see how the module gets things done and mimic it for a final solution in core.

geerlingguy’s picture

@sun - the problem is, this is a pretty serious bug in D6, and should probably be fixed, since the settings page says "make this required," and it doesn't become required. So I'd support seeing this patch get in :-/

sun’s picture

Problem space being that checkbox_validate only fixes required checkboxes. D7 additionally hit the territory of disabled checkboxes, not covered/supported by checkbox_validate, and which may or may not work in D6 currently. Note that there are a couple of possible combinations to support, i.e., required only, disabled only, required and disabled (but checked), and normal (nothing).

While something has been committed to D7 for #179932: Required radios/checkboxes are not validated, that issue basically is still open and still major. One of the other D7 issues that aforementioned commit "broke" is #654796: Identify fix bugs with checkbox element, but actually I'm not sure whether there are not some more interrelated issues. I definitely have to spare some time to clean up this mess.

klonos’s picture

@geerlingguy: Jeff, I am totally with you! This definitely needs to be fixed in core.

@sun: Thanx for taking the time to reply, Daniel. I just didn't get why it was closed, but I realized it was simply because didn't read through the whole issue. So, I guess we need to wait on the second issue and once we have a solid fix for d7 then backport to d6(?).

Status: Needs review » Needs work

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

grendzy’s picture

Status: Needs work » Needs review

bad bot! This is a D6 patch.

marcoBauli’s picture

subscribing (too bad, also Checkbox Validate fails to correct this..)

jurgenhaas’s picture

Isn't there also the problem that form_error() won't work with either checkboxes nor select fields? I've implemented my own validation on checkboxes and selects and get no effect whatsoever, Drupal just moves on to form submission and doesn't go back on a form_error on either of those fields.

jurgenhaas’s picture

It looks as if form_error() fails with checkboxes and selects is because they don't come with a '#parents' element in the field definition. All other field types get that assigned automatically.

nuwaninfo’s picture

The last submitted patch, form.inc_.patch, was successfully tested.

Berliner-dupe’s picture

Hi,

there is a second problem with the checkbox-validation.

If a CCK-Checkbox-field is required and the user dont fill out this field and he will save the add-form then the checkbox-field will not get an DIV-error-classes.

If it is a normal text-field or a radio-button-field the error-class will set.

Examples for fields after saving the add-form (without fill out the field)

normal text-field:
<input type="text" class="form-text required error" id="edit-field-1-adresse-0-city" name="field_1_adresse[0][city]">
text-field (radio button):
<input type="radio" class="form-radio error" value="Kosten angeben" name="field_1_kosten1[value]" id="edit-field-1-kosten1-value-Kosten-angeben">
text-field (checkbox) (no error-class / no required-class will set):
<input type="checkbox" class="form-checkbox" value="girls" id="edit-field-1-test-value-girls" name="field_1_test[value][girls]">

In form.inc in line 2246 the code is ...

function _form_set_class(&$element, $class = array()) {
  if ($element['#required']) {
    $class[] = 'required';
  }
  if (form_get_error($element)) {
    $class[] = 'error'; // <--- there
  }
  if (isset($element['#attributes']['class'])) {
    $class[] = $element['#attributes']['class'];
  }
  $element['#attributes']['class'] = implode(' ', $class);
}

but this works not for checkboxes ......

Has anyone an idea to fix this problem?

Sorry for my bad english, i hope you understand the problem.

Best Regards Matthias

lirix

evrix’s picture

patch from #23 working for me too, and I'm hard to work with :)
thanks

JStarcher’s picture

In the validation function you can add after form_error():

$element['#attributes']['class'][] = 'error';

With the system css this will attempt to add a 2px red border to the checkbox. However, not all browser and OS combinations allow checkboxes to be styled. The workaround for this would be to use a background image for the checkbox.

Berliner-dupe’s picture

@JStarcher

Thank you for your answer but in what line this code must in?

In this (line 692 in form.inc)?

form_error($elements, $t('!name field is required.', array('!name' => $elements['#title'])));

Can you help me please? Im am not a coder-geek!

Regards Matthias

Berliner-dupe’s picture

@JStarcher

I add the code!

form_error($elements, $t('!name field is required.', array('!name' => $elements['#title'])));
$elements['#attributes']['class'][] = 'error';

When i output now the required $elements (and i not fill out this) with Devel dsm($elements); then the #attributes-Array has the correct error-class-entry now. But in the html-source-text the required checkbox has anyhow no error-class (when i dont fill out the checkbox and then will save the node-add-form.) I get the message "Field xxx is requiered" but no error-class is available!

I dont understand this! Has you an idea?
Sorry for my bad english!

Edit:
This code will not work. Now - when you dont fill out a normal required text-field and then save the add-form it will get an error

Fatal error: [] operator not supported for strings ...

???

Berliner-dupe’s picture

For Drupal 6:
With this code (in your template.php or as own module) the error class will set to the label from checkbox when checkbox is required but the user dont fill out it and he save the form.

function YOURTHEME_form_element($element, $value) {
  // This is also used in the installer, pre-database setup.
  $t = get_t();
  $output = '<div class="form-item"';
  if (!empty($element['#id'])) {
    $output .= ' id="' . $element['#id'] . '-wrapper"';
  }

  $output .= ">\n";
  if (form_get_error($element)) {

  $required = !empty($element['#required']) ? '<span class="form-required error" title="' . $t('This field is required.') . '">*</span>' : '';
}
else
{
  $required = !empty($element['#required']) ? '<span class="form-required" title="' . $t('This field is required.') . '">*</span>' : '';
}

  if (!empty($element['#title'])) {
    $title = $element['#title'];
    if (!empty($element['#id'])) {
      $output .= ' <label for="' . $element['#id'] . '">' . $t('!title: !required', array('!title' => filter_xss_admin($title), '!required' => $required)) . "</label>\n";
    }
    else {
      $output .= ' <label>' . $t('!title: !required', array('!title' => filter_xss_admin($title), '!required' => $required)) . "</label>\n";
    }
  }

  $output .= " $value\n";

  if (!empty($element['#description'])) {
    $output .= ' <div class="description">' . $element['#description'] . "</div>\n";
  }

  $output .= "</div>\n";

return $output;
}

The error class is (for example) important when you use "Vertical Tabs". In Vertical Tabs you get a lot of fields in different Tabs and when the user dont fill out the required-Checkbox in Tab Nr. 6 (for example) - then will show (indeed) an Error-Message (Field xyz is required) but the correct Tab will not show the red-Color (because the error class is not availiable in checkbox) and the user dont know in what Tabs the required field is. Therefore he must control all Tabs to find the required field.

Code in top fix this problem, checkbox-label get an error-class and the correct Tab (where field is in) will show in red.

Sorry for my bad english.

Regards Matthias

alanmackenzie’s picture

subscribing.

joachim’s picture

Patch works for me with a custom form. Not tried it with CCK fields.

chinita7’s picture

@Berliner Thanks for your code.
I put your code from #44 in template.php in my theme folder and it works for single check box and multiple radio button. But not for multiple checkbox. .
I tried This code also and again works only for single check box and multiple radio button but not multi checkbox.
Any idea?

squarecandy’s picture

Patches in #22 and #23 works for me with no additional code, with CCK "single on/off" checkbox.

Status: Needs review » Needs work

The last submitted patch, form_checkboxes_validation-D6.patch, failed testing.

_Jay’s picture

Patches #22 and #23 and checkbox validate module don't work for me, I'm using Drupal 6.22. After applying the patch I get "field is required" warning regardless if the checkbox is checked or not. Seems that it's still not validated. Any ideas how to debug this?

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.