I have a select component with a single choice, formatted as checkbox with a default value of being checked. When I edit a submission with an unchecked box, it still shows as checked. This is because the unchecked value is never saved to the "webform_submission_data" table and subsequently the default value is used instead, which checks the box.

I did some step by step debugging and found the submit function for the select component (_webform_submit_select), and cannot understand how it works. There's a comment there about 0 being the result of an unchecked checkbox, but that code is never reached because the previous if() does not validate when the value is 0.

Values when entering this code with an unchecked checkbox is:
$option_value = 0;
$options = array('yes' => 'Yes');
$key = 'yes';

Which means this will never be true, maybe it should use $key in the $options[$option_value] ?

if ($option_value !== '' && isset($options[$option_value])) {
  // Checkboxes submit an integer value of 0 when unchecked. A checkbox
  // with a value of '0' is valid, so we can't use empty() here.
  if ($option_value === 0 && !$component['extra']['aslist'] && $component['extra']['multiple']) {
    unset($value[$option_value]);
  }
  else {
    $return[] = $option_value;
  }
}

Further I do not understand what "unset($value[$option_value]);" is supposed to do, since the $value array is never used again in that function.

I'll post a patch that I'm using, maybe it will help someone else :)

CommentFileSizeAuthor
#7 webform-unchecked_select_default-2390833-7.patch1.33 KBdanchadwick
#1 unchecked_checkbox_not_saved-2390833-#1.patch1.29 KBesbite

Comments

esbite’s picture

esbite commented
StatusFileSize
new unchecked_checkbox_not_saved-2390833-#1.patch1.29 KB
esbite’s picture

esbite commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 1: unchecked_checkbox_not_saved-2390833-#1.patch, failed testing.

danchadwick’s picture

danchadwick commented

Hmmm. Something does seem wrong, but I don't think your patch is correct.

The behavior I saw:

Select component, not a list, multiple selection, one option "yes|Yes" default of "yes".

Initially selected as expected. Deselected. Incorrectly shown as selected in preview page. Saved as unselected.

Edit existing submission. Was re-selected again.

I believe the correct schema for an unchecked multiple-selection select is the absence of a value in the database, not a '' value. But maybe not.

Definitely needs work, plus extensive testing with single/multiple, list/checkbox-radio buttons, select-as-other and not.

esbite’s picture

esbite commented

Thank you for the quick review. Though I'm a newbie to the webform codebase, I think its better to save an empty string for unselected checkboxes, but I can see the failed tests do not agree.

Here's the code for reading the stored value when editing or displaying the form for submission. From what I can see here it's quite clear that unselected checkboxes should be saved as an empty string:
"Note: "No choice" is stored as an empty string, which will match a 0 key for radios; NULL is used to avoid unintentional defaulting to the 0 option."

Should we rewrite the tests then, or rewrite this code? Where do we go from here? :)

Code snippet from: _webform_render_select()

  // Set the default value. Note: "No choice" is stored as an empty string,
  // which will match a 0 key for radios; NULL is used to avoid unintentional
  // defaulting to the 0 option.
  if (isset($value)) {
    if ($component['extra']['multiple']) {
      // Set the value as an array.
      $element['#default_value'] = array();
      foreach ((array) $value as $key => $option_value) {
        $element['#default_value'][] = $option_value === '' ? NULL : $option_value;
      }
    }
    else {
      // Set the value as a single string.
      $element['#default_value'] = '';
      foreach ((array) $value as $option_value) {
        $element['#default_value'] = $option_value === '' ? NULL : $option_value;
      }
    }
  }
  elseif ($default_value !== '') {
    // Convert default value to a list if necessary.
    if ($component['extra']['multiple']) {
      $varray = explode(',', $default_value);
      foreach ($varray as $key => $v) {
        $v = trim($v);
        if ($v !== '') {
          $element['#default_value'][] = $v;
        }
      }
    }
    else {
      $element['#default_value'] = $default_value;
    }
  }
  elseif ($component['extra']['multiple']) {
    $element['#default_value'] = array();
  }
danchadwick’s picture

danchadwick commented
Status: Needs work » Active
Related issues: +#2286429: Select component with radios confuses empty string with key == 0

The part about the preview being out-of-date is a separate issue, now fixed: #2420557: Preview page shows old values.

However, when editing submission with the data unchecked and the default checked, the data is changed back to the default. This is because an unchecked checkbox (i.e. select / not list / multiple) is not stored (i.e. is NULL). This is not distinguished from an item that needs defaulting.

Solution TBD.

danchadwick’s picture

danchadwick commented
Version: 7.x-4.2 » 8.x-4.x-dev
Status: Active » Fixed
StatusFileSize
new webform-unchecked_select_default-2390833-7.patch1.33 KB

The solution was easy. Ensure that NULL is never stored. If there are no checked (or, for list boxes, selected) item, save a single empty string. This will then be used the next time the component is edited to prevent the default from being set again.

Committed to 7.x-4.x and 8.x.

  • DanChadwick committed 5a9c39f on 7.x-4.x 
    Issue #2390833 by DanChadwick: Unchecked single checkbox not saved,...
  • DanChadwick committed d4514c3 on 8.x-4.x
    Issue #2390833 by DanChadwick: Unchecked single checkbox not saved,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

danchadwick’s picture

danchadwick commented
Version: 8.x-4.x-dev » 7.x-4.x-dev
stevendeleus’s picture

stevendeleus commented

Hi, this has been marked as fixed, but I'm still getting this issue with the latest patch, which has already been committed.

stevendeleus’s picture

stevendeleus commented
Status: Closed (fixed) » Active
danchadwick’s picture

danchadwick commented

but I'm still getting this issue with the latest patch, which has already been committed.

Can you reproduce this with the latest dev (currently 7.x-4.x+8-dev)

danchadwick’s picture

danchadwick commented
Status: Active » Closed (fixed)

Just tried to reproduce and it works as expected.

Create select component with only 1 option (no|NO), default of no, multiple.
Create a new submission. Default is no.
Uncheck.
Save
Look at submission and results table. Value is still unchecked.

@vectorbross -- if you can reproduce this issue, please reopen with detailed instructions to reproduce. If you have a different issue, please open a new issue.