Prevent _wrapper_format and ajax_form parameters from bleeding through to generated URLs

I'm confused. These are part of the URL, how would you want to remove them?

I think MainContentViewSubscriber should have a public method like getRequestWrapperFormat() around a protected variable, and subscribe to the REQUEST event where it sets that protected variable and then does: $request->query->remove(static::WRAPPER_FORMAT).

Similar for the _ajax_form parameter, but not sure what service should wrap that one.

#2 answers the "how?" question from #1, but if you meant "why?", then because that these are query parameters is a workaround to unfortunate implementation limitations. In an ideal setup, they wouldn't be query params, so cleaning up $request to match that more ideal setup would be a good thing, I think, and allow various link generation code elsewhere to not have to worry about it.

I think MainContentViewSubscriber should ... subscribe to the REQUEST event

Downside of that is then $main_content_renders need to be constructed that early. Maybe a separate service would be good then.

A test-only patch, mostly taken from work done in #2182555: On Ajax-enabled forms with a pager, pager is disappearing on form validation error and on Ajax form reload.

Since #2263569: Bypass form caching by default for forms using #ajax. was committed, #2182555: On Ajax-enabled forms with a pager, pager is disappearing on form validation error and on Ajax form reload has been fixed as well as AJAX forms no longer get cached.

This patch now demonstrates how, on AJAX rendered forms, the _wrapper_format and ajax_form parameters leak in the page links.

EDIT: #2263569: Bypass form caching by default for forms using #ajax. was committed.

I think MainContentViewSubscriber should ... subscribe to the REQUEST event

I actually disagree, well at least in the current architecture, the controller returns a result and KernelEvents::VIEW convert that result into a request object.

#2 answers the "how?" question from #1, but if you meant "why?", then because that these are query parameters is a workaround to unfortunate implementation limitations. In an ideal setup, they wouldn't be query params, so cleaning up $request to match that more ideal setup would be a good thing, I think, and allow various link generation code elsewhere to not have to worry about it.

Well, maybe the issue title is just chosen not perfectly. I think we should never alter the incoming request, its a detail you should not change, but yes, there should be a service which returns you all the query parameters, maybe the route match could hold such information, for example.

I think we should never alter the incoming request

That's a good point. These query params are part of the request, so we shouldn't remove them from $request.

The added request parameters bleed into links.

That's the problem that needs solving. We have various code that does stuff like UrlHelper::filterQueryParameters(\Drupal::request()->query->all(), .... I wonder if we want to change filterQueryParameters() to automatically add _wrapper_format and _ajax_form to the exclude list. But should that list then be extensible (i.e., for contrib to use their own custom query params that they don't want bleeding into links)?

That's the problem that needs solving. We have various code that does stuff like UrlHelper::filterQueryParameters(\Drupal::request()->query->all(), .... I wonder if we want to change filterQueryParameters() to automatically add _wrapper_format and _ajax_form to the exclude list. But should that list then be extensible (i.e., for contrib to use their own custom query params that they don't want bleeding into links)?

Sure, I totally agree with that part.

The UrlHelper would be a bit unfortunate though, because its static and we should avoid adding yet another public global state. What about putting it into RequestContext->getQueryParameter() ?

This may be the fault of the form system, but I think we're leaning toward a solution within the routing system.

I'll give a specific use case on the 'why' these should be removed. I'm building a list of results that has a pager. The results are updated using ajax. The pager is built using a render element of #type 'pager'. When the pager is built, it pulls all the query parameters, including 'wrapper_format' and 'ajax_form'. So when the user clicks on a page, they are redirected to an ajax result, rather than the next page of results. There is no way to override this, as the URLs are built automatically based on the current GET parameters, and these values are included in the GET parameter of an #ajax call.

This now blocks #2905676: ajax.js overwrites WRAPPER_FORMAT when already specified

I believe the new issue title is accurate?

We are having the same exact issue as #16. As a workaround, we're overriding the "wrapper_format" parameter in the pager render array:

 $pager = array(
        '#type' => 'pager',
        '#quantity' => 3,
        '#parameters' => ['_wrapper_format'=>''],
    );

It makes sense to keep query parameters from the URL for the links, but it doesn't make sense to add an AJAX wrapper format to a non-AJAX link.

