Hey, it's me, the guy who creates issues and contributes, but no one reviews. Another unheard issue by me:
#2939693: MenuLinkTree::buildItems() ignores cache metadata of disabled links
If you're a nice person, please review my patches and maybe commit them, to have a core with less problems. Thanks!

Problem

AJAX only works for POST method requests, and there might be good reasons for it.
Still people might want to have cacheable AJAX (on their own risk), without the need of yet another framework.

Modules like AJAX assets plus are being created just for one purpose in the end: Having a working, cacheable AJAX system which is capable of GET requests.

Some people who enforce GET on AJAX responses might not be aware of the fact that already loaded assets would then be ignored. As a consequence, assets would be attached multiple times on a page with AJAX GET. That's something you really don't want.

Proposed solution

Just add another check for GET query params inside AjaxResponseAttachmentsProcessor. Attaching patch for this.

CommentFileSizeAuthor
#2 2949148-use-query-params-as-fallback.patch1015 bytesmxh

Comments

mxh created an issue. See original summary.

mxh’s picture

mxh commented
StatusFileSize
new 2949148-use-query-params-as-fallback.patch1015 bytes

Here we go. review plzzzzzzzzz

mxh’s picture

mxh commented
Status: Active » Needs review
mxh’s picture

mxh commented
Issue summary: View changes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Category: Feature request » Task
Status: Needs review » Needs work
Issue tags: +Needs tests

It would be great if this was tested. 

+++ b/core/lib/Drupal/Core/Ajax/AjaxResponseAttachmentsProcessor.php
@@ -129,6 +129,12 @@ public function processAttachments(AttachmentsInterface $response) {
     $ajax_page_state = $request->request->get('ajax_page_state');
+    if (!isset($ajax_page_state)) {
+      // By default, AJAX requests are sent by POST methods.
+      // As others might want to use GET methods instead,
+      // check for corresponding query params.
+      $ajax_page_state = $request->query->get('ajax_page_state');
+    }

What about using \Symfony\Component\HttpFoundation\Request::get() here?

mxh’s picture

mxh commented

It would be great if this was tested.

I'll try :)

What about using \Symfony\Component\HttpFoundation\Request::get() here?

Not sure, this would be more convenient at this place, but would always include a lookup at $request->attributes and $request->query, and lastly $request->request. The last call would be the one which is mostly used for AJAX responses, i.e. using Request::get() here would mean adding mostly two unnecessary calls.

Version: 8.4.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Branches prior to 8.8.x are not supported, and Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

smustgrave’s picture

smustgrave commented
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +stale-issue-cleanup

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

mxh’s picture

mxh commented
Priority: Major » Normal
Status: Postponed (maintainer needs more info) » Closed (outdated)

Thanks for revisiting this. I close it since there is obviously no need and I guess this doesn't make much sense to be continued as we may transition to HTMX in core on the long run.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.