Viewsform has incorrect form submit URL if loaded through ajax

@jefuri :wave:

As the test failures show, this part of Views is really fragile. Yeah we want some serious test coverage for this.

Similar to the previous patch but retaining the query options.

I'm still seeing this issue on Drupal 9.5. Patch from #11 did not fix the issue for me, but the patch from #5 did. Not sure why the new patch didn't work. Some investigation is needed.

My use case is a modified version of the commerce cart as well which uses ajax to update the quantities when they are changed. I'm seeing the action of the form change when the views form is replaced.

Updating the target to 11.x since that's the new standard and this code looks unchanged since the 9.5 release.

Neither patch fixes the issue in every case. It's not just broken for Viewsform, but also for ViewsExposedForm.

See #3244756: Submit broken when used with AJAX.

If I have a view with exposed filters that has a Data Export view display and a Block view display, then even if the Block display is alone on a page by itself, without the Data Export attached to it, the Apply button will still incorrectly have the URL to the data export.

When using xDebug, I stepped through the code in ViewsExposedForm.

It has:

    if ($view->hasUrl()) {
      try {
        $form_action = $view->getUrl()->toString();
      }
      catch (InvalidParameterException $e) {
        $form_action = NULL;
      }
    }

The view does have a URL, and it returns the URL for the wrong display here, returning the Data Export URL if it has one. That happens because of this code in DisplayPluginBase:

  public function getRoutedDisplay() {
    // If this display has a route, return this display.
    if ($this instanceof DisplayRouterInterface) {
      return $this;
    }

    // If the display does not have a route (e.g. a block display), get the
    // route for the linked display.
    $display_id = $this->getLinkDisplay();
    if ($display_id && $this->view->displayHandlers->has($display_id) && is_object($this->view->displayHandlers->get($display_id))) {
      return $this->view->displayHandlers->get($display_id)->getRoutedDisplay();
    }

    // No routed display exists, so return NULL
    return NULL;
  }

and getLinkDisplay()...

  public function getLinkDisplay() {
    $display_id = $this->getOption('link_display');
    // If unknown, pick the first one.
    if (empty($display_id) || !$this->view->displayHandlers->has($display_id)) {
      foreach ($this->view->displayHandlers as $display_id => $display) {
        if (!empty($display) && $display->hasPath()) {
          return $display_id;
        }
      }
    }
    else {
      return $display_id;
    }
    // Fall-through returns NULL.
  }

So despite the fact that the block is on a page with its own route, it uses the Data Export URL, which is wrong.

The views code should not assume that the first link it finds on a display will be valid for a display without a link.

However, it does reveal a workaround. I expect that adding a Page display with the correct URL and putting it right after the displays without links would make the views form have the correct URL on the form action.

I want to draw attention to this issue again.
This is a fairly old issue that is still reproducible and that would be desirable to close. I personally encountered this bug on 4 projects.
To resolve the bug caused by this issue, this patch or a custom hardcode has always been used.

I am not sure how to reproduce it on a clean core, but it is a fact that it has an impact on other modules (contrib/custom) that require the correct form action when using AJAX.

This patch does not make any major changes, but only adds an additional scenario for the case when the current route is 'views.ajax'

Added steps to reproduce in core. Patch works to fix this, although on the front page you are sent back to /node instead of /. Needs work for test coverage.

Added a test and rearranged the logic a bit

RTBC from my side
The patch works as expected and the new test looks good.

Many thanks to @mstrelan for providing the test and steps to reproduce

ViewsExposedForm hasn't been fixed yet. See #17

@solideogloria I think that's a slightly different scenario and I think we should raise a separate issue. FWIW you can set the "Link Display" of the block display to a Custom URL (it's weirdly in the Pager part of the configuration) which would fix this issue.

Created #3396365: ViewsExposedForm has incorrect form submit URL if loaded through ajax

I'm triaging RTBC issues. I read the IS.

The steps to reproduce look great, although I haven't tried them and rhere are still items in the remaining tasks. The proposed resolution says this was fixed in other cases. Are there other cases that need to be checked? Do we need a meta for this work? (I have not searched to see if one exists).

I then read the change record. Instead of a statement of the bug and the error it should include actions required by the reader to make any changes needed in their core or site.

Next, I started the test only job in GitLab.

I then read the comments them and see a followup has been made for Exposed forms. However, the proposed resolution implies that exposed forms have been fixed. This goes back to my questions about maybe needing a meta to organize the work. I also do not see that anyone has reviewed the MR and there are not comments on the MR.

Back to the test job. The results show the test ran successfully when it should have failed. It also has a deprecation notice. That needs to be sorted.

I have updated the remaining tasks with the items I found. Setting to needs work.

Marked #1109980: Exposed Form Reset button Inherits the page display URL when using as a block and AJAX as a duplicate of this.

Replying to #29

The proposed resolution says this was fixed in other cases. Are there other cases that need to be checked? Do we need a meta for this work? (I have not searched to see if one exists).

It's just pointing out other areas where similar bugs have been fixed. I only know of this issue and the other one #3396365: ViewsExposedForm has incorrect form submit URL if loaded through ajax so far so not sure we need a meta.

I also do not see that anyone has reviewed the MR and there are not comments on the MR

I've reviewed the MR, it looks good mostly but we need some work on the new constructor params.

It also has a deprecation notice. That needs to be sorted.

I don't see any failures in the gitlab pipelines, deprecations would trigger a failure? Where were you seeing it?

I've also rebased the MR.