Nice workaround! I'll be sure to use that in the future.

While passing a "wrapper_format" parameter to the pager is working, I'm having the same problem with the "sort" headers on a table render array and cannot find a workaround. This is definitely a major issue; one cannot use AJAX on the same page as a pager, sortable table headers, etc., without breaking the resulting functionality.

I found a patch in the following issue to resolve the table pager and sort header issues but I'm not sure about the approach: https://www.drupal.org/project/drupal/issues/2925598

This and the below issue are proving to be devastating to the app we are attempting to build (I've posted this on both, apologize for redundancy but am really looking for some creative solutions).

#2504115: AJAX forms should submit to $form['#action'] instead of <current>

We have 4-5 pages where the majority of our functionality takes place, but almost everything happens via forms, modals and AJAX.

For example, let's we have a page with a list of users. Each user has certain actions such as "edit". When you click "edit" it uses AJAX to load a form with the user's data in a modal, with save and cancel buttons. At the top of page is a checkbox called "Display only active users". When the checkbox is clicked, a global variable is set, a new user list is pulled, and the current user list is replaced via AJAX.

The Problem: The new user list was built in the AJAX callback for the checkbox, and is wrapped in all of the AJAX data, query parameters, wrappers, URL's, etc., of the checkbox. This breaks all of the AJAX functionality for the user list, including the aforementioned "edit" link. It also builds all non-AJAX links as AJAX links, which break table headers, pager links, etc.,

I want to make sure I am taking the right approach and not missing anything, because this appears to be a pretty common use-case for AJAX.

Another related issue - #2933413: Improve test coverage of using bulk actions when the view has an exposed form using AJAX

Solution for #2933413: Improve test coverage of using bulk actions when the view has an exposed form using AJAX should solve all the functional problems forms in AJAX views are having, but the leaking parameters in links will remain. I'd put that data in a js variable after an AJAX request instead.

I'm confused and apologize. The referenced Views issue seems to be implementing a workaround for a core issue, which is AJAX form action URL's are being written incorrectly, and reference any past AJAX request which the form was built in (https://www.drupal.org/project/drupal/issues/2934463). Am I using forms wrong?

The request query parameters for ajax_wrapper's don't seem to be consistently or properly implemented. I still don't understand why I should have to specify or strip out the wrapper format manually (ajax_form, drupal_ajax, etc.,) when I code my forms and link render arrays; shouldn't Drupal core be smart enough to build these correctly? And if it's not, how I can fix it so it is? Because I would like to help if I can.

The workaround posted in the linked issue by mukila involves:
- Specifying a button with an AJAX callback URL (from a custom route that needs to be created; can't just use a callback)
- Specifying a direct override of the $request->query ajax_form parameter (set ajax_form = 1)

Why can't we just use a regular form submit AJAX callback?

I think this related issue needs to be solved in combination with this one, in order to get the AJAX built within AJAX issue fixed.

So #2798947: Views pagers include ajax metadata has resulted in this being fixed for views. However that patch is doing what we're saying we don't want. Which is changing the request. If we work on stuff here then we probably need to undo the changes that issue has made.

Currently I am having issues with the wrapper_format bleeding through my urls in the Core modal popup, this is my scenario:

I am using Drupal core modal popup and rendering node detail in the popup.

    return [
      '#type' => 'link',
      '#title' => $this->t('Read on'),
      '#url' => Url::fromRoute('entity.node.canonical', ['node' => $entity->id()]),
      '#attributes' => [
        'class' => ['use-ajax'],
        'data-dialog-type' => 'modal',
        'data-dialog-options' => Json::encode([
          'title' => '',
          'maxWidth' => 915,
          'width' => '80%',
          'height' => 'auto',
          'maxHeight' => '3000',
          'closeOnEscape' => TRUE,
          'closeText' => $this->t('Back to selection'),
        ]),
      ],
    ];

In this popup multiple fields and entities are rendered from the node detail view. Some of these fields are rendered links, the wrapper_format is bleeding trough in all these links and when clicked I see this:

As a workaround we are going to remove the wrapper_format query parameter from all urls in the popup with javascript. Does anybody have a better solution for this? Also tried removing these query parameter with hook_link_alter(), but this doesn't work.

Fixed the title (Don't know why, but _wrapper_format starts with an underscore, while ajax_form doesn't.) and reset to "Active" as apart from a test there's not yet a patch in this issue.

Just in case someone else runs into this, I was getting this issue with custom ajax requests on views (not pager) and the solution proposed by @effulgentsia on #2 seems to work very well.

I added an Event Subscriber that strips out the wrapper format on REQUEST in a custom module, leaving it here in case it's useful for someone else.

<?php

namespace Drupal\my_module\EventSubscriber;

use Drupal\Core\EventSubscriber\MainContentViewSubscriber;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;

/**
 * Request subscriber that removes the WRAPPER_FORMAT.
 *
 * @see https://www.drupal.org/project/drupal/issues/2504709
 * @see \Drupal\Core\EventSubscriber\MainContentViewSubscriber
 */
class AjaxFilterSubscriber implements EventSubscriberInterface {

  /**
   * Filters out the ajax wrappers from the query.
   *
   * @param \Symfony\Component\HttpKernel\Event\GetResponseEvent $event
   *   The event to process.
   */
  public function onRequestAjaxFilter(GetResponseEvent $event) {
    $request = $event->getRequest();
    $request->query->remove(MainContentViewSubscriber::WRAPPER_FORMAT);
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[KernelEvents::REQUEST][] = ['onRequestAjaxFilter', -10];

    return $events;
  }

}

Any progress on this? I'd like to help if I can. I am stuck on the table sort headers at the moment -- the only recourse seems to be using JavaScript to manually strip the _wrapper_format parameter out of the each table header URL whenever a table is rendered by AJAX.

EDIT: I am going to try to modify JQuery I stole from here: https://drupal.stackexchange.com/questions/214358/drupal-tableselect-in-...

EDIT 2: The ^^ above JQuery doesn't work. It does strip the parameters off the table sort URL, which is great, but for some reason the table sort header Links are using the URL for the AJAX request the table was built within, while the Pager links stick to the main page.

It looks like views completely rewrites these in their own ViewAjaxController and references this issue in a comment:

From views\src\Controller\ViewAjaxController.php:

      // Remove all of this stuff from the query of the request so it doesn't
      // end up in pagers and tablesort URLs.
      // @todo Remove this parsing once these are removed from the request in
      //   https://www.drupal.org/node/2504709.
      foreach ([
          'view_name',
          'view_display_id',
          'view_args',
          'view_path',
          'view_dom_id',
          'pager_element',
          'view_base_path',
          AjaxResponseSubscriber::AJAX_REQUEST_PARAMETER,
          FormBuilderInterface::AJAX_FORM_REQUEST,
          MainContentViewSubscriber::WRAPPER_FORMAT,
        ] as $key) {
        $request->query->remove($key);
        $request->request->remove($key);
      }

Views also uses JavaScript to re-write the form action paths and ajax href paths to send through this controller instead.

After reading this issue, it looks to me like the most promising hint to fix this is @dawehner's #11:

That's the problem that needs solving. We have various code that does stuff like UrlHelper::filterQueryParameters(\Drupal::request()->query->all(), .... I wonder if we want to change filterQueryParameters() to automatically add _wrapper_format and _ajax_form to the exclude list. But should that list then be extensible (i.e., for contrib to use their own custom query params that they don't want bleeding into links)?

Sure, I totally agree with that part.

The UrlHelper would be a bit unfortunate though, because its static and we should avoid adding yet another public global state. What about putting it into RequestContext->getQueryParameter() ?

Unfortunately we only have RequestContext->getQueryString(), not RequestContext->getQueryParameter().

But why and under what conditions (and from which code) do query parameters go into links in the first place? Wouldn't we want a whitelist, not a blacklist?

Answering my own question: E.g. see #3070461: Facet links in modal popup break. Where facet links add all query parameters (to e.g. preserve views exposed filters) but must drop others (e.g. pager, or wrapper_format).

Note that changing the request like (#36) breaks some ajax (e.g. in admin/config/search/search-api/index/article/fields).

So the best i can see is adding some whitelist/blacklist registry to UrlGenerator.

I worked on solution based on 38 and replaced ajax generated href from current url and page number.

/**
       * Accept an url and return the query string being asked.
       *
       * @param name
       *   Name of the query string.
       * @param ur
       *   Url with query strings.
       */
      function getParameterByName(name, url) {
        if (!url) url = window.location.href;
        name = name.replace(/[\[\]]/g, '\\$&');
        var regex = new RegExp('[?&]' + name + '(=([^&#]*)|&|#|$)'),
          results = regex.exec(url);
        if (!results) return null;
        if (!results[2]) return '';
        return decodeURIComponent(results[2].replace(/\+/g, ' '));
      }

      // Change pager links to non ajax links.
      $('.pager ul.pager__items li.pager__item a').click(function (event) {
        // Check if there are any GET parameters to send to views.
        let queryString = getParameterByName('page', this.href);
        const baseUrl = drupalSettings.win_filter.baseUrl;
        this.href = baseUrl + '?page=' + queryString;
      });

Form class

$form['#attached'] = [
      'library' => ['winwin_examples/win_filter'],
      'drupalSettings' => [
        'win_filter' => [
          'baseUrl' => Url::fromRoute('my_examples.filterPager')
            ->setAbsolute()->toString(),
        ]
      ]
    ];

This is not complete solution but give some idea who can't wait to get it solved and patched.

I am facing this as well and I am wondering how bad is the following:
1. In the places where FormBuilderInterface::AJAX_FORM_REQUEST is set, do instead $options['ajax'] = TRUE or something similar
2. Create an outbound path processor that clears out the "_wrapper_format" and "ajax_form", but adds ajax_form if the $options['ajax'] is set.
3. Do a dodgy backward compatibility for modules such as ctools that also does this and issue a warning. It being a check whether the query param is TRUE or 1, and remove it if it's 1, as it's not explicitly set, but rather inherited from the current request query params.

Solution #43 works great as a work around. I tested #2925598-17 Fixed pagination and sorting of tables on pages with ajax on 9.4 and still bleeding thru.

My issue is partially related to the topic, but still, I'm here because of that piece of code

      // Remove all of this stuff from the query of the request so it doesn't
      // end up in pagers and tablesort URLs.
      // @todo Remove this parsing once these are removed from the request in
      //   https://www.drupal.org/node/2504709.
      foreach ([
        'view_name',
        'view_display_id',
        'view_args',
        'view_path',
        'view_dom_id',
        'pager_element',
        'view_base_path',
        AjaxResponseSubscriber::AJAX_REQUEST_PARAMETER,
        FormBuilderInterface::AJAX_FORM_REQUEST,
        MainContentViewSubscriber::WRAPPER_FORMAT,
      ] as $key) {
        $request->query->remove($key);
        $request->request->remove($key);
      }

I really need access to the original data from the request (as for many of you my $form['#action'] is incorrect).

My proposal to sort this out is to clone the request object and push it to the request stack.
Thanks to that we can still get the main/master request and the current one.

In my case, I just simply wrote the Route Subscriber and alter the `views.ajax`
route with a custom controller.
In the controller I simply do the following:

  /**
   * {@inheritdoc}
   */
  public function ajaxView(Request $request) {
    $request = clone($request);
    $this->requestStack->push($request);
    
    return parent::ajaxView($request);
  }

The view is working without any issues (at least seem to be working fine), and I'm able to now do whatever I want with the data from the main request by

 $request = $this->requestStack->getMainRequest();

I hope this workaround will somehow help you with finding the right solution.

#3399951: ajax_page_state leaks through request in Views Ajax may have changed things here a bit.

I created an MR with the changes that were also suggested in #2504709-10: Prevent _wrapper_format and ajax_form parameters from bleeding through to generated URLs. It might not be the best nor a complete solution, but it fixes our issue.

Left a comment but also test is failing.

I think #51 will work. In my case, I’m using a view inside another view, with the sub-view rendered in an iframe; both are AJAX-enabled. The sub-view has an export button (Views Data Export). A normal export without filters works, but when filters are applied it leads to a text area instead.

This will work. Simple:

function hook_views_pre_build(ViewExecutable $view) {
  if ($view->style_plugin->getPluginId() == 'data_export') {
    \Drupal::request()->query->remove(MainContentViewSubscriber::WRAPPER_FORMAT);
  }
}