body.removeChild( container ); is unreliable and can cause an error

Reported by:	Ian Yang <ian.html@gmail.com>	Owned by:
Priority:	undecided	Milestone:	None
Component:	unfiled	Version:	1.7.2
Keywords:		Cc:
Blocked by:		Blocking:

Description

An answer of a question on stackoverflow shows that when jQuery has been loaded, it performs some background tests by adding an elements (the outmost one is container) into the body.

The following code on line 1537 of jQuery un-minified version shows the addition of the container:

// Run tests that need a body at doc ready
jQuery(function() {
var container, outer, inner, table, td, offsetSupport,
...

And at line 1654 jQuery wants to remove the container with:

body.removeChild( container );

However, by using the DOMNodeInserted event, all contents of body (including the container added by jQuery) are always able to be wrapped inside another element before jQuery can remove the container, causing body.removeChild( container ); to fail because the container isn't a child element of body anymore.

Not only the execution of the DOMNodeInserted event is faster than the removal of the container, but the following approach which utilizes both the CSS3 animation and the Javascript animationstart event to achieve DOM manipulation can sometimes make the same jQuery error occurs.

<!DOCTYPE html>
<html lang="en">
<head>
	<style>
		@-webkit-keyframes nodeInserted {
			50% { opacity: 0.99 }
		}
		@-moz-keyframes    nodeInserted {
			50% { opacity: 0.99 }
		}
		@-ms-keyframes     nodeInserted {
			50% { opacity: 0.99 }
		}
		@-o-keyframes      nodeInserted {
			50% { opacity: 0.99 }
		}
		@keyframes         nodeInserted {
			50% { opacity: 0.99 }
		}
		body {
			-webkit-animation-duration: 1ms;
			   -moz-animation-duration: 1ms;
			    -ms-animation-duration: 1ms;
			     -o-animation-duration: 1ms;
			        animation-duration: 1ms;
			-webkit-animation-name: nodeInserted;
			   -moz-animation-name: nodeInserted;
			    -ms-animation-name: nodeInserted;        
			     -o-animation-name: nodeInserted;
			        animation-name: nodeInserted;
			}
	</style>
	<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js"></script>
	<script>
		function insertListener(event) {
			if (event.animationName == 'nodeInserted') {
				// DOM manipulation start
				document.body.innerHTML = '<div class="wrapper">' + document.body.innerHTML + '</div>';
				// DOM manipulation end
			}
		}
		document.addEventListener('webkitAnimationStart', insertListener);
		document.addEventListener(    'MSAnimationStart', insertListener);
		document.addEventListener(     'OAnimationStart', insertListener);
		document.addEventListener(      'animationstart', insertListener);
	</script>
</head>
<body>
	<p>Lorem ipsum dolor sit amet.</p>
</body>
</html>

As of this time (2012-07-12), Firefox 13 has 33% chance to be faster than the removal of the container to have this error occurs. And other browsers' performance might be improved in the future and then they might have the same error occurs on them.

The above mentioned two experiments indicate that the use of body.removeChild( container ); is unreliable and can cause an error. It will be safer to change the code into the following one or something better.

jQuery( container ).remove();

Attachments (0)

Change History (4)

Changed July 12, 2012 12:50PM UTC by dmethvin comment:1

resolution:	→ duplicate
status:	new → closed

Changed July 12, 2012 12:50PM UTC by dmethvin comment:2

Duplicate of #12047.

Changed July 12, 2012 01:17PM UTC by Ian Yang <ian.html@gmail.com> comment:3

Replying to [comment:2 dmethvin]:

Duplicate of #12047.

Is it because I mentioned the DOMNodeInserted event which was mentioned in another ticket that you thought this ticket is duplicate and should be closed?

In this ticket, I also mentioned a new thing: the use of CSS3 animation and the Javascript animationstart event which can result in the same error. And you didn't advise on it. Could you please advise?

Changed July 12, 2012 01:18PM UTC by dmethvin comment:4

Can you provide a test case in the duplicate #12047? Thanks.

Side navigation