Bug Tracker

Opened 12 years ago

Closed 11 years ago

Last modified 8 years ago

#2997 closed bug (fixed)

$.clone() bugs in IE when $.length>1 and elements don't belong to document

Reported by: shazam Owned by: flesler
Priority: minor Milestone: 1.3
Component: core Version: 1.2.6
Keywords: clone Cc:
Blocked by: Blocking:

Description

If you are manipulating elements in a child iframe, the clone method tries to do some manips inserting nodes belonging to child iframe document into main frame document.

The clone method should be replace by this one which uses this.ownerDocument when possible

	clone: function( events ) {
		// Do the clone
		var ret = this.map(function(){
			if ( jQuery.browser.msie && !jQuery.isXMLDoc(this) ) {
				// IE copies events bound via attachEvent when
				// using cloneNode. Calling detachEvent on the
				// clone will also remove the events from the orignal
				// In order to get around this, we use innerHTML.
				// Unfortunately, this means some modifications to
				// attributes in IE that are actually only stored
				// as properties will not be copied (such as the
				// the name attribute on an input).
				var clone = this.cloneNode(true),
					container = (this.ownerDocument||document).createElement("div");
				container.appendChild(clone);
				return jQuery.clean([container.innerHTML], this.ownerDocument||document)[0];
			} else
				return this.cloneNode(true);
		});

Attachments (2)

cloneBug.rar (1.2 KB) - added by shazam 12 years ago.
clone[5790].diff (1.1 KB) - added by flesler 12 years ago.
This is the patch, but it breaks many many tests :(

Download all attachments as: .zip

Change History (11)

comment:1 Changed 12 years ago by shazam

Title is wrong and should be $.clone() bugs in IE The clone function is not completely false and only the begining has been written in the post

comment:2 Changed 12 years ago by shazam

Title is wrong and should be '$.clone() bugs in IE'
The clone function is not completely false and only the begining has been written in the post

comment:3 Changed 12 years ago by flesler

need: PatchTest Case
Priority: majorminor

So, does that cause an error or what ? Could you provide a test case ? a minimalistic html file with the requires html and js to reproduce the problem ? And please detail the error or odd behavior. Thanks.

comment:4 Changed 12 years ago by shazam

Yes this causes an 'invalid argument' error when you manipulate elements in a child iframe.

I took the time to make a complete sample.

I also made a better fix to the problem which is well explained in the sample.

So, this part of the $.clone() function :

var clone = this.cloneNode(true),
	container = (this.ownerDocument||document).createElement("div");
container.appendChild(clone);
return jQuery.clean([container.innerHTML], this.ownerDocument||document)[0];

Should be replaced by :

// As this line is executed only in IE we can use the outerHTML property 
// and we use this.ownerDocument in the $.clean() function to avoid cross document bug
return jQuery.clean([this.outerHTML], this.ownerDocument||document)[0];

Changed 12 years ago by shazam

Attachment: cloneBug.rar added

comment:5 Changed 12 years ago by flesler

Owner: set to flesler
Status: newassigned

Right, clever proposition, could be reduced to:

return jQuery.clean([this.outerHTML], this)[0];

jQuery.clean will handle the rest. I'll assign this to myself and will check this asap. This obviously needs a lot of testing. Will surely be in for the next release if all goes well.

Thanks

comment:6 Changed 12 years ago by flesler

need: Test CaseCommit

comment:7 in reply to:  2 Changed 12 years ago by Xiao Xu

Replying to shazam:

Title is wrong and should be '$.clone() bugs in IE'
The clone function is not completely false and only the begining has been written in the post

So do you know the situation about clone a node from child frame to parent window? Regards

Changed 12 years ago by flesler

Attachment: clone[5790].diff added

This is the patch, but it breaks many many tests :(

comment:8 Changed 12 years ago by antix

This works..

var clone = this.cloneNode(true), container = this.ownerDocument.createElement("div"); container.appendChild(clone); return jQuery.clean([container.innerHTML], this)[0];

comment:10 Changed 11 years ago by dmethvin

Resolution: fixed
Status: assignedclosed

Fixed at [6186].

Note: See TracTickets for help on using tickets.