View's pagination doesn't carry the exposed filter value

Hello! I fixed the problem with this patch.

I think that the test that this patch failed should be updated to reflect the new functionality of the exposed form.

I can confirm both the issue, and the fact that the patch #6 is working.

Confirmed patch in #6 fixes the issue. Marking as RTBC.

It'd be great to add an actual test of the bug discovered here. The test change here is related but obviously we've not got test coverage of the specific bug.

Here's a test.

diff --git a/core/modules/views/src/Form/ViewsExposedForm.php b/core/modules/views/src/Form/ViewsExposedForm.php
index e9d5fa4f41..0412c11c00 100644
--- a/core/modules/views/src/Form/ViewsExposedForm.php
+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -177,18 +177,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
     $exposed_form_plugin->exposedFormSubmit($form, $form_state, $exclude);
     foreach ($values as $key => $value) {
       if (!empty($key) && !in_array($key, $exclude)) {
-        if (is_array($value)) {
-          // Handle checkboxes, we only want to include the checked options.
-          // @todo: revisit the need for this when
-          //   https://www.drupal.org/node/342316 is resolved.
-          $checked = Checkboxes::getCheckedCheckboxes($value);
-          foreach ($checked as $option_id) {
-            $view->exposed_raw_input[$key][$option_id] = $value[$option_id];
-          }
-        }
-        else {
-          $view->exposed_raw_input[$key] = $value;
-        }
+        $view->exposed_raw_input[$key] = $value;
       }
     }
   }

Appears to be an alternate fix. I'm not sure what checkboxes have to do with this as we've got no checkboxes exposed. I can;t see how to expose checkboxes - i've tried to expose a checkbox field but that didn't work.

@alexpott the checkboxes were for when you use Better Exposed Filters, it's not a core thing that broke that put that in. See #2651102: Using checkboxes for exposed filters results in zero rows displaying.

But that added test coverage for the checkbox use case in \Drupal\Tests\views\Functional\Plugin\ExposedFormCheckboxesTest, so if that is still green with the alternate fix.....

I discovered this in #3024672: The views exposed form incorrectly assumes all array based filter values are checkboxes and thus discards all filter values of "0" and @Lendude kindly pointed out this issue.

I can't see how to expose checkboxes - i've tried to expose a checkbox field but that didn't work.

This is a bit of a red herring, this actually operates on any input value which is an array, so multi-value select lists are also processed by this. So, essentially the implementation in #12 would include all values in a multi-select in the URL, instead of just the ones the user has selected.

For two selected terms, links would look like:

?field_foo[500] = 500
&field_foo[501] = 501
&field_foo[502] = 0
&field_foo[503] = 0
&field_foo[504] = 0
&field_foo[505] = 0

Instead of:

?field_foo[500] = 500
&field_foo[501] = 501

For lists with thousands of terms, this might be an issue.

I really like preserving the keys, like the approach in #11. I also thought that might be viable in #3024672#2, so if that passes the tests, I think that solution works well.

For an equal comparison of this approach, it would mean:

?field_foo[500] = 500
&field_foo[501] = 501

Instead of HEAD, which is:

?field_foo[] = 500
&field_foo[] = 501

One issue remains with this whole method: since all array query params are still processed as if they are checkboxes, "0" will never actually be preserved for arrays, regardless of the filter type. So if ?field_date[min]=0 was valid somehow, that would also drop off in the next paginated URL.

This is actually broken in the exact same way in HEAD right now and fixing this issue as it is doesn't make it any better or worse, so I think these are actually two bugs with the same chunk of code.

With that in mind, I would suggest we RTBC and fix this, and then repurpose and follow up in #3024672: The views exposed form incorrectly assumes all array based filter values are checkboxes and thus discards all filter values of "0" to dive deeper into the aforementioned issue.

I have repurposed the other issue to describe the bug in #16: #3024672: The views exposed form incorrectly assumes all array based filter values are checkboxes and thus discards all filter values of "0".

The line in question references #342316: Introduce proper Form API #types for 'option' and 'optgroup', and make #options consistent. - however that is a long way off.

I agree with @Sam152s assessment - this doesn't make things any worse but I feel its a bit of a half-way fix. Assuming any array value is a checkbox feels wrong.

I think the size of the patch in #3024672: The views exposed form incorrectly assumes all array based filter values are checkboxes and thus discards all filter values of "0" makes it a candidate for fixing the issue properly here, instead of punting half of it to later.

Thoughts?

@larowlan I think we should fix separate bugs as we find them - I don't think the size of a patch on another issue should influence whether we combine efforts. They might be related yes but solving them in separate issues is fine.

Hi Alex, I have tested your patch from https://www.drupal.org/project/drupal/issues/2987598#comment-12867202

For my use case it is working perfect. I also enabled Ajax on the view and it keeps working nicely in combination with the pager: the date range filter is keeping its value.

Thanks!

Re-upping #11 because the test only patch somehow got marked for daily retest, which is useless and annoying

Committed fcf5177 and pushed to 8.8.x. Thanks!

c/p to 8.7.x