When using a text field as an exposed grouped filter with the checkbox to allow multiple selections, I get the following error when trying to use the pager to go to the next page of results:

An illegal choice has been detected. Please contact the site administrator.

This shows up in the watchdog as:

Illegal choice 0 in field_presenter_last_name_value element.

Using Better Exposed Filters with the exposed form in a block.

I've attached screenshots and the display export.

Comments

rollsd’s picture

Confirming this bug. It still exists in 7.x-3.5, and on 7.x-3.x-dev branch.

This not only affects pagination links, but anything that uses the
views->raw_exposed_filters property to generate links. Table click-sort links are also affected by this.

To reproduce this error one can do the following on a test site:

  1. Install the Devel generate module and go to /admin/config/development/generate/content
  2. Select Article content type only and generate some nodes (eg 50).
  3. Create a new view by importing: test-search-exported-view.txt
  4. Try click on the column sorting for title on the view or pagination options.

You should see something like the following:
An illegal choice has been detected.

rollsd’s picture

StatusFileSize
new766 bytes
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

The problem is that pagination and quicksort links are being created from a set of inputs that does not understand that checkboxes set to zero should not be sent as part of the request.
Have found a fix that uses array_filter to remove values that have a value of zero, if it is a multiple exposed checkbox.

function views_exposed_form_submit(&$form, &$form_state) {
  foreach (array('field', 'filter') as $type) {
    $handlers = &$form_state['view']->$type;
    foreach ($handlers as $key => $info) {
      $handlers[$key]->exposed_submit($form, $form_state);
    }
  }
  $form_state['view']->exposed_data = $form_state['values'];
  $form_state['view']->exposed_raw_input = array();

  $exclude = array('q', 'submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', '', 'reset');
  $exposed_form_plugin = $form_state['exposed_form_plugin'];
  $exposed_form_plugin->exposed_form_submit($form, $form_state, $exclude);

  foreach ($form_state['values'] as $key => $value) {
    if (!in_array($key, $exclude)) {

      // If we have a multiple checkbox with values set to zero, we don't want
      // these included in the exposed_raw_input array as it breaks pagination.
      if (is_array($value) && $form[$key]['#type'] === 'checkboxes') {
         $form_state['view']->exposed_raw_input[$key] = array_filter($value);
      } else {
        $form_state['view']->exposed_raw_input[$key] = $value;
      }
    }
  }
}

rollsd’s picture

Version:7.x-3.7» 7.x-3.x-dev
Status:Active» Needs review
rollsd’s picture

Note: better exposed filters introduces a complication here as you have the option to move fields into a field-set named secondary (ie by making them a secondary filter). The above fix doesn't work in this case as it would need to check $form['secondary'][$key] for the type.

rollsd’s picture

StatusFileSize
new1.11 KB
PASSED: [[SimpleTest]]: [MySQL] 1,652 pass(es).
[ View ]

Have modified this patch so it checks any children on the form (such as fieldsets) if it cannot find the $key directly on $form

function views_exposed_form_submit(&$form, &$form_state) {
  foreach (array('field', 'filter') as $type) {
    $handlers = &$form_state['view']->$type;
    foreach ($handlers as $key => $info) {
      $handlers[$key]->exposed_submit($form, $form_state);
    }
  }
  $form_state['view']->exposed_data = $form_state['values'];
  $form_state['view']->exposed_raw_input = array();

  $exclude = array('q', 'submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', '', 'reset');
  $exposed_form_plugin = $form_state['exposed_form_plugin'];
  $exposed_form_plugin->exposed_form_submit($form, $form_state, $exclude);

  foreach ($form_state['values'] as $key => $value) {
    if (!in_array($key, $exclude)) {

      $form_state['view']->exposed_raw_input[$key] = $value;

      // If we have a multiple checkbox with values set to zero, we don't want
      // these included in the exposed_raw_input array as it breaks pagination.
      if (is_array($value)) {
        $field = isset($form[$key]) ? $form[$key] : NULL;

        if ($field === NULL) {
          // The field may be burried in a fieldset somewhere so find it.
          foreach (element_children($form) as $child) {
            if (isset($form[$child][$key])) {
              $field = $form[$child][$key];
              continue;
            }
          }
        }

        if (isset($field) && $field['#type'] === 'checkboxes') {
          $form_state['view']->exposed_raw_input[$key] = array_filter($value);
        }
      }
    }
  }
}

Darren Oh’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community

The patch in comment #5 works.

markocall’s picture

The patch in #5 worked for me as well, thanks a lot!

dawehner’s picture

Status:Reviewed & tested by the community» Needs work
Issue tags:+Needs tests

To be honest, this amount of change is to awkward to be just committed without some serious test coverage, I am sorry.

rreiss’s picture

The patch in #5 worked for me. Thanks @rollsd !

enjaygee’s picture

Thanks @rollsd -- the patch in #5 worked for me as well.

rmathew’s picture

The patch in #5 resolves the "illegal choice" issue for me, however the pager URL is not quite right. eg:

http://[...]=All&search_api_views_fulltext=digital&&&page=1

Note the "&&&" - due to two checkbox values being ignored by the patch.

Crell’s picture

I just ran into this issue as well. I am seeing the &&& in the URL even without this patch, so that appears to be a separate issue.

Fendo’s picture

#5 worked for me too. only thing I wasn't aware need to logout and login again sessions to see changes.

mr. chips’s picture

I had to change one line in #5 patch:
from this
if (isset($field) && $field['#type'] === 'checkboxes') {
to this
if (isset($field['#type']) && $field['#type'] === 'checkboxes') {
or I would get this:
Notice: Undefined index: #type in views_exposed_form_submit() (line 2079 of ..... httpdocs/sites/all/modules/views/views.module)

I have several exposed filters with some as checkboxes, filtering entities from a custom table. I get the notice whether or not I use hook_form_alter. I can't duplicate that notice by filtering on Body of content type Article.

Other than that it worked for me. Paging was my problem before - if a checkbox group was left un-checked it would throw an error and check ALL the boxes in that group when going to the next page of results.

fietserwin’s picture

Same as #14, patch works but is giving undefined index notices. So that case should be handled as in #14.

BTW: BEF does not have this problem, as it leaves the #type to select, but changes the theme, and only in the theme function it renders checkboxes instead of a select.

rcodina’s picture

Patch on #5 works for me too.

Darren Oh’s picture

Issue tags:-Needs tests

I can understand holding up a feature until it has tests, but continuing to release this module without a bug fix that passes existing tests makes no sense.

fietserwin’s picture

Status:Needs work» Needs review
StatusFileSize
new1.12 KB
Test request sent.
[ View ]

Reroll of #5 (line number offset) + #14