#2973 closed bug (fixed)

Opened June 03, 2008 06:02PM UTC

Closed February 14, 2009 05:21PM UTC

Last modified March 14, 2012 05:06PM UTC

IE6 clone(true) bug when cloning a cloned element with events

Reported by:	futileohm	Owned by:
Priority:	major	Milestone:	1.3.2
Component:	core	Version:	1.3.1
Keywords:	clone ie6	Cc:
Blocked by:		Blocking:

Description

I seem to have discovered an obscure bug in IE (6, I don't have a copy of 7 to test with) when using

clone(true)

that occurs when you clone an element with certain nested elements and events, and then clone it again. This is a regression from 1.2.3, which seems to have worked fine.

I had a hard time generalizing this bug as it seems to only happen in very specific circumstances, but I have attached a test case to demonstrate the behavior.

As far as I can tell, the most generic way to make the bug occur is to take a DOM node structured like so:

<div class='a'>
	<span>
		<span class='b'>abc</span>
	</span>
	<span>
		<span class='c'>abc</span>
	</span>
</div>

and bind an event to

span.b

and another event to

span.c

. Then, clone

div.a

using

clone(true)

, and then clone the clone using

clone(true)

again. Removing any of the surrounding

span

elements causes the bug to disappear, although swapping the class designation on the first set of nested

span

elements demonstrates the bug as well.

The specific error reported by IE is "'nodeType' is null or not an object" and occurs on the first line of the

event.add

function:

if ( elem.nodeType == 3 || elem.nodeType == 8 )

I tried replacing the

event.add

function in 1.2.3 with the

event.add

function from the trunk, and 1.2.3 still doesn't demonstrate the bug, leading me to believe it's elsewhere that the issue's occurring. I compared the

clone

functions from 1.2.3 and the trunk and didn't see any major changes that may have caused this sort of an issue, but at that point I was out of my league.

I'm not sure that this is a major regression, but it's a regression nonetheless, so I would imagine it should probably be fixed to maintain backwards compatibility.

Attachments (3)

clone-testcase.html (1.1 KB) - added by futileohm June 03, 2008 06:03PM UTC.

Test case demonstrating the bug.
ie6-clone-events-patch.diff (1.2 KB) - added by futileohm June 05, 2008 06:29PM UTC.

Actual patch with potential fix.
prism.exe (104.0 KB) - added by futileohm June 05, 2008 06:27PM UTC.

Patch with potential fix.

Change History (6)

Changed June 04, 2008 03:46PM UTC by flesler comment:1

You're trying this with 1.2.5 or 1.2.6 ?

Changed June 04, 2008 04:45PM UTC by futileohm comment:2

It doesn't work in 1.2.5, 1.2.6, or the latest trunk (r5712). (I think I chose version 1.2.5 because it was the latest listed at the time).

I can also confirm that the same bug occurs in IE7.

Changed June 05, 2008 06:37PM UTC by futileohm comment:3

(Apologies for the prism.exe attachment, I have no clue how that got selected instead of the patch itself, please delete it.)

I tracked down the source of the bug, and the actual cause is due to the way expandos are handled and stripped within the clone function. The actual change in behavior between 1.2.3 and the current trunk is due to a

unique()

call within the

add()

function. Within the clone function, the following is used to strip expandos from the newly cloned object:

var clone = ret.find("*").andSelf().each(function(){
	if ( this[ expando ] != undefined )
		this[ expando ] = null;
});

However, the

andSelf()

call, which then invokes

add()

, barfs because of the yet-to-be-cleaned expandos on the newly cloned elements. Thus, until the expandos are cleaned,

unique()

will return an incorrect set of elements because the copied expandos are inconsistent.

I attached a patch (against r5713) with a fix, which breaks the expando cleaning out into two separate calls to avoid using

andSelf()

. There's probably a more efficient way of doing it, but I couldn't find one that wasn't incredibly tedious in my few minutes of tinkering. The new code looks like this:

ret.find('*').each(function(){
	if ( this[ expando ] != undefined )
		this[ expando ] = null;
});

ret.each(function(){
	if ( this[ expando ] != undefined )
		this[ expando ] = null;
});

var clone = ret.find('*').andSelf();

As such, it seems this may not apply only to elements with events, but any data carrying nodes that are cloned within IE could be susceptible to this bug, which could in theory remove elements from the cloned set.

Changed July 25, 2008 08:43PM UTC by num comment:4

Replying to [comment:3 futileohm]:

Applied your patch to 1.2.6 and works well in prelim testing with http://plugins.jquery.com/project/jCal plugin which suffered from this issue.

Changed January 20, 2009 03:30PM UTC by dmethvin comment:5

keywords:	→ clone ie6

Changed February 14, 2009 05:21PM UTC by john comment:6

milestone:	→ 1.3.2
resolution:	→ fixed
status:	new → closed
version:	1.2.5 → 1.3.1

This should be fixed now in SVN trunk.

Side navigation