Bug Tracker

Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#7717 closed bug (wontfix)

.clone(true) does not clone data properly

Reported by: kennykwok@… Owned by: dmethvin
Priority: blocker Milestone: 1.5
Component: data Version: 1.5
Keywords: Cc:
Blocked by: Blocking:

Description

Problem found in jQuery 1.4.4.

Modification of data in cloned object will affect the original object.

Here is the sample code:

<script>
$(function() {
  var original_button = $("<input/>", {"type":"button","value":"Original"})
  .appendTo("body")
  .data("obj", ["1"])
  .bind("click", function() {
    alert($(this).data("obj").join(" - ")); });

  var clone_button = original_button.clone(true)
  .val("Clone").appendTo("body");

  clone_button.data("obj").push("2", "3", "4");
});
</script>

Result: Click on "Original" will show "1 - 2 - 3 - 4"

Expected Result: Click on "Original" should show "1"

Note: jQuery 1.4.2 does not have this issue.

Change History (18)

comment:2 Changed 9 years ago by jitter

Component: unfileddata
Milestone: 1.next1.4.5
Priority: undecidedblocker
Status: newopen

comment:3 Changed 9 years ago by dmethvin

Owner: set to dmethvin
Status: openassigned

comment:5 Changed 9 years ago by dmethvin

Keywords: needsdocs added
Resolution: wontfix
Status: assignedclosed

For performance and reliability reasons (e.g., potential circular references in the data) we have decided to only copy the first level of the data object. That is also consistent with the behavior of $().data(obj) which shallow-extends the element's existing data object with obj.

Any sub-objects or arrays in the data will continue to be shared by the cloned elements. If that is not desirable, you can do something like this after the clone operation to (shallow) copy the array of strings:

clone_button.data("arr", original_button.data("arr").concat());

or to deep-copy the array,

clone_button.data("obj", $.extend(true, [], original_button.data("obj")));

or to deep-copy a sub-object in the original data,

clone_button.data("obj", $.extend(true, {}, original_button.data("obj")));

Here's an updated test case: http://jsfiddle.net/dmethvin/uvpPw/1/

comment:6 Changed 9 years ago by Colin Snover

Resolution: wontfixfixed

Fix #7717 and #7165. Thanks to dmethvin and iliakan for their help fixing these issues.

Changeset: faefbb1ad0b81e8001b582d06d5bd9c9236e62ce

comment:7 Changed 9 years ago by snover

Resolution: fixed
Status: closedreopened

comment:8 Changed 9 years ago by snover

Resolution: wontfix
Status: reopenedclosed

That fix was not applied.

comment:9 Changed 8 years ago by anonymous

Petition: Is it possible to extend the clone() to support deep copying of data, in future version of jQuery?

e.g. clone(true, true) with the 2nd "true" to indicate deep copying of data.

comment:10 Changed 8 years ago by john

Version: 1.4.41.5

There wasn't a 1.4.5 release, was actually 1.5.

comment:11 Changed 8 years ago by john

Milestone: 1.4.51.5

There was no 1.4.5 release, was actually 1.5.

comment:12 Changed 8 years ago by timmywil

This should probably be set to fixed and take out the needsdocs. The jQuery.extend has been added since snover's pull. See manipulation.js#L363.

comment:13 in reply to:  12 Changed 8 years ago by jitter

Replying to timmywil:

This should probably be set to fixed and take out the needsdocs. The jQuery.extend has been added since snover's pull. See manipulation.js#L363.

I don't see this fixed. We still only copy the first level of data (extend without deep option) so daves comment is still the way to go for stuff like this.

So it still needs to be documented that clone(true) only copies the first level of data and thus the clone and original might share some nested data objects.

comment:14 Changed 8 years ago by timmywil

Ah right, without the deep option. My bad.

comment:15 Changed 8 years ago by timmywil

Ok, I've created a clap here: http://oksoclap.com/jQuery-clone Also, you do not need to deep extend as you are creating a new object simply by doing the extend.

Notice I've updated Dave's fiddle to not do the $.extend(true, ...) and data is still not shared. http://jsfiddle.net/timmywil/uvpPw/2/

comment:16 Changed 8 years ago by anonymous

needsdocs may be removed. See the updated docs

comment:17 Changed 8 years ago by timmywil

Sorry, wasn't logged in, that last comment was me.

comment:18 Changed 8 years ago by dmethvin

Keywords: needsdocs removed
Note: See TracTickets for help on using tickets.