Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12066 closed bug (duplicate)

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

Reported by: Ian Yang <ian.html@…> 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();

Change History (4)

comment:1 Changed 7 years ago by dmethvin

Resolution: duplicate
Status: newclosed

comment:2 Changed 7 years ago by dmethvin

Duplicate of #12047.

comment:3 in reply to:  2 Changed 7 years ago by Ian Yang <ian.html@…>

Replying to 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?

comment:4 Changed 7 years ago by dmethvin

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

Note: See TracTickets for help on using tickets.