Ajax exposed filters not working for multiple instances of the same Views block placed on one page

To instances of the same form id will always work this way

This is the use case: In Layout Builder, we want to add the same block to the page more than once, but configured differently each time. If we can't do this, we would have to make more than one identical block type with just a different name and be careful to only add each one once.

As it is, when I add more than one instance of a block containing a View with an exposed filter, the filter forms have the same id attribute. This creates invalid HTML, which Drupal ought to prevent. The forms also don't work as expected.

This patch fixes the problem of multiple forms having the same id attribute.

The Views exposed filters still do not work. They are configured to use Ajax, but they still do a page refresh and use the filtering information from the query string in the URL. This is why multiple View blocks use the same filter settings. Ajax filtering needs to work properly for multiple blocks to work.

Updated steps to reproduce.

I have discovered that having two different Views blocks with exposed filters will not work correctly. It looks like it only works to have one set of exposed filters per page.

Here's the patch for the issue. It's not ideal, but afaik there is no way to determine the relation between a view and a exposed form, especially in a complex cases.

OK, let's try another one

Well, removing form ID altogether was too radical :)

As views exposed form IDs are unique now, loading a form with AJAX adds a random string to the ID. Patch updated

Adding a test showing the issue

Hm, #15 looks like a random failure, and the test only patch does not work as expected, but for the sake of purity it should be changed to rely on the old markup

Let's try D9.2 version, as this won't be committed to D8.9.x (I guess)

Alright, and here is a test-only-patch that does fail "correctly"

There is a very similar issue with D7 #1735096: Allow multiple instances of the same exposed filter form on a single page. The proposed patch is almost the same, however, there are updates to the form caching.

OK, the test-only patch fails as expected

Nice coverage. What we do need to check in the test though that the reload is actually done with AJAX. Calling $assert_session->assertWaitOnAjaxRequest(); doesn't assure that AJAX was used, it could just be that the page reloaded. I'm saying this because some JS changes lead to AJAX stopping to work at all and the exposed form reverting back to just triggering page reloads.

And if at all possible we should get rid of $assert_session->assertWaitOnAjaxRequest(); and replace this with a waitForXYZ method to be more specific in the waits. $assert_session->assertWaitOnAjaxRequest(); leads to random fails unfortunately :(

@Lendude actually it uses AJAX, and without AJAX it will fail because all forms will have the same value (because there are 3 instances of the same form with the same form elements with the same name) -> all views will have the same filter applied and the test will fail. Works by side effect :)

Anyway, I was not aware of the potential issues caused by using $assert_session->assertWaitOnAjaxRequest(). Will try to update the test whenever I have spare time.

@Taran2L, is the patch in #18 the same as #15 except ported to 9.2.x? We need to use this on 8.9.x. Thanks

I'm trying patch #15 on 8.9. It's working except that the reset buttons redirect me to <view-matchine-name>/all/all/all instead of resetting by ajax.

Update: I re-installed my site and I am no longer seeing the above problem.

Replacing usages of $assert_session->assertWaitOnAjaxRequest(); and fixing some minor nits.

Should probably get $assert_session back since it was already there

Also, just realized that we should have more specific selectors

The patch in #31 is the same as the patch in #18 except for assertWaitOnAjaxRequest() being replaced with waitForElement() and waitForElementRemoved() in tests, and some coding standards fixes. An interdiff is attached.

Looks good to me, I was deliberately avoiding such precise selectors (so less dependency on the markup), but seems like this is the preferred way

I have created an issue branch with the patches from #18 and #31, and made a merge request.

Great work here. I just ran into the other case - I have a view consisting of two (or more) blocks (or other view displays), in my example a Map and a results list.

When a user filters the results, all displays of the view should show the same results. On the map and in the result list. I'm not sure if this is or should be in the focus of this issue, but perhaps someone might use this "bug" as expected behavior. Furthermore, it could be helpful to introduce an option to configure that behavior?

I fixed core/modules/views/js/ajax_view.es6.js in the issue fork to comply with prettier formatting. That solves the "Custom Commands Failed" test failure.

That said, to move this forward I assume the issue branch should be rebased on upstream 9.4.x, and the MR updated accordingly, but I'm not sure how to do that.

It needs to be manually rebased in a local repository.

