Problem/Motivation

#956186: Allow AJAX to use GET requests caused a(nother) regression with bulk actions on ajax-enabled views.

Steps to reproduce

  1. Install Drupal 11.x with standard profile
  2. Enable ajax on the /admin/content view
  3. Create one article node
  4. Visit /admin/content and apply a filter (e.g. Type = Article)
  5. Delete the article through bulk operations

This causes POST request with extra state, which somehow just reloads the page without any CSS, no message and nothing happens:

Proposed resolution

CommentFileSizeAuthor
#6 3372678-6.patch5.67 KBlauriii
#8 interdiff.txt2.05 KBlauriii
#8 3372678-8.patch4.66 KBlauriii
#6 3372678-6-test-only.patch3.94 KBlauriii
#5 3372678-5.patch1.05 KBlauriii
#7 3372678-7.patch5.61 KBlauriii
Screenshot 2023-07-05 225450.png70.26 KBberdir
#7 3372678-7-test-only.patch3.88 KBlauriii

Comments

Berdir created an issue. See original summary.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Parent issue: #3364088: Ajax state leaking to Views destination paths »
Related issues: +#956186: Allow AJAX to use GET requests
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

It looks like this is caused by \Drupal\Core\Form\FormSubmitter::redirectForm which is using current request and $request->query->all() to generate URL to redirect to. I'm not sure what would be the best way to address the bug here. We could define a redirect URL for this form, but I think we should try to find a more generic solution 🤔

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Active » Needs review
StatusFileSize
new 3372678-5.patch1.05 KB

Maybe something like this?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3372678-6-test-only.patch3.94 KB
new 3372678-6.patch5.67 KB

Worked on some tests for this and realized that we'll need to do something about \Drupal\Core\Routing\RedirectDestination too. This is starting to be in quite a few places so I'm wondering if we should try to get rid of the query params from the request in the request stack.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3372678-7-test-only.patch3.88 KB
new 3372678-7.patch5.61 KB
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 3372678-8.patch4.66 KB
new interdiff.txt2.05 KB

Another potential solution 🤔

The last submitted patch, 7: 3372678-7-test-only.patch, failed testing. View results

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Works fine for me. Having to handle this all over the place isn't nice, but we have better and better test coverage of this stuff, so hopefully we can refactor it into something better eventually.

  • catch committed 097be7ab on 10.1.x 
    Issue #3372678 by lauriii, Berdir: Ajax state leaking to Views bulk...

  • catch committed 2ea053b3 on 11.x 
    Issue #3372678 by lauriii, Berdir: Ajax state leaking to Views bulk...
catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.1.x-dev
Status: Reviewed & tested by the community » Fixed

#3348789: Compress ajax_page_state doesn't remove this from the query params yet but it could, and the extra test coverage will definitely help that issue to not break even more things.

Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.