Problem/Motivation

When AdvAgg combines the ajaxPageState for all scopes it creates $setting['ajaxPageState']['js'] and fills it with the keys of all current JS/CSS items in the scope.
Then it checks if the scope being processed has an item 'settings'. If not, $setting get pushed back in using drupal_add_js($setting, 'settings'). If the scope did have a 'settings' item (expected in the last scope to be processed, header) it does a "lookup" by reversing the entire list of scripts added using drupal_add_js() and tries to find whatever it stuffed back in earlier by looking at the first few items. If one of them has 'ajaxPageState' the lookup stops and puts that item in with the rest of the output.

This sort of worked when there was only the footer and header scopes to consider. But, now that there's an 'above css' scope, it may silently ignore parts of the page state. The result is that some script/CSS gets re-sent back with the AJAX response even if they already exist on the page, which is disastrous to scripts like wysiwyg.js.

This process must not stop at the first item it finds as there may have been more items containing page state, added during processing of the other scopes.

Proposed resolution

Keeping this "add it back in and do a reverse lookup later" functionality becomes much less efficient if we simply fix this issue by removing the break; statement in advagg_get_js().
Why not simply keep track of the page state in a static variable and add them in when the last scope is encountered ($items['settings'] is set)?

We've made a first attempt at this which has been running fine in production for a long time now. The only possible issue we foresee is if the AdvAgg processing happens to run twice in the same page, but switching to drupal_static() and resetting it before starting scope processing would fix that.

Comments

kevineinarsson created an issue. See original summary.

kevineinarsson’s picture

Status: Active » Needs review
StatusFileSize
new2.78 KB
kevineinarsson’s picture

Issue summary: View changes
mikeytown2’s picture

StatusFileSize
new3.52 KB

This should fix the php 5.3 issue.
Well if this breaks then it looks like more tests should be added in the future. I'd say this might cause issues with the aggressive render cache as I remember having to do some odd things to get this working correctly in there.

  • mikeytown2 committed 8c40242 on 7.x-2.x
    Issue #2956697 by kevineinarsson, mikeytown2: ajaxPageState settings are...
mikeytown2’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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