#7717 closed bug (wontfix)

Opened December 07, 2010 09:00AM UTC

Closed December 23, 2010 12:23AM UTC

Last modified April 07, 2011 02:14PM UTC

.clone(true) does not clone data properly

Reported by:	kennykwok@gmail.com	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.

Attachments (0)

Change History (18)

Changed December 07, 2010 09:09AM UTC by anonymous comment:1

http://jsfiddle.net/uvpPw/

Changed December 07, 2010 11:39PM UTC by jitter comment:2

component:	unfiled → data
milestone:	1.next → 1.4.5
priority:	undecided → blocker
status:	new → open

reduced test case\\\\

test case with object instead of array

Changed December 09, 2010 04:23AM UTC by dmethvin comment:3

owner:	→ dmethvin
status:	open → assigned

Changed December 11, 2010 04:04AM UTC by dmethvin comment:4

Fix: https://github.com/jquery/jquery/pull/126

Changed December 22, 2010 07:11PM UTC by dmethvin comment:5

keywords:	→ needsdocs
resolution:	→ wontfix
status:	assigned → closed

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/

Changed December 23, 2010 12:21AM UTC by Colin Snover comment:6

resolution:	wontfix → fixed

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

Changeset: faefbb1ad0b81e8001b582d06d5bd9c9236e62ce

Changed December 23, 2010 12:22AM UTC by snover comment:7

resolution:	fixed
status:	closed → reopened

Changed December 23, 2010 12:23AM UTC by snover comment:8

resolution:	→ wontfix
status:	reopened → closed

That fix was not applied.

Changed January 06, 2011 04:38AM UTC by anonymous comment:9

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.

Changed January 31, 2011 05:53PM UTC by john comment:10

version:	1.4.4 → 1.5

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

Changed January 31, 2011 05:54PM UTC by john comment:11

milestone:	1.4.5 → 1.5

There was no 1.4.5 release, was actually 1.5.

Changed February 17, 2011 11:36PM UTC by timmywil comment:12

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.

Changed February 18, 2011 01:29PM UTC by jitter comment:13

Replying to [comment:12 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.

Changed February 22, 2011 12:56AM UTC by timmywil comment:14

Ah right, without the deep option. My bad.

Changed February 22, 2011 01:44AM UTC by timmywil comment:15

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/

Changed February 26, 2011 05:16PM UTC by anonymous comment:16

needsdocs may be removed. See the updated docs

Changed February 26, 2011 05:17PM UTC by timmywil comment:17

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

Changed April 07, 2011 02:14PM UTC by dmethvin comment:18

keywords:	needsdocs

Side navigation