Exposed filter reset redirects user to 404 page on AJAX view when placed as a block

Looks like there are a couple of issues
1. AJAX behavior is explicitly not applied to reset buttons (/core/modules/views/js/ajax_view.js)

// Exclude the reset buttons so no AJAX behaviours are bound. Many things
    // break during the form reset phase if using AJAX.
    $('input[type=submit], input[type=image]', this.$exposed_form).not('[data-drupal-selector=edit-reset]').each(function (index) {

2. Views AJAX is changing the form action to "/views/ajax" somewhere.

My workaround was to implement a form alter and explicitly set the form action to the correct URL.

Encountered this today as well, in 2 different ways:

Firstly I had a page display in the same view as the block and tried writing some behat tests (i,e no javascript).
When the test hit the Apply button it was being sent to the page display with filters applied rather than the page the block was on. To fix that I had to move the page display to a new view, no biggy.

Second issue I've now encountered is as described here, except I get redirected to /views/ajax.

The bug seems to be coming from this line in Drupal\views\Form\ViewsExposedForm

$form['#action'] = $view->hasUrl() ? $view->getUrl()->toString() : Url::fromRoute('<current>')->toString();

When the exposed form is submitted via ajax, it sets the action to views/ajax

Here's a failing test to prove the problem.

I'm having this issue as well but noticed I had a second side-effect caused by the line of code mentioned in #5.

In my view I had several blocks with exposed filters that worked until I added page display's. From that moment the exposed filters on the blocks started using the path's of the first page display in the form action, instead of the "current page".

I don't really understand why the code in views gets to decide where to redirect the form action to. "If it doesn't find a display with a path, redirect to current page". It seems so random to me.

Wouldn't it be better SBX to add a setting "Form redirect path (for example)" within the exposed filters options of each view display (or somewhere else within the view display settings) with a default setting "Current" in which the sitebuilder can define a page display or a path if needed?

Quick fix with the hook_form_alter implementation:

function THEME_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if ($form_id == 'views_exposed_form') {
    if ($form['#id'] == 'FORM ID HERE') {
      $form['#action'] = '/node/XXX';
    }
  }
}

Rerolling on 8.3.x with updated submit filter text as the default has changed.

Fix using the request referer if the current page is identified as the views ajax route. I'm not entirely fond of this approach but with my limited views foo this is what I've come up with. The other option would be to make Reset use ajax as well but that would be a more involved change. Let's see if this breaks anything.

Missed this change to the test.

Expecting this issue at my project.
Patch at #15 fix this problem.
I have one problem - when I'm using ajax at the view, reset button click refreshes current page.
Is it normal in this case?

@anpolimus yeah, that is the normal functionality. One option for fixing this would be to change the reset button to use ajax as well, but I think that is going to be a far broader change (without looking into it), probably best for another issue.

In this case, this isssue is fixed.

Bit of nitpicking:

1. +++ b/core/modules/views/tests/modules/views_test_config/test_views/views.view.test_block_exposed_ajax.yml
   @@ -0,0 +1,81 @@
   +uuid: aaeabe85-2292-4d20-a0ae-8e1afc9c5227
   

   UUID needs to be removed

2. +++ b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php
   @@ -0,0 +1,78 @@
   +    // Verify that only the page nodes are present.
   

   Shouldn't we check then that $this->assertNotContains('Article A', $html);

Thanks @Lendude, fixed those up.

@acbramley++
RTBC++

Needs reroll for #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase.

Using the convert trick in that issue won't work here because only the trailing ) of array() is in the patch.

Patch rerolled.

Thanks @tameeshb reroll looks good, back to RTBC

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -111,7 +111,24 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+        $referer = $this->getRequest()->headers->get('referer');
+        $form_action = parse_url($referer, PHP_URL_PATH);

I'm not sure about this. I think this might under certain circumstances allow an open redirect. We shouldn't trust a referer.

@alexpott yeah I didn't like the referer solution either, but I'm not sure how else to solve it. I tried using the parent request object but that wasn't working either. Any guidance here would be great.

@acbramley I think you should ping @tim.plunkett about it. He might be able to help you out here.

Borrowed the solution from @dawehner in #2866386: Assert the view path is set correctly after second ajax request.

Also, the use of assertWaitOnAjaxRequest() can lead to random fails, so taken that out, but we don't have wait methods for removed elements yet, so added that too.

Thanks, patch looks good to me.

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -111,7 +126,23 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+      if (!$current->getRouteName() == 'views.ajax') {

It can be converted !==.

@jibra, could you please review the below code so that I can create the patch as per comment #31
+ if ($current->getRouteName() !== 'views.ajax') {

Thanks for the feedback @jibran, here we go.

Thanks! @Lendude

Patch #33 looks good and implements #31.
Here is a screengrab. +1 to RTBC

Thanks @Lendude, much nicer solution!

Confirmed test failure locally.

EDIT: It seems like getRouteName() is always returning "" now. Not sure when this changed as it was certainly working before.

Fixed #33 failure

Interdiff, please.

+++ b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php
@@ -0,0 +1,80 @@
+use Drupal\Core\Url;

This can be removed now.

Sorry, forgot to upload the interdiff! Url is still being used here:

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -111,7 +126,23 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+      $current = Url::fromRoute('<current>');

EDIT: Ahh from the test...fixing now.

Last little nitpick:

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -111,7 +126,23 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+      $current = Url::fromRoute('<current>');
+      if ($this->getRouteMatch()->getRouteName() !== 'views.ajax') {
+        $form_action = $current->toString();
+      }

$current can go inside the if now

Here is an updated patch and interdiff.
Implements #45

The last patch is not correct at all. @Dinesh18 please read https://www.drupal.org/patch/apply and https://www.drupal.org/patch first before uploading the patch. Thanks!

Here is an updated patch which implements #45.

Could you please provide interdiff as well

#50 indicates we are missing test coverage here since the test we have is fine with using currentPathStack.

@rsmylski any chance you can indicate what was different in your situation then in the existing test?

I have tested the patch, and it works well. Thanks @rsmylski!

@rsmylski++

Tested that the changed test coverage indeed fails with the fix from before #50, so we have coverage for the problems described in #50, nice work.

1. +++ b/core/modules/views/src/Form/ViewsExposedForm.php
   @@ -111,7 +111,15 @@ public function buildForm(array $form, FormStateInterface $form_state) {
    
   -    $form['#action'] = $view->hasUrl() ? $view->getUrl()->toString() : Url::fromRoute('<current>')->toString();
   +    if (!$view->hasUrl()) {
   +      // Default to the current path.
   +      $form_action = '';
   +    }
   +    else {
   +      $form_action = $view->getUrl()->toString();
   +    }
   +
   

   It's minor, but why change from the ternary to the if/else? The ternary seems more readable here and it'll be a shorter line too.

   Also even with the if/else could it not set $form['#action'] directly instead of the temporary variable? But for me I'd stick with the ternary.

2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php
   @@ -72,6 +72,32 @@ public function waitForElement($selector, $locator, $timeout = 10000) {
    
      /**
   +   * Looks for the specified selector and returns TRUE when it is unavailable.
   +   *
   +   * @param string $selector
   +   *   The selector engine name. See ElementInterface::findAll() for the
   +   *   supported selectors.
   +   * @param string|array $locator
   +   *   The selector locator.
   +   * @param int $timeout
   +   *   (Optional) Timeout in milliseconds, defaults to 10000.
   +   *
   +   * @return bool
   +   *   TRUE if not found, FALSE if found.
   +   *
   +   * @see \Behat\Mink\Element\ElementInterface::findAll()
   +   */
   +  public function waitForElementRemoved($selector, $locator, $timeout = 10000) {
   +    $page = $this->session->getPage();
   

   Do we really not have anything usable for this purpose already?

@catch you're right, the if/else made more sense when were doing more than just setting it to an empty string. I can't comment on point 2.

These are the wait options we have:
waitForButton
waitForElement
waitForElementVisible
waitForField
waitForId
waitForLink
waitOnAutocomplete

So no we don't have anything to wait for stuff to disappear as far as I can tell.

+++ b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php
@@ -0,0 +1,82 @@
+    $this->assertSession()->assertWaitOnAjaxRequest();
+    $this->assertSession()->waitForElementRemoved('xpath', "//text()[normalize-space() = 'Article A']");

But we do need to remove the assertWaitOnAjaxRequest call, which I apparently failed to do :(

Fixes #61

Feedback has been addressed, back to RTBC

That fail looks unrelated.

Codesniffer did find an unused Use though, so lets take that out.

drupal/core/modules/views/src/Form/ViewsExposedForm.php:9 use Drupal\Core\Url;

Fixed #66

@acbramley thank you!

This needs a change record for the new method on WebAssert, but otherwise looks good.

Change record drafted https://www.drupal.org/node/2906685

CR looks good, back to RTBC

Seems like a CI blip

Retest passed, back to RTBC

I wanted to test that this didn't regress or change other behaviors of the block exposed form, for a page that was the frontpage, etc., but I'm having some issues reproducing the bug.

I tested this by making the frontpage AJAX and adding an exposed form in a block with a reset button in HEAD. For me, in HEAD the reset button disappears strangely. If I navigate to e.g. node/1 and apply the filter, I get redirected to the node frontpage (with its node path) and have the reset button, which works. But if I reset the filters and then submit the exposed form from the page of the view itself, the reset button does not show up and I have no way of resetting the view results. Strange... But I was not able to reproduce the 404 at all, in HEAD or with the patch.

https://www.drupal.org/pift-ci-job/556226 does seem to show a failing result so I'll try testing a scenario closer to the test view.

Meanwhile, uploading a new test-only patch since the failing result was on an earlier version of the test and an earlier version of Drupal.

Hi @xjm, sorry but I don't quite understand your comments in #76, are you able to post steps to reproduce your problems? The FAIL patch has correctly failed and the other has passed so is there anything more to do in this issue?

Thanks!

The test fails seems unrelated.

I just tested the patch manually and can luckily confirm RTBC :) It works now!
We've tested it in a multilang environment with translated reset button.

RTBC++ and please commit to next release! Should we perhaps set a higher priority, because it breaks things?

@acbramley, I didn't have "problems"; I was describing what steps I, as a Drupal core committer, took in reviewing the RTBC patch. There were no recent test-only patches until the one I posted. Which is why I posted it. :) Also note that I left the issue at RTBC, which means I didn't think it needed more work.

Re: #82, nah, it's a normal bug because the steps to reproduce it are pretty edgecase-y and specific. Thanks!

@xjm no worries at all! Sorry I must have misinterpreted what you said, thanks for clarifying :)

Thanks for all your efforts, I manually tested the latest patch and seems to work fine, without introducing regressions, and the additional test coverage looks great. However I'm not completely sure about the proposed solution.

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -111,7 +110,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+    $form['#action'] = $view->hasUrl() ? $view->getUrl()->toString() : '';

This doesn't look proper to me for two reasons:

1. We have a view_path POST parameter that's meant to store the original path for AJAX requests. This is exactly what we need here.
2. An empty form action would break if the BASE tag were set.

Therefore I'd suggest to go back to the solution proposed in #49 that in fact relies on the view_path parameter (see ViewAjaxController lines 125 and 149). I think we should solve the double slash issue mentioned in #50 instead of trying a completely different approach.

Of course we would keep all the following improvements the led to #76 :)

We'll also want to add a test case for the form action/base tag issue (I should have caught that).

Btw, also the behavior of $view->getUrl() does not make a lot of sense here: if the display doesn't define a path, it will fall back to the first display it can find that does define one. However this display could be totally unrelated to the original one, which would lead again to an unexpected destination on submit (at least without AJAX). Maybe we could just use the current path and skip the view url logic altogether?

I tried to simply remove the additional slash in ViewAjaxController, line 149, and tests are green, which makes me think we are missing test coverage for that and the HEAD behavior is buggy. See https://www.drupal.org/project/drupal/issues/2938524#comment-12552036.

Hi! I think I have a similar Issue to this.

https://stackoverflow.com/questions/49709766/contact-form-throws-the-req...

I have created a "Contact storage contact form" within a custom page object. When I submit this contact storage contact form directly from the page it works super. But when I submit it from a block it throws error "The requested page could not be found." More specific this block uses the view module to filter these pages.

You can test this yourself by going to http://mijnverkeersovertreding.be/rechtsbijstand

In the input "Postcode" fill in 1910 and press Apply. A contact form appears once. You can fill it in with random data and when you submit it, it throws error The requested page could not be found on url /views/ajax.

Kr.

Btw, also the behavior of $view->getUrl() does not make a lot of sense here: if the display doesn't define a path, it will fall back to the first display it can find that does define one.

@plach You're totally right, I just ran into this situation after adding a new display to a view which was previously just block displays. The new display extended PathPluginBase and therefore was picked up by $view->getUrl() and ended up resulting in a fatal due to missing arguments.

I'm going to have a crack at using view_path like you've suggested.

This should fail. Also cleaned up the deprecated traits.

And the fix. Instead of setting the action always, we can leave it as null for almost all cases as it will then default to the page the form is on. For ajax routes, we override it with an empty string as this seems to ensure the action is set to the path the page is rendered on rather than the ajax path (I'm not sure why this happens, but it works :)).

Still needs tests for the base tag issue mentioned in #85

I applied the #67 patch and everything works fine for me, but since I have applied the #92 an exposed filter block of my search view is not redirecting to the search page (/search) anymore. When the filter is applied from any page, the url still the same but with the search params. So for example, using my filter block on the homepage I'm getting "/?keys=test" instead of "/search/?keys=test".

Patch rolled to take care of #67.

RTBC +1 for #96

We're using this since a longer period of time on several sites now on 8.5.x and 8.6.x, I think we can set #96 RTBC now.

1. +++ b/core/modules/views/src/Form/ViewsExposedForm.php
   @@ -113,7 +112,14 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +    else {
   +      $form['#action'] = '';
   +    }
   

   Do we need the else here?

2. +++ b/core/modules/views/tests/src/FunctionalJavascript/BlockExposedFilterAJAXTest.php
   @@ -0,0 +1,81 @@
   +class BlockExposedFilterAJAXTest extends JavascriptTestBase {
   

   This needs to use WebDriverTestBase now - JavascriptTestBase has been deprecated.

3. As per #93 - still needs tests for the base tag issue mentioned in #85

The last patch from #96 working great but one thing is missing. It's not respecting the destination and it's value. In this case if you reset your filter it will remove the destination from the URL und you wont be able to go back to the origin page!

Current work - since pretty much the beginning - makes sure form action is empty when views is ran through AJAX.
This does fix this issue according to its title - user is not redirected to a 404 page - however the result is a really weird UX due the reset button not having any AJAX event handler attached. The page is consequently re-loaded when you press the reset button and the meaning of having an AJAX exposed form is completely lost. The user needs to scroll the page to find back the view/form, and that's OK if the view/form exists in page on loading and not for example if it's part of a multi-step AJAX process.

On the other side the JS code states this about why reset is excluded from AJAX behaviours:

// Exclude the reset buttons so no AJAX behaviours are bound. Many things
// break during the form reset phase if using AJAX.

So are we only tackling the 404 on reset or the reset not working at all on AJAX?
If the former, than we should state in the IS about the weird UX.

Addressed #100.

Turn out we can't remove #100.1. Fixed reset of the feedback so setting it back to RTBC. @gambry feel free to update the IS.

@jibran I believe what #100.1 means is something like:

$form['#action'] = '';
if ($view->hasUrl() && $this->getRouteMatch()->getRouteName() !== 'views.ajax') {
  $form['#action'] = $view->getUrl()->toString();
}

I'll update the IS as soon, as I need to re-test my assumptions from #102 ("we are fixing the 404, but causing a potential weird UX") still applies.

The current solution produces invalid HTML code: according to the standard, action attribute must be non-empty if it is specified in the form element.

So +1 for #93.
Or to switch to solutions from #2866386: Assert the view path is set correctly after second ajax request which use ltrim(..., '/') to avoid double slashes.

This patch can be tried. It is the combination of #49 and the patch from #2866386: Assert the view path is set correctly after second ajax request. No tests yet.

Patch from #110 fix broken reset button via https://www.drupal.org/project/views_field_formatter embedded views with exposed forms for me.

#110 works great on 8.6.15, thank you!

Is reloading of the page upon reset normal behaviour, even though it is AJAX ?

(@Careless)

I second that ^^ question!

Here's #2820347-110: Exposed filter reset redirects user to 404 page on AJAX view when placed as a block plus the tests from #2820347-105: Exposed filter reset redirects user to 404 page on AJAX view when placed as a block. It applies to 8.7.x and solved this problem for me. However I discovered a *ton* of related issues while tracking this down, conveniently collected in #3055018: URL's generated within AJAX request are re-routed to that AJAX request, that seem to indicate this might be a bigger problem... or just the same mistake repeated in a few different places? Anyway, hope this helps.

Another patch, hopefully passing...?

Bumped into this on a project, thanks for working on it

1. +++ b/core/modules/views/src/Form/ViewsExposedForm.php
   @@ -24,21 +25,35 @@ class ViewsExposedForm extends FormBase {
   +  public function __construct(ExposedFormCache $exposed_form_cache, CurrentPathStack $current_path_stack) {
        $this->exposedFormCache = $exposed_form_cache;
   +    $this->currentPathStack = $current_path_stack;
   

   I realise that constructors are not part of our API, but recently we've been doing this in a BC fashion, e.g. see

   </li>
   
   <li>
   <code>
   +++ b/core/modules/views/views.module
   @@ -60,7 +60,7 @@ function views_views_pre_render($view) {
   -          'view_path' => Html::escape(Url::fromRoute('<current>')->toString()),
   +          'view_path' => \Drupal::service('path.current')->getPath(),
   

   We're losing the Html::escape here - are we sure we want that change?

2. +++ b/core/tests/Drupal/FunctionalJavascriptTests/JSWebAssert.php
   @@ -71,6 +71,32 @@ public function waitForElement($selector, $locator, $timeout = 10000) {
   +  public function waitForElementRemoved($selector, $locator, $timeout = 10000) {
   

   nice!

Confirmed, #110 works for me too.

the patch from #110 fails using composer

Addresses #118.

Adds the deprecation with a reference to a new CR https://www.drupal.org/node/3066604

The failure in #121 is the same issue that we saw all the way back in #87 due to $view->getUrl() returning the url from the first display if the current one doesn't have a path.

This patch fixes #123, however I think this will break displays that do have linked displays.

After chatting about the issue with some colleagues we agreed that it does seem like the functionality of submitting a block's exposed form and being redirected to the linked (or first routed) display is intended and not a bug.

With that in mind, I've rolled back to #121 fixed the test, and added another scenario.

Patch looks good now.

+++ b/core/modules/views/src/Form/ViewsExposedForm.php
@@ -113,7 +132,22 @@ public function buildForm(array $form, FormStateInterface $form_state) {
+      // If we are building an ajax form, don't set the action to the views
+      // ajax route.
+      if ($this->getRouteMatch()->getRouteName() !== 'views.ajax') {

This is non-ajax condition but the comment is about ajax form.

Fixed #127 and changed to use node url without the leading slash which seems cause the failure in #126

Thanks for addressing the feedback this looks good now so setting it to be RTBC.

Committed b6b8e0f and pushed to 8.8.x. Thanks!

Published both change records.

The constructor change makes this too disruptive for a backport, so leaving at 8.8.x, sites impacted by this can continue to run the patch at 129 via composer-patches or similar.

Great news, thanks everyone!

The patch is fixing the issue when there is only one exposed filter.
When there are at least 2 filters, after submitting Reset button you get redirected to the current page with previously applied filters in query parameters (?type=All&field_some_vocabulary_target_id=123&op=Reset) and the following error is displayed:

Drupal\Core\Form\EnforcedResponseException: in Drupal\Core\Form\FormBuilder->buildForm() (line 351 of core/lib/Drupal/Core/Form/FormBuilder.php).
Drupal\Core\Form\FormBuilder->buildForm('\Drupal\views\Form\ViewsExposedForm', Object) (Line: 135)
Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase->renderExposedForm() (Line: 1238)
Drupal\views\ViewExecutable->build(NULL) (Line: 1391)
Drupal\views\ViewExecutable->execute(NULL) (Line: 1454)
Drupal\views\ViewExecutable->render() (Line: 131)
Drupal\views\Plugin\views\display\Block->execute() (Line: 1630)

I am not sure if I need to open another issue since this one was already committed in 8.8 or just re-opening this one.

@andreyjan - please open a new issue and link it here 🏆 - thanks

Just commenting to let you know this patch #129 unfortunately does not applied on Drupal 8.7 (tested on 8.7.7).

#129 is working for Drupal 8.7.8
Waiting for Drupal 8.8.0 :)

This patch didn't apply on 8.8.0 for me.
Thanks @acbramley, I didn't read the whole thread, sorry!

@bbombachini the fix was committed before 8.8.0 so you don't need the patch :)

I know this issue is closed and don't expect any action -- I'm just reporting on my experience. On our project, if using exposed filters with ajax turned on, resetting the filters on a view block sends the user to the first page display that was created. I've followed the advice of another commenter to separate the views as a work around. I'd be interested at some point to hear whether others are experiencing the same.

(Leaving this comment here primarily for folks who land here via Google search.)

@anneeasterling if that is indeed a bug (sounds like it might be), could you open a new issue (if one doesn't already exist), providing as much detail as possible on how to reproduce the problem.

Thanks

Sounds a lot like the issue i am having with ver 4.0.0: #3244756: Submit broken when used with AJAX.

Running into this issue with Version 9.5.0 can anyone help with a patch for current versions of Drupal 9?

Running into this issue with Version 9.5.0 can anyone help with a patch for current versions of Drupal 9?

Same, haven't confirmed that it's happening on D10, but I'm going to submit a new issue.