Bug Tracker

Opened 14 years ago

Closed 13 years ago

#921 closed bug (fixed)

IE Fires click event after doing a .clone()

Reported by: elicochran@… Owned by:
Priority: critical Milestone: 1.1.3
Component: event Version: 1.1.1
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by brandon)

Adding a function to a new "cloned" object causes function to fire in IE

There is an example at: http://aos.elicochran.com/

Example works fine in Firefox (on Mac and Windows), Safari, but on IE 6&7 instead of dimming the source row, the row is deleted.

The offending code is:

function addRowElement(row_obj) {

if ( $(row_obj).attr('class') != "dim" ) {

var newRow = $(row_obj).clone(true); $(newRow).attr('name',$(row_obj).attr('id')); $(newRow).attr('id',);

$(newRow).click( function() {

removeRowElement(this) ; return false;

}); if ( $('#placeholder') ) $('#placeholder').remove() ;

$(newRow).appendTo('#bucket'); $(row_obj).addClass("dim");

} scrollBottom(); setCounter();

}

Oddly enough when the same code is called from this loop, it works fine. (Try the Add All button in the example.)

function addAllRows() {

$('#fruit_list tr').each(function(count) {

addRowElement(this);

});

}

This may be related to the following thread on the forum: http://www.nabble.com/Bind-triggers-function-t3146103.html#a8721515

Attachments (3)

test.html (1.5 KB) - added by brice 13 years ago.
IE Clone Test
jquery.diff (667 bytes) - added by brandon 13 years ago.
Patch
jquery.clone.diff (1.2 KB) - added by brandon 13 years ago.
This patch fixes it all.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 14 years ago by brandon

Priority: majorcritical
Summary: Adding a function to a new "cloned" object causes function to fire in IEIE Fires click event after doing a .clone()

I've created a simplified test case here: http://brandon.jquery.com/testing/921/

This is related to #863

comment:2 Changed 13 years ago by brandon

Resolution: fixed
Status: newclosed

This is not related to #863.

The issue was with the $events expando not being nulled out on the clone.

Fixed in Revision 1407.

Changed 13 years ago by brice

Attachment: test.html added

IE Clone Test

comment:3 Changed 13 years ago by brice

Resolution: fixed
Status: closedreopened

IE $.clone() event is still prevalent.

In short: The scope of clones is the cloned element (donor), not the clone. Further, events attached to a "clone" are instantly fired, and events attached to the donor remain attached to the clone.

See: http://dev.iceburg.net/jquery/cloneTest.html (source will be attached to ensure availability)

~ Brice

comment:4 Changed 13 years ago by edwin

Very nasty (and critical) bug.

My workaround:

Instead of:

myClonedElement.click(myHandler);

Do this:

myClonedElement.onclick = myHandler;

Ofcourse this doesn't work if you want to attach more handlers of the same type to the same element.

comment:5 Changed 13 years ago by edwin

Correction:

My workaround:

Instead of:

myClonedElementJQueryObject.click(myHandler);

Do this:

myClonedElementJQueryObject.get(0).onclick = myHandler;

comment:6 Changed 13 years ago by john

need: Test Case

comment:8 in reply to:  3 Changed 13 years ago by brandon

need: Test CasePatch

Replying to brice:

Thanks for the great test case. I'm investigating this further.

Changed 13 years ago by brandon

Attachment: jquery.diff added

Patch

comment:9 Changed 13 years ago by brandon

need: PatchCommit

I believe this patch puts this nasty bug to rest. You can see the test with the patched jQuery here: http://brandon.jquery.com/testing/921/newTest.html

comment:10 Changed 13 years ago by john

Milestone: 1.1.3
Version: 1.1.1

Does this only occur for child elements? or does it also occur for descendant elements as well?

a = a.cloneNode( deep != undefined ? deep : true );

// drop $events expando to avoid firing incorrect events in IE
jQuery(a).find("*").add(a).each(function(){
  this.$events = null;
}); 

Also, if this only occurs in IE, then we may want to add a conditional statement; but I suspect that this also occurs in Opera.

comment:11 in reply to:  10 Changed 13 years ago by brandon

Replying to john: I believe that would be the best way to handle this and it is only happening in IE. This is because IE clones the event handlers along with the element.

comment:12 Changed 13 years ago by brandon

need: CommitPatch

Since the switch to DOM Level 2 handlers, this patch/method no longer works.

comment:13 Changed 13 years ago by brandon

Everything I've tried since moving to DOM Level 2 handlers is not working out very well. Anything you do to the parent is also done to the clone and anything you do clone is done to the parent. The only other way around this would be to actually implement a custom clone method for IE. A method that would document.createElement, copy the attributes and handle the children on deep clones. Messy but it seems that this will be the only way around this nasty bug.

comment:14 Changed 13 years ago by brandon

Description: modified (diff)

Okay I've managed to fix this bug in its simplest form.

http://brandonaaron.net/jquery/tickets/921/test2.html

Changed 13 years ago by brandon

Attachment: jquery.clone.diff added

This patch fixes it all.

comment:15 Changed 13 years ago by brandon

Resolution: fixed
Status: reopenedclosed

This is now fixed in Rev [1906].

Note: See TracTickets for help on using tickets.