Bug Tracker

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#12047 closed bug (wontfix)

body.removeChild( container ); can cause an error

Reported by: Ian Yang Owned by: Ian Yang
Priority: low Milestone: None
Component: support Version: 1.7.2
Keywords: Cc:
Blocked by: Blocking:

Description

An answer of a question on stackoverflow shows that jQuery adds some elements (the outmost one is container) to do some tests (line 1537 in the un-minified version):

// 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 that test element with:

body.removeChild( container );

However, as addressed in the question, by using the DOMNodeInserted event, all contents of body can be wrapped inside another element before jQuery can remove that test element, causing body.removeChild( container ); to fail because the container isn't a child element of body anymore.

So the suggestion is to change line 1654 into:

jQuery( container ).remove();

Change History (8)

comment:1 Changed 7 years ago by timmywil

Component: unfiledsupport
Priority: undecidedlow
Resolution: wontfix
Status: newclosed

DOMNodeInserted is deprecated and should no longer be used.

http://www.w3.org/TR/DOM-Level-3-Events/#event-type-DOMNodeInserted

comment:2 Changed 7 years ago by dmethvin

#12066 is a duplicate of this ticket.

comment:3 Changed 7 years ago by dmethvin

Resolution: wontfix
Status: closedreopened

comment:4 Changed 7 years ago by dmethvin

Owner: set to Ian Yang
Status: reopenedpending

Can you provide a test case on jsFiddle that reproduces the issue, not using the deprecated DOMNodeInserted?

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

Replying to dmethvin:

Can you provide a test case on jsFiddle that reproduces the issue, not using the deprecated DOMNodeInserted?


Here is the test case on jsFiddle. Please use the Firebug on Firefox 13 to see the error. And please click on the "Run" button to run it again if you don't see the error. On jsFiddle, the chance of this happening is 50%.

Note: the technique used in the test case is inspired by this article.

comment:6 Changed 7 years ago by gibson042

Resolution: wontfix
Status: pendingclosed

Support loads before most other modules, in particular manipulation, so that line can execute before .remove is defined. And more importantly, we're just not prepared to tolerate external manipulation of our internal-use elements.

comment:7 Changed 7 years ago by dmethvin

We could get around the error sans jQuery with container.parentNode.removeChild(container) but I don't think that resolves the issue -- it only hides it. If you are wrapping the HTML we use to do feature detects it is going to affect the jQuery.support outcomes, and cause jQuery to misbehave in strange ways down the road. The workaround would be do not attach your DOMNodeInserted handler until document ready, since we attach our ready handler for support checks first.

comment:8 Changed 7 years ago by Ian Yang <ian.html@…>

Thanks for the advise anyway.

Btw, I have no intention of manipulating anyone's internal-use elements. I won't even know they exist if they didn't cause my browser to throw the error.

Note: See TracTickets for help on using tickets.