Skip to main content

Bug Tracker

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)
  • jquery-20071012.patch (2.1 KB) - added by diz October 12, 2007 03:59PM UTC.

    New patch that should correct kennydee's test case.

  • jquery.patch (1.4 KB) - added by diz September 20, 2007 03:51PM UTC.

    Patch

Change History (12)

Changed September 21, 2007 01:41PM UTC by c_t comment:1

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

Changed September 25, 2007 08:33AM UTC by diz comment:2

A reduced test case has been submitted by Carlos in this ticket:

http://dev.jquery.com/ticket/1725

Changed September 25, 2007 08:40AM UTC by diz comment:3

This ticket probably also describes the same bug: http://dev.jquery.com/ticket/1654

Changed September 27, 2007 12:51PM UTC by kennydee 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 rrjanbiah comment:5

Are you sure that it will work for external sourced scripts like Google AdSense?

Changed October 12, 2007 03:43PM UTC by diz 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 diz comment:7

And this ticket describes the same bug again, with a simple workaround using setTimeout:

http://dev.jquery.com/ticket/1793

Changed October 15, 2007 05:45PM UTC by Jebber007 comment:8

Does anyone know if this solution will be integrated into the next release?

Changed October 16, 2007 08:24AM UTC by diz comment:9

I've no idea, Jebber007. I'm asking myself the same question!

Changed October 16, 2007 06:08PM UTC by Jebber007 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 john comment:11

resolution: → fixed
status: newclosed

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 diz comment:12

Very elegant, how you did that! I tested the nightly build and everything seems to work perfectly here.

Thanks a lot!