OK, I rebased against 9.4.x. There was a little bit of conflict resolving involved, but I think I got it all sorted out.

Since the rebase required me to do git push --force, anyone who has already checked out the issue fork will probably need to do a hard git reset before continuing to work on it.

I couldn't yet figure out why the test is failing.

It fails in \Drupal\Tests\views\Functional\Plugin\ExposedFormTest::testExposedBlock where it loops over the select options and clicks the apply button for each option. The second iteration of the foreach, $elements[0]->find('css', 'select') returns NULL instead of the expected NodeElement object.

[update]
The part of testExposedBlock mentioned above was added in issue #2681947: Allow 'Put the exposed form in a block' setting to be used with views block displays and committed on the 9.3.x branch in June. So at least that explains why this did not fail before, but only now after rebasing from 9.2.x onto 9.4.x.

All right, I think I found the problem. As said, inside the foreach loop in testExposedBlock, the apply button of the exposed form is clicked. Obviously this triggers a page reload. Before the changes in this issue, the form's id attribute remained the same, so the next iteration could reuse the xpath reference to the form element. This magically worked, even across page loads, but IMHO this was lazy test design waiting to break. I will push a new commit which finds the current form element inside the foreach.

3163299-31.patch cannot be applied after drupal core update to version 9.3.0

hi,
check the merge request.(req 88)
patch apply form #45 is working fine for me. (patch applied successfully, view block results are the same on the page. )
patch applied on Drupal 9.4.x-dev.
thankyou.

Here is a patch of the latest MR for 9.3.x.

As far as I can tell, no one ever addressed the question in #38 by @Anybody. It seems like a potentially valid use case where you want to have one exposed filter block and multiple view block instances that update based on which exposed filters are selected. It seems like the code changes here would disrupt that possibility.

Wondering what this would look like in terms of settings that allow for controlling the behavior. Possibly a view block allow setting to "Choose whether block is unique or connected to other instances." Then a corresponding checkbox in the block form to make the block "Unique" in that it has its own HTML identifier. If it was not unique, then it would fall back to a shared HTML identifier.

+++ b/core/modules/views/js/ajax_view.js
@@ -66,7 +66,7 @@
+    this.$exposed_form = $("form.views-exposed-form[data-drupal-target-view=\"".concat(settings.view_dom_id, "\"], form.views-exposed-form[data-drupal-target-view=\"").concat(settings.view_name, "-").concat(settings.view_display_id, "\"]"));
     once('exposed-form', this.$exposed_form).forEach($.proxy(this.attachExposedFormAjax, this));

Just noticed something interesting going on here. The code originally selects a single form, and then calls attachExposedFormAjax. With the patch applied, this.$exposed_form contains multiple forms, and attachExposedFormAjax is called for each form with the same context. attachExposedFormAjax uses this.$exposed_form as a context to find submit buttons, which will cause each form to attach the same AJAX request to all forms (so 3 forms will cause all 3 forms to perform 3 AJAX requests on submit).

We should probably use the passed element to find submit buttons in attachExposedFormAjax. Attached patch seems to fix the issue.

Not sure why this fails the new test. Might be an issue with the test somehow relying on Drupal.ajax being attached multiple times to each button?

Found there was a problem using entity browser with the patch #51. Changed the approach a bit and use once() to make sure the AJAX calls are only attached once to each button.

Rerolled with the "Rebase" button in GitLab. I think anyone can do this if they click on "Get push access" in the ticket first.

