Allow AJAX to use GET requests

📌 Compress aggregate URL query strings Fixed landed, which means we can look at compressing ajax_page_state here now. Leaving needs review for #166 since it still needs reviews.

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

#166 still applied, and I noticed that #168 was missing several hunks from the patch.

Re-uploading #166, re-rolled just in case there's any patch fuzz but not sure there was even that.

I bet some fixes will have collision with related 📌 Add the session to the request in KernelTestBase, BrowserTestBase, and drush Fixed

the patch no longer applys on latest of 10.0.x, rerolling.

The last submitted patch, 174: 956186-174.patch, failed testing. View results →

Here's a back-port patch that applies to Drupal 9.5.2, if it's useful to anyone: 956186-175-for-D9.patch →

Patch no longer applies

Per @catch in #167 should 📌 Convert Views to use GET for AJAX Closed: duplicate be included here?

@smustgrave - that's actually done in the patch. I think it makes sense to do them together, since Views is more or less our only non-form use of AJAX in core, so going to go ahead and mark the other issue duplicate.

Looking through the related issues I rediscovered 📌 Add views render caching on views ajax requests Closed: outdated which adds a configuration option to views pagers whether they use GET or POST or not.

If we wanted to do that, we might want to remove the views changes from this patch after all, and then focus on that one (which requires this patch anyway). Probably need @lendude here.

As @catch pointed out in #179:

since Views is more or less our only non-form use of AJAX in core

much of the confidence we have that this change works is based on existing Views test coverage not breaking. So if we remove the Views changes, we also lose quite a bit of coverage for this change, right? I think that doing the changes to Views here is probably better.

Only without the option to switch back to POST this might break some custom code that depends on this being POST? Probably not likely to happen, but might we need another CR that outlines the Views changes? Or expand the existing one?

Discussed this with @lendude a bit in slack. We both think the configuration option added in 📌 Add views render caching on views ajax requests Closed: outdated could end up being overkill. So, we could continue with just switching Views AJAX to GET in this issue.

That's most likely to be successful if we compress ajax_page_state, so I've opened an issue for that: 📌 Add views render caching on views ajax requests Closed: outdated . Can think about precedence and whether to merge issues once there's a working patch there.

Going to call it, I've clicked around after making core views use ajax and nothing appears broken: pagers in different themes, media library, bigpipe.

removing the needs test tag since views tests are implicitly testing this. added a line about views in the CR

Hello, we needed this patch on 9.5 so I took the patch in #177 📌 Allow AJAX to use GET requests Fixed and re-rolled it to apply to 9.5.x.

Attaching it here in case is helpful for someone else.

Thanks!

m

I've made a start on 📌 Compress ajax_page_state Fixed .

I'm correcting the patch provided in #184 📌 Allow AJAX to use GET requests Fixed since when I did the re-roll I forgot to:

• Add the javascript changes to the *es6.js files
• Build js with yarn build:js
• Commit the changes after the build

We need to do those steps since Drupal 9.5.x still uses the build commands :)

Checked the new patch changes running bash core/scripts/dev/commit-code-check.sh in local and all checks passed OK, so hopefully they also pass in drupalci.

Another reroll based on #177.

I was reviewing the code and I was wondering if we should be using the method property from jQuery.ajax() instead because type is an alias for it. It seems like it would also be clearer because it would separate this from #type, and would also align closer to the terminology that is usually used in the context of HTTP requests. However, it looks like we are already using the method key in the ajax settings for configuring the method used for inserting new content in the targeted element. 😞

I went through contributed modules to see if they would be impacted by this to address #181. I found that there are at least two modules that would be impacted by this; Smart Content module and a submodule of the Content-Security-Policy module. Maybe we could just file issues in their issue queue for this? I also updated the CR to explicitly mention this aspect of the change.

Committed 29763ea and pushed to 10.1.x. Thank you for everyone who worked on this over the past 13 years!

Filed an issue in the Content-Security-Policy module queue: 🐛 Add support for GET Ajax requests Fixed .

Discussed #189 briefly with @lauriii and we wondered about 'httpMethod' as a key name, so I opened 📌 Use httpMethod instead of type for AJAX get/post request property Fixed .

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

When opening a dialog from a link (the route/controller method) with GET request, the data-dialog-options are not respected. I've opened 🐛 Dialog options are not honoured when open a dialog using GET Needs work