wrap removes expandos

[Sam]

When you create an element, add expandos to it, then use wrap, the expandos that have been added are removed. This happens with all the browsers I have tried it on.

var wrapper = document.createElement("div");
wrapper.sometext = "Some text";
wrapper.clickcss = {border: "2px solid #eee", padding: "6px"};
$("#foo").wrap(wrapper).click(
	function(e)
	{
		e.preventDefault();
		alert(this.parentNode.sometext);
		$(this.parentNode).css(this.parentNode.clickcss);
	}
);

​Example

The CSS is set in Internet Explorer when jQuery 1.1.2 (the default on the example page) is used, but not when 1.2.1 is used.

The alert is 'undefined' in Firefox 2, but shows the correct text in Internet Explorer 6 and 7.

So:

Firefox: both expandos are removed

Internet Explorer: object expando (clickcss) removed, text expando still present

[brandon]

Patch to remove clone from wrapAll

[brandon]

The wrap method (actually wrapAll) does a clone on the supplied HTML. Doing a clone does not clone expandos and is why you are getting this error. I'm not sure what the purpose is of doing a clone ... maybe to avoid moving an existing DOM element, but who is to say I don't want to move that element. I suggest we remove the call to clone and allow the user to decide if they want to clone the element or not. However, I could be missing something simple as to why we are doing a clone. I've attached a patch to remove the call to clone and allow this code to work as expected.

[Sam]

I'm not entirely sure why there is a clone in their as well.

Perhaps clone can be fixed to enable expandos to be copied as well?

[Sam]

their should be there...

[dmethvin]

The .wrapAll documentation says:

This works by going through the first element provided (which is generated,
on the fly, from the provided HTML) and finds the deepest ancestor element
within its structure -- it is that element that will enwrap everything else.

This patch fixes an undocumented case; the arguments can actually be a DOM node or array-like collection of DOM nodes and I guess you want that to work.

Sam's example at ​http://www.texotela.co.uk/wrapexpandos.php only selects a single element #foo, but if it selected multiple elements then removing the .clone from .wrapAll would break it:

var wrapper = $("<div>").attr("special", "very")[0];
$("p").wrap(wrapper);

In FF with the patch installed, I get a message "Node cannot be inserted at this point in the hierarchy" and in IE something similar but less coherent.

It seems like the .clone was there for the benefit of .wrap and .wrapInner. Perhaps they can be implemented through .domManip with .wrapAll in the callback? Something like this works for the case above:

   wrap: function() {
        return this.domManip(arguments, false, 1, function(elem){
            jQuery( this ).wrapAll( elem );
        });
   },

This behaves differently for the currently undocumented case where multiple nodes are provided for the wrapper though; it would wrap the jQuery nodes in a successively nested wrapper for each top-level argument, with the last argument being the outermost wrapper. I think that's an equally reasonable interpretation of a weird case. I wasn't quite sure how execScripts might factor into this, that's new code since the last time I looked at .domManip.

None of this solves the problem of .clone clobbering expandos, including the "hidden" uses of clone by append/prepend/domManip/etc. when multiple nodes are selected. I don't know of any cross-browser way to enumerate custom expandos and copy them over.

[Sam]

This still does not work in jQuery 1.2.2b2.

While I only need to preserve expandos in Internet Explorer, it would be good if they were also preserved in Firefox and other browsers (they are removed in Firefox, regardless of which version of jQuery you use).

[dmethvin]

I guess we've been leaving this open in the hopes that some solution will arrive. However, none of the underlying DOM methods preserve properties/expandos so it's unlikely we could do it either without a lot of overhead. If anyone comes up with a bettersolution let us know and we can reopen the ticket.