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.First of all thanks for nice module, its very helpful.
I have 2 questions according module behaviour:
1) Do we need to load core scripts during ajax_loading?
ajax_load.js line 23:
var types = ['core', 'module', 'theme'];
We got issue that happens when jquery_update module is installed and we are trying to load Views display - misc/jquery.js is loaded instead of jquery.js from jquery_update package. But if line 23 will contain only 'module' & 'theme' - script will use core javascripts from the page and no jquery conflicts will appear. Is there any strong points to load core javascripts with ajax_load?
2) We have discovered that ajax_load doesn't keep track of scripts and styles loaded with its help. For example Views display (which was loaded with the help of the ajax_load) will reload whole list of required js on each self-reload. That is not cool, that's why I propose to provide caching for already loaded scripts and styles. Here is the patch which gives us this option.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | ajax_load-389148-8.patch | 4.47 KB | markus_petrux |
| #4 | ajax_load-389148-4.patch | 4.48 KB | markus_petrux |
| ajax_load.js_.patch | 756 bytes | akhodakovskiy |
Comments
Comment #1
nedjoThanks for the proposed patch.
This part of the existing code was supposed to ensure that already-added scripts were not attached again:
(!$('script[src*=' + src + ']')It says "if we already have a script beginning with this path loaded, don't reload it."
Is this not working? If it is broken, do you have ideas about why?
Re core scripts, hmm, I think we do, in case they aren't already present on the page.
Comment #2
markus_petrux CreditAttribution: markus_petrux commentedI believe 2) could only be necessary if the URLs compared by the existing check are not written the same exact way. For example, comparing absolute URLs against relative URLs. So it might make sense to try to check this in some other way.
Making this as CNW as 1) is not really addressed. The problem here seems to be related to the way jquery_update replaces misc/jquery.js with the new version. It hook into registry alter method of theme('page'). If 'page' is not involved in an AJAX request, then jquery_update is not able to replace the jquery library. Maybe this would have to be solved by the code that generates the AJAX response?
Comment #3
markus_petrux CreditAttribution: markus_petrux commentedhmm... I believe I know the problem that causes reloading of css/js files. It's that letter that gets added as an argument.
To address this, I believe we would have to change
ajax_data_get_data()in a way similar todrupal_get_js()anddrupal_get_css().That is we need to append the following to certain js/css URLs:
But maybe we also need to react on the preprocess options. But maybe this could simply be avoided here. Only the argument would really be needed.
Comment #4
markus_petrux CreditAttribution: markus_petrux commentedWell, here's a patch that tries to solve 3 different things:
1) Deals with jQuery Update replacements.
2) Ensures Drupal behaviors are executed, even when no external javascript file has been loaded (*).
3) Caches the list of external javascript files on the page to ensure the same script is not loaded more than once.
(*) When new content does not come with external javascript files, no inline javascript file is executed, and Drupal.attachBehaviors() is not invoked either, but it might come for example with a collapsible fieldset that will not get the behavior attached.
Comment #5
markus_petrux CreditAttribution: markus_petrux commented@deadmonk: could you please try the patch in #4?
Comment #6
akhodakovskiy CreditAttribution: akhodakovskiy commentedSorry, i was out for two weeks, so didn't follow status of my issue.
@nedjo
it means that if script was loaded with ajax_load this check won't verify it as already loaded. For example, for css files module appends some html in the head:
$('<link type="text/css" rel="stylesheet" media="' + media + '" href="' + src + '" />').appendTo('head');that makes module be aware of already loaded css files.
That's why I proposed to store script paths (which were loaded on page with ajax_load) and add an extra check.
Comment #7
akhodakovskiy CreditAttribution: akhodakovskiy commented@markus_petrux
That's great. It works exactly like I have requested in my issue. Thanks.
I hope that this patch would be promoted to module code base.
Comment #8
markus_petrux CreditAttribution: markus_petrux commentedSweet. Thanks for the feedback. :)
I have committed a slight variation of the patch, updated in this post. Also, changing the issue title to reflect what we ended up doing. :)
Comment #9
markus_petrux CreditAttribution: markus_petrux commented@nedjo: If you read this... the queue is now empty :) ...so... may we create a new release or wait a few days just in case?
Comment #10
nedjo@markus_petrux: thanks for applying the fix. I'm travelling in Central America these days and not finding a ton of time for project maintenance. Yes, a new release would be great, please go ahead and create if you have time. I've noted you as co-maintainer on the project page. Please go ahead and apply patches and make releases as you see fit. Thanks!