Checkbox setting ignored if Form validation in use and checkbox not being validated. #949

Closed
AvalonNZ opened this Issue Jan 22, 2012 · 6 comments

Projects

None yet

4 participants

@AvalonNZ

I have a registration form with multiple fields on it including two checkboxes. Most of the fields are validated (they're required) but I have a ONE checkbox that is optional and doesn't need to be validated so no rules are set for it.

When there's an error on the form I redisplay it and every field is redisplayed with the value set by the user EXCEPT the checkbox that wasn't validated.

Investigation in the library code suggests that the problem is due to the fact that there are two set_checkbox functions: one in form_helper.php which is globally available and one as a method in form_validation.php. The global set_checkbox function checks to see if the form_validator object is available and if so just hands control to it even if the field in question is NOT being validated. And since the form_validation object only contains POST information about fields that are being validated, the original value of any field (such as my checkbox) that has no validation rule is lost.

I've fixed the problem by altering the library code for the form validator and it's attached to this message. However it's pretty quick and dirty and someone who is more familiar with the form validation code might find a better solution.

I got around the problem by simply dumping the logic from the global set_checkbox function (which reads the $_POST array directly) into the section of the set_checkbox method that deals with the field not being found in the _field_data array.

@AvalonNZ

Here's the workaround code from form_validation.php:

public function set_checkbox($field = '', $value = '', $default = FALSE)
{
    /* ORIGINAL CODE
     *               
    if ( ! isset($this->_field_data[$field]) OR ! isset($this->_field_data[$field]['postdata']))
    {
        if ($default === TRUE AND count($this->_field_data) === 0)
        {
            return ' checked="checked"';
        }
        return '';
    }
    */

    //=============================================
    // Start of SIMON'S REVISION
    //=============================================

    if ( ! isset($this->_field_data[$field]) )
    {
        // if we don't have a _field_data entry for this field
        // then there are no rules for this field. And therefore 
        // there is no _field_data entry to contain the POST data.  

        if ( ! isset($_POST[$field]))
        {
            // The checkbox was NOT checked 

            if (count($_POST) === 0 AND $default == TRUE)
            {
                // there are no POST entries so the checkbox was not deliberately SET to OFF
                // so if the default is true then we can set it to checked

                return ' checked="checked"';
            }
            return '';
        }

        // The field contains some data and may have been checked

        $field = $_POST[$field];

        if (is_array($field))
        {
            // The fieldname is an array

            if ( ! in_array($value, $field))
            {
                // The array does NOT contain the stated value 
                // so the checkbox must NOT be checked

                return '';
            }
        }
        else
        {
            if (($field == '' OR $value == '') OR ($field != $value))
            {
                // The fieldname is blank, or the value is blank 
                // or the field does not contain the stated value
                // so the checkbox must NOT be checked

                return '';
            }
        }

        // The checkbox WAS checked

        return ' checked="checked"';

    }
    elseif (! isset($this->_field_data[$field]['postdata']))
    {
        // we have a _field_data entry but no postdata, so the checkbox was not set  

        if ($default === TRUE AND count($this->_field_data) === 0)
        {
            return ' checked="checked"';    // representing a checkbox that IS "checked"
        }
        return '';   // representing a checkbox that is NOT "checked"
    }


    //=============================================
    // End of SIMON'S REVISION
    //=============================================

    $field = $this->_field_data[$field]['postdata'];

    if (is_array($field))
    {
        if ( ! in_array($value, $field))
        {
            return '';
        }
    }
    else
    {
        if (($field == '' OR $value == '') OR ($field != $value))
        {
            return '';
        }
    }

    return ' checked="checked"';
}
@AvalonNZ

This is the code from form_helper.php to save you having to look it up for comparison.

if ( ! function_exists('set_checkbox'))
{
function set_checkbox($field = '', $value = '', $default = FALSE)
{
$OBJ =& _get_validation_object();

    if ($OBJ === FALSE)
    {
        if ( ! isset($_POST[$field]))
        {
            if (count($_POST) === 0 AND $default == TRUE)
            {
                return ' checked="checked"';
            }
            return '';
        }

        $field = $_POST[$field];

        if (is_array($field))
        {
            if ( ! in_array($value, $field))
            {
                return '';
            }
        }
        else
        {
            if (($field == '' OR $value == '') OR ($field != $value))
            {
                return '';
            }
        }

        return ' checked="checked"';
    }

    return $OBJ->set_checkbox($field, $value, $default);
}

}

@cryode
cryode commented Jan 24, 2012

This isn't a bug so much as it's a limitation of the form validation library and subsequent helpers.

The set_value() and other similar functions do NOT carry values over unless that field has been defined using the form library's set_rules() method. You can define an empty ruleset for your checkbox, and that will allow set_checkbox() to function properly without taking that field into consideration when running the validation checks.

$this->form_validation->set_rules('checkbox', 'My Checkbox', '');

This feature has been discussed and requested for CodeIgniter many times. I'd expect it to happen sometime in the not-too-distant future.

@sasquatcho

Im just going to chime in here having been bitten by this before...

Possibly its just a matter of changing the documentation to reflect that everything that a developer wants repopulated via set_xxxx() needs to have a rule. That in itself would at least communicate the requirement. The absence of such communication does indeed present itself as a "bug" to everyone who will eventually come across the behaviour.

Just my .02

@AvalonNZ

@cryode: thanks for the workaround.

And I agree that it's a limitation rather than a bug, but an "undocumented limitation" is pretty close on the irritation scale. I hope you're right about the issue being resolved in the near future.

Still I'd be happy with a quick change to the documentation as suggested by sasquatcho. Is there anyone we can talk to who can change the docs as suggested?

@narfbg
Collaborator
narfbg commented Jun 19, 2012

Most likely a duplicate of #104, just referencing.

@narfbg narfbg closed this Nov 3, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment