Use the original element/fragment as the last item to be appended to the document...
authorColin Snover <github.com@zetafleet.com>
Fri, 28 Jan 2011 16:55:39 +0000 (10:55 -0600)
committerColin Snover <github.com@zetafleet.com>
Fri, 28 Jan 2011 16:55:39 +0000 (10:55 -0600)
src/manipulation.js
test/unit/manipulation.js

index b7f2d3e..442a14d 100644 (file)
@@ -186,7 +186,7 @@ jQuery.fn.extend({
        clone: function( dataAndEvents, deepDataAndEvents ) {
                dataAndEvents = dataAndEvents == null ? true : dataAndEvents;
                deepDataAndEvents = deepDataAndEvents == null ? dataAndEvents : deepDataAndEvents;
-               
+
                return this.map( function () {
                        return jQuery.clone( this, dataAndEvents, deepDataAndEvents );
                });
@@ -311,12 +311,19 @@ jQuery.fn.extend({
                        if ( first ) {
                                table = table && jQuery.nodeName( first, "tr" );
 
-                               for ( var i = 0, l = this.length; i < l; i++ ) {
+                               for ( var i = 0, l = this.length, lastIndex = l - 1; i < l; i++ ) {
                                        callback.call(
                                                table ?
                                                        root(this[i], first) :
                                                        this[i],
-                                               i > 0 || results.cacheable || (this.length > 1 && i > 0) ?
+                                               // Make sure that we do not leak memory by inadvertently discarding
+                                               // the original fragment (which might have attached data) instead of
+                                               // using it; in addition, use the original fragment object for the last
+                                               // item instead of first because it can end up being emptied incorrectly
+                                               // in certain situations (Bug #8070).
+                                               // Fragments from the fragment cache must always be cloned and never used
+                                               // in place.
+                                               results.cacheable || (l > 1 && i < lastIndex) ?
                                                        jQuery.clone( fragment, true, true ) :
                                                        fragment
                                        );
@@ -484,9 +491,9 @@ jQuery.each({
 
 jQuery.extend({
        clone: function( elem, dataAndEvents, deepDataAndEvents ) {
-               var clone = elem.cloneNode(true), 
-                               srcElements, 
-                               destElements, 
+               var clone = elem.cloneNode(true),
+                               srcElements,
+                               destElements,
                                i;
 
                if ( !jQuery.support.noCloneEvent && (elem.nodeType === 1 || elem.nodeType === 11) && !jQuery.isXMLDoc(elem) ) {
index 739868b..f0e2eae 100644 (file)
@@ -883,6 +883,19 @@ test("jQuery.clone() (#8017)", function() {
        equals( main.childNodes.length, clone.childNodes.length, "Simple child length to ensure a large dom tree copies correctly" );
 });
 
+test("clone() (#8070)", function () {
+       expect(2);
+
+       jQuery('<select class="test8070"></select><select class="test8070"></select>').appendTo('#main');
+       var selects = jQuery('.test8070');
+       selects.append('<OPTION>1</OPTION><OPTION>2</OPTION>');
+
+       equals( selects[0].childNodes.length, 2, "First select got two nodes" );
+       equals( selects[1].childNodes.length, 2, "Second select got two nodes" );
+
+       selects.remove();
+});
+
 test("clone()", function() {
        expect(37);
        equals( 'This is a normal link: Yahoo', jQuery('#en').text(), 'Assert text for #en' );