Using checkboxes with an "is all of" filter fails if any options are selected

Just a quick +1 to indicate I'm seeing this problem too. For Google-ability, this is the error I get when I try using the pager on a BEF-powered views filter with a bunch of checkboxes where none are checked:

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

I have the same issue as described in #3 when using the "Remember the last selection" option for a filter using checkboxes. In the watchdog I get this message:

Illegal choice 0 in field_tags_target_id element.

In the frontend I get this message:

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

The patch in #2 doesn't fix this issue.

Apologies for the delay getting to this issue... I appreciate your patience! (And thank you, @Lukas von Blarer, for pinging me in the other issue.)

Using the latest 8.2.x core and BEF, I'm not able to reproduce the original issue. (Though it's been long enough since it was reported that it may have been fixed by a change in core?). But I am getting a variation on it. STR:

1. Enable the bef_test module and generate a bunch of "BEF Test" content using Devel
2. Create a view with an exposed "bef_letters" filter, tick "Allow multiple selections"
3. Enable BEF and set the bef_letters filter to render using checkboxes
4. Change the pager to a full pager showing 5 items

When I navigate to the page for this view and click around in the pager it works as expected. If I select a checkbox and apply it, it works as expected for the initial page. If I then click a pager link, I get something similar to letters%5Ba%5D=a&a=a&page=1 (note the extra &a=a, which continues for each option checked). The patch in #2 doesn't remove the additional &a=a.

Can someone provide steps to reproduce the Cascade-of-Zeros or confirm that it's no longer happening? Thanks.

Yes, this exactly how it works for me as well by reproducting the steps described. I also attached the exported view to make it easier to reproduce. There is a step missing in your instructions. The issue I am facing is when having the "Remember the last selection" option active for the filter. After activating that remove the GET parameters from the URL and you should get the error I described.

I needed something along these lines ... to at least get rid of the fatal.

Wow, that solved the issue for me. Thank you!

What is missing to fix this properly?

@dawehner - Your patch works great for both the view I was initially having trouble with, as well as another view that I had the same problem with but on a different type of filter. So thanks!

+1 from me, please merge to make the module usable without the patch for exposed checkboxes/multi-select.

Of course it would be super nice to have some tests ...

@amateescu
That is a fair question! I guess we would want to apply this cleanup just for GET forms.

@ameteescu Yes I also thought about that... Would make sense...

This patch fixes the error message, but the functionality is inconsistent. Sometimes it works, sometimes it doesn't. I can't really see any logic in which cases it breaks. Any ideas?

All, Can this issue be fixed in core ? I am not using BEF and i am facing the same issue!

Thanks,
Sukanya

Submitted a patch for this issue at https://www.drupal.org/node/2651102#comment-11542843

Please review and let me know if it works!

Thanks
Sukanya

Changing project to core to provide a fix based on changes made for 2651102 - using checkboxes for exposed filters results in zero results displaying.

Submitting a patch for core. My proposed fix is based upon my debugging and subsequent discovery that $view-> exposed_raw_input doesnt contain the checkbox values in the right format which causes form_state to not retain values of the checkbox properly upon pager navigation.

Submitting a patch for 8.1 and 8.2(both are essentially the same but still submitting two patches to avoid confusion).

8.1 should be applied only after applying https://www.drupal.org/node/2651102 patch from #83.

Thanks!
Sukanya

Updated patch for 8.2, there were problems introduced when there were multiple exposed checkbox filters with values set in each filter.

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -181,7 +181,10 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
+            $checked_array[$option_id] = $value[$option_id];

What was happening was that after each checkbox filter was processed, the $checked_array variable persisted values from previous filters. I've uploaded an updated patch file with the proposed fix for #18 attached.

What about filtering out unchecked checkboxes at the ManyToOne filter handler instead?

No interdiff since this is a completely different approach. Curious what the testbot thinks, if it comes back green I'll work on tests.

What about filtering out unchecked checkboxes at the ManyToOne filter handler instead?

No interdiff since this is a completely different approach. Curious what the testbot thinks, if it comes back green I'll work on tests.

This approach has the side effect of fixing #2821515: "is all of" filter operator doesn't work with checkboxes as well, which makes my life better! :)

No idea why it double-posted when I was just trying to edit... Sorry about the noise.

Is the IS still up to date? I see some things about not being able to reproduce the original issue and some new issues being introduced in 8.2. The title makes this look like a task, but it's a bug.

Some clear steps to reproduce or a test would really help show what we are trying to fix here.

Is the IS still up to date?

Nope, it was way out of date but should be better now. Add STR and noted the two possible options to fix the issue. Also retitled to clarify the issue.

Thanks for pointing that out @Lendude.

@mikeker thanks for the IS update, makes much more sense now!

I like the look of the fix in #24, it could use an inline comment about why we are doing that, and it obviously still needs tests, but that looks like a good direction to take.

Sorry for taking so long to get back to this issue...

Attached is a tests-only patch which should highlight the issue and the patch from #24 (plus an inline comment, as requested by @Lendude). The tests-only patch is essentially the interdiff.

The failure in #31 seems to be unrelated. Regardless, I've made the coding standards change in this patch.

Forgot to mention in #31, that I refactored the checkboxes tests into their own files as I didn't want to view changes to affect the other exposed form tests.

@mikeker++

just some nits:

1. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormCheckboxesTest.php
   @@ -0,0 +1,171 @@
   +  use \Drupal\field\Tests\EntityReference\EntityReferenceTestTrait;
   

   Lets not use a fully qualified name here. (See all the other uses of EntityReferenceTestTrait).

2. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormCheckboxesTest.php
   @@ -0,0 +1,171 @@
   +  /**
   +   * Views used by this test.
   +   *
   +   * @var array
   +   */
   

   Can just be @inheritdoc

3. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormCheckboxesTest.php
   @@ -0,0 +1,171 @@
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   

   Can just be @inheritdoc

4. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormCheckboxesTest.php
   @@ -0,0 +1,171 @@
   +    ViewTestData::createTestViews(get_class($this), ['views_test_config']);
   

   lets use self::class instead of get_class, it's what the cool kids do I've been told ;)

5. +++ b/core/modules/views/tests/src/Functional/Plugin/ExposedFormCheckboxesTest.php
   @@ -0,0 +1,171 @@
   +    // Create two content types
   

   missing period at the end.

@Lendude, thank you for the review! Nits have been picked. :)

@mikeker thanks for pickin' them!

Committed/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!