Bug Tracker

Modify

Ticket #7717 (closed bug: wontfix)

Opened 3 years ago

Last modified 2 years ago

.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:
Blocking: Blocked by:

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

comment:2 Changed 3 years ago by jitter

  • Priority changed from undecided to blocker
  • Status changed from new to open
  • Component changed from unfiled to data
  • Milestone changed from 1.next to 1.4.5

comment:3 Changed 3 years ago by dmethvin

  • Owner set to dmethvin
  • Status changed from open to assigned

comment:5 Changed 2 years ago by dmethvin

  • Keywords needsdocs added
  • Status changed from assigned to closed
  • Resolution set to wontfix

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 2 years ago by Colin Snover

  • Resolution changed from wontfix to fixed

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

Changeset: faefbb1ad0b81e8001b582d06d5bd9c9236e62ce

comment:7 Changed 2 years ago by snover

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:8 Changed 2 years ago by snover

  • Status changed from reopened to closed
  • Resolution set to wontfix

That fix was not applied.

comment:9 Changed 2 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 2 years ago by john

  • Version changed from 1.4.4 to 1.5

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

comment:11 Changed 2 years ago by john

  • Milestone changed from 1.4.5 to 1.5

There was no 1.4.5 release, was actually 1.5.

comment:12 follow-up: ↓ 13 Changed 2 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 2 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 2 years ago by timmywil

Ah right, without the deep option. My bad.

comment:15 Changed 2 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 2 years ago by anonymous

needsdocs may be removed. See the updated  docs

comment:17 Changed 2 years ago by timmywil

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

comment:18 Changed 2 years ago by dmethvin

  • Keywords needsdocs removed

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.