Problem/Motivation

It's difficult to describe, but there are issues with ajax when ajax commands are running more than once on a page. Best description is an example:

1) Add link button to WYSIWYG
2) Edit a node with WYSIWYG field
3) Enter text, press link button, the link dialog pops up
4) Press save - ckeditor is updated
5) Select text, press link button, the link dialog pops up
6) Press save

At this point, instead of ckeditor being updated the browser is redirected to a full page view of the link editor form.

After some investigation it seems this is the same issue as https://www.drupal.org/node/2647916 (Views ajax modals stop working in certain scenarios: due to JS settings caching, AJAX responses may set wrong ajaxPageState.libraries, causing problems for subsequent AJAX requests/responses). There are other examples here.

Proposed resolution

https://www.drupal.org/files/issues/views_dialog_not_opening-2647916-66.... Contains a patch to core/lib/Drupal/Core/Asset/AssetResolver that also needs applying to advagg/src/Asset/AssetResolver.php

Remaining tasks

Create patch for advagg

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#8 interdiff.txt2.17 KBandyrigby
#8 advagg-ajax-issue-2833686-8.patch2.4 KBandyrigby
#7 advagg-ajax-issue-2833686-7.patch2 KBnikolay.sh
#2 advagg-ajax-issue-2833686-2.patch993 bytesandyrigby
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andyrigby created an issue. See original summary.

andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
FileSize
advagg-ajax-issue-2833686-2.patch993 bytes

Attached patch

andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
Issue summary: View changes
andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
Status: Active » Needs review
andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
Related issues: +#2647916: Views ajax modals stop working in certain scenarios: due to JS settings caching, AJAX responses may set wrong ajaxPageState.libraries, causing problems for subsequent AJAX requests/responses
scm6079’s picture

scm6079 CreditAttribution: scm6079 commented

This patch resolves one of the use cases corrected in the core ticket mentioned and should be included. Previously, however, there was an additional ticket in Core which resolved other cache problems.

Advagg is affected by the same bug as was resolved in core 8.2.x - Issue #2603138 (CSS/JS asset caching can easily be trashed), and the subsequently the referenced issue 2647916 (Views ajax modals stop working in certain scenarios: due to JS settings caching, AJAX responses may set wrong ajaxPageState.libraries, causing problems for subsequent AJAX requests/responses).

It would be prudent to review Issue #2603138, which was after advagg's AssetResolver was forked from core.

nikolay.sh’s picture

nikolay.sh CreditAttribution: nikolay.sh commented
FileSize
advagg-ajax-issue-2833686-7.patch2 KB

I've faced with similar ajax/views issue.

Ajax views works only once after flush cache (broken after reload page). It happens because view_dom_id stored in drupalSettings which cached. Views module generates new view_dom_id on each reload page, but ajax command takes old view_dom_id from cached drupalSettings. So views content cannot be updated because selector has not found.

Provided patch#2 not fixed my issue.

I've ported other fixes from issue #2603138 to AdvAgg. It solved my problem and maybe will be helpful for others.

andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
FileSize
advagg-ajax-issue-2833686-8.patch2.4 KB
interdiff.txt2.17 KB

Thanks @nikolay.sh and @scm6079. I am also seeing an additional ajax issue with views ajax filtering and this does seem to be caused by https://www.drupal.org/node/2603138. The patch in #7 resolves that for me.

I've tweaked the patch in #7 in two ways:

1) The cache id generation line I have reverted to the same as in patch #2. This version is the most recent in AssetResolver in core (see http://cgit.drupalcode.org/drupal/tree/core/lib/Drupal/Core/Asset/AssetR...) committed as a result of https://www.drupal.org/node/2647916

2) The removal of the "$settings = NestedArray::mergeDeepArray" line makes the getJsSettingsAssets() method identical to the parent class so this overridden method does not need to exist.

This resolves both the issue with ckeditor link plugin and the issue with views ajax filtering for me.

andyrigby’s picture

andyrigby CreditAttribution: andyrigby at Ixis - UK Drupal Support, Maintenance, Hosting and Development commented
Related issues: +#2603138: CSS/JS asset caching can easily be trashed
das-peter’s picture

das-peter CreditAttribution: das-peter at Cando commented
Related issues: +#2856190: Cache poisoning of DrupalSettings with selected entities

I think we just came across another nasty side-effect of this here #2856190: Cache poisoning of DrupalSettings with selected entities
Will give the patch a test ride and report back.

das-peter’s picture

das-peter CreditAttribution: das-peter at Cando commented
Status: Needs review » Reviewed & tested by the community

My tests were successful, I'd claim this is ready to go in as it definitely solves existing issues, issues that are hard to debug and even occur with most of Advaggs features disabled.

  • mikeytown2 committed 178df1c on 8.x-2.x authored by andyrigby 
    Issue #2833686 by andyrigby, nikolay.sh: Issues with ajax commands /...
mikeytown2’s picture

mikeytown2 CreditAttribution: mikeytown2 commented
Status: Reviewed & tested by the community » Fixed

Thanks for getting this patch to RTBC! #8 has been committed.

Status: Fixed » Closed (fixed)

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