Side navigation
#1698 closed bug (fixed)
Opened September 20, 2007 03:30PM UTC
Closed October 17, 2007 10:44PM UTC
Last modified March 15, 2012 07:42PM UTC
Scripts should be executed after HTML is inserted to DOM
Reported by: | diz | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | 1.2.2 |
Component: | core | Version: | 1.2.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
Hello!
I previously posted this ticket: http://dev.jquery.com/ticket/1598, but by solving that problem for v1.2.1, another one has been created:
When HTML code is added to the DOM (by domManip function), the <SCRIPT> elements it contains should be automatically executed. The problem is that this execution now takes place before the HTML code is inserted!
I corrected this behavior within jQuery 1.2.1, so that the JavaScript code is executed after the DOM manipulation, here is my code from line 385 of the full version:
domManip: function(args, table, dir, fn) { var clone = this.length > 1, a; return this.each(function(){ if ( !a ) { a = jQuery.clean(args, this.ownerDocument); if ( dir < 0 ) a.reverse(); } var obj = this; if ( table && jQuery.nodeName(this, "table") && jQuery.nodeName(a[0], "tr") ) obj = this.getElementsByTagName("tbody")[0] || this.appendChild(document.createElement("tbody")); jQuery.each( a, function(){ var elem = clone ? this.cloneNode(true) : this; // Changes start from here ======================================== var script = extractScript(-1, elem); fn.call( obj, elem ); if (script !== false) evalScript(script); }); }); } }; function extractScript(i, elem) { if ( jQuery.nodeName(elem, "script") ) { if ( elem.parentNode ) elem.parentNode.removeChild(elem); // Don't need to return a jQuery object if it was a recursive call if (i == -1) return jQuery(elem); else return; } else if ( elem.nodeType == 1 ) return jQuery("script", elem).each(extractScript); return false; } function evalScript(script) { script.each(function() { if ( this.src ) jQuery.ajax({ url: this.src, async: false, dataType: "script" }); else jQuery.globalEval( this.text || this.textContent || this.innerHTML || "" ); }); }
What I did, was to split the current evalScript function into two function: one that extracts (and removes) the scripts and one that evaluates what has been extracted. This way, the DOM manipulation can be done after the scripts have been extracted (else Firefox will execute the script twice), but before the scripts are executed.
I didn't extensively test my correction. It solves the differences with JavaScript execution for the different browsers in my case. But I tried to make it in the jQuery logic, so that it can be added easily to the next build after having been reviewed by you. ;-)
Thanks for your work!
Gabriel
Attachments (2)
Change History (12)
Changed September 21, 2007 01:41PM UTC by comment:1
Changed September 25, 2007 08:33AM UTC by comment:2
A reduced test case has been submitted by Carlos in this ticket:
Changed September 25, 2007 08:40AM UTC by comment:3
This ticket probably also describes the same bug: http://dev.jquery.com/ticket/1654
Changed September 27, 2007 12:51PM UTC by comment:4
Hi, Thanks for your patch witch correct this issue but it create one another bug :
If we load $('#mydiv').load('test.html')
And test.html contains something like this :
<script src='file1.js' type='text/javascript'></script>
<script src='file2.js' type='text/javascript'></script>
It seems like file1 and file2 are loaded twice !
One with a timestamp, one without,
In firebug you will see something like this in "script" part :
file1.js
file1.js?_=1154545454545
file2.js
file2.js?_=1154545454599
If you know how to correct this issue ! :)
Hope this will integrated into a new jquery release soon !
Changed October 01, 2007 07:02PM UTC by comment:5
Are you sure that it will work for external sourced scripts like Google AdSense?
Changed October 12, 2007 03:43PM UTC by comment:6
Ok kennydee, I corrected the script to behave correctly for your test case.
I have also integrated the extractScript and evalScript function to jQuery and made them a bit easier to use.
Here is the modified jQuery code, again from line 385:
domManip: function(args, table, dir, fn) { var clone = this.length > 1, a; return this.each(function(){ if ( !a ) { a = jQuery.clean(args, this.ownerDocument); if ( dir < 0 ) a.reverse(); } var obj = this; if ( table && jQuery.nodeName(this, "table") && jQuery.nodeName(a[0], "tr") ) obj = this.getElementsByTagName("tbody")[0] || this.appendChild(document.createElement("tbody")); jQuery.each(a, function() { var elem = clone ? jQuery.extractScript(this.cloneNode(true)) : jQuery.extractScript(this); if (elem[1]) fn.call(obj, elem[1]); if (elem[0]) jQuery.evalScript(elem[0]); }); }); } }; // This function should be called with a single argument: the DOM elements in jQuery format. // Returns an array containing at key 0 the extracted script elements and at key 1 the remaining DOM elements. jQuery.extractScript = function() { var r = !arguments[1], elem = r ? arguments[0] : arguments[1]; if ( jQuery.nodeName(elem, "script") ) { if ( elem.parentNode ) elem.parentNode.removeChild(elem); if ( r ) return [elem, null]; } else if ( elem.nodeType == 1 ) return [jQuery("script", elem).each(jQuery.extractScript), elem]; else return [null, elem]; }; // This function should be called with a single argument: the script element alone or in a jQuery list. jQuery.evalScript = function() { var scr = arguments[1] ? arguments[1] : arguments[0]; if ( scr.each ) { scr.each(jQuery.evalScript); } else { if ( scr.src ) jQuery.ajax({ url: scr.src, async: false, dataType: "script" }); else jQuery.globalEval( scr.text || scr.textContent || scr.innerHTML || "" ); } };
Thanks for your feedback!
Gabriel
Changed October 15, 2007 02:39PM UTC by comment:7
And this ticket describes the same bug again, with a simple workaround using setTimeout:
Changed October 15, 2007 05:45PM UTC by comment:8
Does anyone know if this solution will be integrated into the next release?
Changed October 16, 2007 08:24AM UTC by comment:9
I've no idea, Jebber007. I'm asking myself the same question!
Changed October 16, 2007 06:08PM UTC by comment:10
Great solution, diz. Works like a champ. It's better than mine, because mine doesn't evaluate inline functions correctly. Hope the developers see this an implement it!
Changed October 17, 2007 10:44PM UTC by comment:11
resolution: | → fixed |
---|---|
status: | new → closed |
Fixed in SVN rev [3667]. I re-worked the evaluation order, pushing inline scripts onto a stack to be executed upon completion (except for scripts that are inline, before DOM elements - those are executed immediately). See the test case for an example.
Changed October 19, 2007 09:01PM UTC by comment:12
Very elegant, how you did that! I tested the nightly build and everything seems to work perfectly here.
Thanks a lot!
Thanks Gabriel for that patch! I was having the same problem you described and your patch fixed it for me! I hope it gets integrated to the jquery-source soon since this issue probably brakes a lot of ajax-applications!
Cheers, Christoph