It looks like the patch no longer applies in 9.4.8 (#54 works on 9.4.7).

Made a quick change so the patch applies again.

While the patch in #64 is applying successfully, it doesn't really work as expected. Views with ajax enabled + 2 instances of the exposed form block (Basic style or BEF, tried both) do not do an ajax submit, but it rather refreshes the page which is not the intended behavior.

Hi all,

I can confirm #54 works on 9.4.x
Also, tried patches from https://www.drupal.org/project/drupal/issues/2968207, but none of the patches worked, meaning, views with exposed filters using Ajax just refreshed the page rather then submits using Ajax.
Attaching a reroll, as the patch doesnt apply cleanly on 9.4.x

The patch in #66 does not apply against 9.4.8.

Re-rolling the above patch for Drupal 10x, couldn't generate a interdiff

#54 and #66 do not apply against 9.4.8

PS: #66 has wrong paths with "web" folder..

Instead of posting a new patch, it would be better to update the merge request.

Hi @Liam Morland
That makes sense.
I think you are the owner of PR https://git.drupalcode.org/project/drupal/-/merge_requests/88#note_110635
So may be, you'll have to rebase the PR and change the destination branch to 9.5.x, since it will be the active branch.
After doing that, we will have the working PR/patch for the 9.5.x branch as well.

Meanwhile, I have ported the patch for 10.1.x and fixed test case failures.
All 3 failing tests in #68 are passing on local now.

I changed the branch to 10.1.x. You should be able to get push access to update it.

I was using patch 59, which has stopped working for me in Core 9.4.8. Patch 72 does not apply to Core 9.4.8. I have a use case in 9.4.8 where this applies.

Uploading patch that works with Drupal 9.4.8 as I have critical need of it. Someone else might too.

Thank you @cpierce42. I have confirmed that the patch applies on 9.4.8.

Re-rolled the patch for 9.4.8 with a condition to exclude the 'widget' type. Fixes condition that would break this patch when media library with 'permissions denied'.

test only file.

I have fixed the CCF in #77, attached a patch and interdiff for same. Please review

Reroll for 9.5.x

Patch in #82 doesn't work for us, it removes the ajax_view.js changes present in #80.

#80 applied to 9.5.5 cleanly and works for us.

Changing the target branch of the merge request

1. broke our build
2. made the MR have 700+ commits and 1000+ changes

I created a new merge requests against 9.5.x: https://git.drupalcode.org/project/drupal/-/merge_requests/3687.

Patch #80 work for us with Drupal 9.5.8 not with 9.5.9
Patch #82 doesn't work at all...

This issue should be fixed first for Drupal 10 and then backport if it applies.

I left some comments in the MR from #90. I think is the right approach.

Since the project where I need this is in Drupal 9.5.9 you can find attached the patch for convenience. The rebase is quite trivial.

Ugh. Now the non-empty patch

Arghhh, I think I have found source of the issue why tests are so unreliable, and its #1852090: Cached render elements can have duplicate HTML IDs .. I think the fix is correct, but the test is not due to issues with duplicate form CSS ids.

Can the issue summary be updated to include the proposed solution and any remaining tasks.

I have detected that the block preprocess functions are not triggered for the exposed filter blocks when using this patch.

The problem appears because of this file changes

--- a/core/modules/views/src/Plugin/Block/ViewsExposedFilterBlock.php
+++ b/core/modules/views/src/Plugin/Block/ViewsExposedFilterBlock.php
@@ -32,15 +32,17 @@ public function getCacheContexts() {
    *   context of current view and display ID.
    */
   public function build() {
-    $output = $this->view->display_handler->viewExposedFormBlocks() ?? [];
+    $output = [];
+    $build = $this->view->display_handler->viewExposedFormBlocks() ?? [];
     // Provide the context for block build and block view alter hooks.
     // \Drupal\views\Plugin\Block\ViewsBlock::build() adds the same context in
     // \Drupal\views\ViewExecutable::buildRenderable() using
     // \Drupal\views\Plugin\views\display\DisplayPluginBase::buildRenderable().
-    if (!empty($output)) {
+    if (!empty($build)) {
       $output += [
         '#view' => $this->view,
         '#display_id' => $this->displayID,
+        'content' => $build,
       ];
     }

It seems that adding the block render array inside the `content` key avoids to trigger the preprocess functions.

Reroll for D10.2

There is no need to make a new merge request. You can force-push the branch of the existing merge request and it will use the new code. It will leave behind a tag pointing at the old code.

@Liam Moreland Good to know. Not sure I would have figured that out on my own and I have yet to discover the docs indicating the proper workflow with MRs yet.

Tested MR from #116 over core 10.3.5 on views embedded with viewsreference + viewsreference_filter + better_exposed_filter with success.
Thanks a lot !

Uploading patch from MR 6796 #116 for core 10.3.5

patch 120 work for me , thank you

I can confirm the latest patch also solved this issue for us.

Issue summary is still incomplete and there are multiple MRs but don’t appear to be one for 11.x

This patch fixes only views in blocks, but not exposed forms in blocks as in https://www.drupal.org/project/drupal/issues/3032353, right?
Trying to find out how to fix conflicts between patches from both issues and apply to project I'm working on now.

The 10.3 patch works to fix the issue with multiple views on a page but we have the issue where exposed filters in media library views break when adding blocks with Layout Builder because there is no ajax applied to them. I have created an MR and attached a patch to use the original form IDs for media library views. We're using Gin LB.

@laura.j.johnson@gmail.com

I tried reproducing the media browser issue in vanilla core + layout builder, but I could not reproduce this.
It might be a Gin LB specific issue, which I did not test. If so, gin_lb seems to be the appropriate place to fix this issue.

We discussed this issue at dev days with Lendude & Wannesdr.

For ajax updates of exposed forms, which is required for e.g. Facets 3 users, we need ajax enabled exposed forms exposed in a block to update the form(s) too.

However, some users' usecase is that they explicitly use "expose form in block" to prevent the form from updating, which is the current behaviour.

That's why we suggest introducing an extra setting, only visible when "expose form in block" is set to "yes", which allows users to choose how the block form should behave (update or do not update on form submit). For existing views, we keep the old behaviour as default value.

3163299-views-multiple-ajax-exposed-filters-11.x is a rework of 3163299-views-multiple-ajax-exposed-filters-10.3 which fixes the test and codestyle.

This patch does not include (optional) replacing of the exposed form for e.g. Facets 3.
I'd suggest to keep this patch focussed on replacing the form ID + using a data attribute as selector for AJAX (instead of an ID)
and fix "optionally replace exposed filters in block form" in a follow up.

using a data attribute as selector for AJAX (instead of an ID)

Maybe it worth mentioning that in the latest proposed fix for #3032353: Exposed forms in a block are not updated by AJAX I have introduced a wrapper element around the form that might be also a valid alternative here, although as I wrote in #3032353-63: Exposed forms in a block are not updated by AJAX I do not want to the two issue to become a blocker of each others.

Patch from current state of MR !11923

I found problem with exposed filter on other views after using #120 and #110 patches.
Re rolled and Adding a quick fix in patch.

Thanks

Here is interdiff between patches 110 and 140
thanks

@_pratik_ why update an outdated patch when there is an issue fork with more changes? Why upload a patch when the patch workflow is deprecated?

Update the MR/issue fork if you think your change is relevant.

Hiding patches, closing old MRs

Rebased as main MR was 600+ commits back, tests are still green

Test coverage is here https://git.drupalcode.org/issue/drupal-3163299/-/jobs/6679875 not posting the output but these failed to match the changes from the MR

Drupal\Tests\views\Functional\Plugin\ExposedFormTest::testExposedBlock with data set "page_display"
Drupal\Tests\views\Functional\Plugin\ExposedFormTest::testExposedBlock with data set "block_display"
Drupal\Tests\views\FunctionalJavascript\BlockExposedFilterAJAXTest::testMultipleExposedFormsForTheSameView
Drupal\Tests\views\Kernel\Plugin\ExposedFormRenderTest::testExposedFormRender

Can vouch that the MR fixes scenario 2.

Don't see any open feedback for this one and believe it's been addressed

Attempted to save credit since last time it was updated.

This needs a rebase unfortunately.

<<<<<<< HEAD
    this.$exposed_form = $(
      `form.views-exposed-form[data-drupal-target-view="${settings.view_dom_id}"], form.views-exposed-form[data-drupal-target-view="${settings.view_name}-${settings.view_display_id}"]`,
    );
=======
>>>>>>> c2e40edf7410cd3b6a1df898c3e56d3ddd0f08b2

Which came from #343535: Enable bookmarking of AJAX views

I removed it so lets see if it still works.

I added back the $exposed_form but still appears to be breaking ajax tests so this will need additional work

Seems some tests are failing as the change here is failing to remove empty parameters like 'title='

So removing getUniqueId() got all the tests back to green. Want to get a review before RTBC.

surely the getUniqueId() call is needed as the same view multiple times on the page will end up with duplicate IDs?

That was my thought too so maybe the test case written here wasn’t enough? Because it passes

Curious if anyone has used the MR after the last bit of changes?

Unfortunately, I have some bad news. I replicated the issue as described in the issue summary. The MR does work to fix it. But the form IDs are duplicated without getUniqueId().