Skip to main content

Bug Tracker

Side navigation

#12197 closed bug (wontfix)

Opened August 04, 2012 07:04PM UTC

Closed November 22, 2012 03:08PM UTC

Last modified July 18, 2013 01:06PM UTC

.clone(true, true) does not clone DATA on text nodes

Reported by: info@rok-kralj.net Owned by: info@rok-kralj.net
Priority: low Milestone: 2.0
Component: manipulation Version: 1.8rc1
Keywords: Cc:
Blocked by: Blocking:
Description

As title says it all, the data is not preserved on text nodes, despite deep clone.

Below is the session from JS console.

var a=$('<p>a</p>'); //create an object with one text node
undefined
----
a.contents().data('key', 'value'); //add data to that text node
["a"]
----
a.contents().data('key'); //check presence of data, succeeds
"value"
----
var b=a.clone(true, true); //deep clone object a to b
undefined
----
b.contents().data('key');
undefined

Expected output of last query should be "value". Thanks for your interest.

Attachments (0)
Change History (13)

Changed August 04, 2012 11:38PM UTC by anonymous comment:1

http://jsfiddle.net/MApSt/2/

var a=$('<p>a</p>'); //create an object with one text node
undefined

a.contents().data('key', 'value'); //add data to that text node
["a"]

a.contents().data('key'); //check presence of data, succeeds
"value"

var b=a.clone(true, true); //deep clone object a to b
undefined

b.contents().data('key');
undefined

Changed August 05, 2012 02:40AM UTC by dmethvin comment:2

owner: → info@rok-kralj.net
status: newpending

Try your test on IE 6/7/8, it throws an error when trying to set data on a text node. That's why jQuery doesn't even try to do it when cloning.

The operations allowed on non-element nodes in jQuery is **very** limited, by design.

What exactly are you trying to do?

Changed August 05, 2012 08:11AM UTC by info@rok-kralj.net comment:3

status: pendingnew

Oh. It is a translation script and I store some data directly to text nodes. It is cool and it works, even on very dynamic data. https://tarock.us/allow - try clicking the US flag multiple times.

For now, I am going to implement "nasty workaround". On clone, I am going to traverse complete descendant tree element by element, assigning the data from source to clone.

As far as I am concerned, this should be marked as medium priority bug. I think that if something can't be done on some weird X browser, that is not an excuse not to do it on A,B,C,D browsers. You have just shown an example - data on text nodes works on A,B,C,D and not on X.

Thank you for your time.

Changed August 06, 2012 04:56PM UTC by info@rok-kralj.net comment:4

Any updates?

And WHY did the status change from pending to new?

Changed August 06, 2012 05:07PM UTC by rwaldron comment:5

"pending" means we're waiting for info from you; when you respond the status is changed from "pending" back to the previous status, which was "new"

Changed August 06, 2012 05:23PM UTC by dmethvin comment:6

_comment0: > some weird X browser \ \ IE 6/7/8 are still something like one-third of all Internet users. If we "supported" setting data on text nodes we'd just be setting ourselves up for the inevitable bug report that it doesn't work in IE 6/7/8. In contrast, it took someone **six years** to report that they couldn't set data on a text node, so it's not a critical issue for too many folks. \ \ Attaching data to text nodes cannot be done cross-browser, so it's not likely to happen until jQuery 2.0 when IE 6-9 are dropped -- assuming it works elsewhere. I'm still not convinced this is good practice; text and comment nodes are not something jQuery manipulate consistently vs. elements. \ \ The "nasty workaround" you describe is exactly what we'll have to do inside jQuery to make this work, so if your concern is about performance it won't help to have us fix this soon anyway.1344273870289170
component: unfiledmanipulation
milestone: None2.0
priority: undecidedlow
status: newopen
some weird X browser

IE 6/7/8 are still something like one-third of all Internet users. If we "supported" setting data on text nodes we'd just be setting ourselves up for the inevitable bug report that it doesn't work in IE 6/7/8. In contrast, it took someone **six years** to report that they couldn't set data on a text node, so it's not a critical issue for too many folks.

Attaching data to text nodes cannot be done cross-browser, so it's not likely to happen until jQuery 2.0 when IE 6-8 are dropped -- assuming it works elsewhere. I'm still not convinced this is good practice; text and comment nodes are not something jQuery manipulate consistently vs. elements.

The "nasty workaround" you describe is exactly what we'll have to do inside jQuery to make this work, so if your concern is about performance it won't help to have us fix this soon anyway.

Changed August 06, 2012 05:35PM UTC by info@rok-kralj.net comment:7

Ok, that is fine with me.

But you can do this workaround in source in O(n), while I can do it O(n²). Any plans to add callback to clone, so I could do it in O(n) like this?

$('selector').clone( function(origin, copy) {
    copy.data(origin.data());
});

Changed August 06, 2012 05:38PM UTC by rwaldron comment:8

clone() isn't that simple—it must also handle all children of the source element as well.

Changed August 06, 2012 05:46PM UTC by info@rok-kralj.net comment:9

Oh, the callback function is ment to be called for each children/descendant ''separately''. It enables you to do some work in paralell with origin and copy.

I might file a feature request sometime. Thanks for now!

Changed November 22, 2012 03:08PM UTC by dmethvin comment:10

resolution: → wontfix
status: openclosed

I'm not sure why I thought this would be a good idea, but I don't think it would be a good idea now. There are not a lot of good reasons to attach data to text and comment nodes.

Changed November 22, 2012 04:00PM UTC by info@rok-kralj.net comment:11

Probably because this is not an enhancement, but a defect.

It is possible for text nodes to have data, but .clone() doesn't copy it, despite that being guaranteed in the documentation.

Changed November 22, 2012 05:29PM UTC by dmethvin comment:12

You're not even supposed to have text nodes in a jQuery collection, most methods don't like them. And of course oldIE won't support it. If you can point out the guarantee in the documentation we can correct it.

Changed July 18, 2013 01:06PM UTC by dmethvin comment:13

#14149 is a duplicate of this ticket.