Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Drupal.theme functions implemented by other modules are broken when javascript compression is enabled.
The problem is as follows:
- When javascript compression is enabled, almost all javascript files are packed into a single file.
- When Ajax Load processed the response of an AJAX request, this request comes with the list of .js and .css files that were generated in that request. Ajax Load compares this list of files with those that are already on the page, and dynamically loads those that are not present.
- However, when javascript compression is enabled, Ajax Load does not find matches because the file names do not match. The javascript files on the pages are all included into the one big file, so Ajax Load compares apples to oranges, and loads again javascript files that were already included in the compressed file.
- When that happens, functions dynamically added to extend Drupal objects such as Drupal.theme are lost when drupal.js is loaded again. And this is what breaks the page after an AJAX request, and then the code tries to use Drupal.theme('hook').
So... we need to find a way to compare apples to apples. We need to change Ajax Load so that it knows client-side which files are present on the page, even when these are compressed.
I think we can use Drupal.settings on the main page to let Ajax Load know which .js and .css files are loaded, and then use that list to compare with those that come from AJAX requests.
Working on it...
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | ajax_load-685918-1.patch | 5.13 KB | markus_petrux |
Comments
Comment #1
markus_petrux CreditAttribution: markus_petrux commentedHere's a patch for review that seems to solve the issue here.
How this patch tries to solve the issue:
1) When css/js aggregation is enabled, ajax_load builds a list of files of each type and sends it to the main page using Drupal.settings.
2) When an AJAX request is made, it compares the incoming css/js files with that list, not with the files in the DOM because the files in the DOM are aggregated, so the file names will never match.
When css/js is NOT enabled, it works as before. That is, it compares the incoming files with those that are present in the page, using the DOM to find out existing files.
So no changes when css/js aggregation is not enabled.
Comment #2
nedjoClever solution, nice work!
I see where new CSS files are loaded, but it looks like the JS version is missing?
Comment #3
markus_petrux CreditAttribution: markus_petrux commentedWe were already using the externalScript array client-side, so part of magic to deal with the list of .js files was already in place. Other than that, I'm not sure what you mean. :( ...the server-side code, from the new page preprocess function builds the list of css as well as js files, when aggregation is enabled, for each type. So the list of files in Drupal.settings is only there when aggregation is enabled, otherwise, the client-side code builds the externalStyles and externalScripts array parsing the DOM using the proper jQuery selectors for each kind of file (css and js). I might be missing something I fail to see? It could happen because I'm dealing with lots of stuff at a time. :-/
PS: Nice to see you here! :D ...and thanks again for letting me deal with this module, because it's a basic little piece in our project.
Comment #4
markus_petrux CreditAttribution: markus_petrux commentedI created the 6.x-1.3 release prior to opening this issue and posting the patch, just because it's not a trivial issue, but since we do not have any other outstanding issues, I'm tempted to commit here... for the moment, it is working like a charm in all cases where we've been testing it.
If no more feedback is provided, I may proceed tomorrow, so that we can turn this page for a while to see if being in the -dev snapshot can be more easily tested.
Comment #5
markus_petrux CreditAttribution: markus_petrux commentedOk, I have committed the patch to CVS. And hopefully now can be easily tested by using the -dev snapshot, that it could leave as-is for a while. Next, if no other issues arise, we can create a new stable release so that Ajax Load can be used too on sites where js/css aggregation is enabled.
http://drupal.org/cvs?commit=317480
I have also fixed minor doxygen issues in ajax_load.js:
http://drupal.org/cvs?commit=317482
Comment #7
stevehuk CreditAttribution: stevehuk commentedWould it be possible to have a new 6.x-1.n release to include this? I was just tripped up by a side effect of the aggregation issue causing loading of duplicates of javascript files causing issues with Views and ajax. The dev version worked for me. Thanks for the fix.
Comment #8
markus_petrux CreditAttribution: markus_petrux commentedDone: http://drupal.org/node/1249824