Refactored before/after/replaceWith to not pushStack. Fixes #12664, closes gh-987

diff --git a/src/manipulation.js b/src/manipulation.js
index 11b6cc430be572f185f9c8c4029fac3e8ba78c18..7d18d04c9423211de6e0f248d0c82e546eeb1c22 100644 (file)
--- a/src/manipulation.js
+++ b/src/manipulation.js
@@ -160,29 +160,19 @@ jQuery.fn.extend({
        },
 
        before: function() {
-               if ( !isDisconnected( this[0] ) ) {
-                       return this.domManip(arguments, false, function( elem ) {
+               return this.domManip( arguments, false, function( elem ) {
+                       if ( !isDisconnected( this ) ) {
                                this.parentNode.insertBefore( elem, this );
-                       });
-               }
-
-               if ( arguments.length ) {
-                       var set = jQuery.clean( arguments );
-                       return this.pushStack( jQuery.merge( set, this ), "before", this.selector );
-               }
+                       }
+               });
        },
 
        after: function() {
-               if ( !isDisconnected( this[0] ) ) {
-                       return this.domManip(arguments, false, function( elem ) {
+               return this.domManip( arguments, false, function( elem ) {
+                       if ( !isDisconnected( this ) ) {
                                this.parentNode.insertBefore( elem, this.nextSibling );
-                       });
-               }
-
-               if ( arguments.length ) {
-                       var set = jQuery.clean( arguments );
-                       return this.pushStack( jQuery.merge( this, set ), "after", this.selector );
-               }
+                       }
+               });
        },
 
        // keepData is for internal use only--do not document
@@ -277,40 +267,41 @@ jQuery.fn.extend({
        },
 
        replaceWith: function( value ) {
-               if ( !isDisconnected( this[0] ) ) {
-                       // Make sure that the elements are removed from the DOM before they are inserted
-                       // this can help fix replacing a parent with child elements
-                       if ( jQuery.isFunction( value ) ) {
-                               return this.each(function( index ) {
-                                       // HTML argument replaced by "this" element
-                                       // 1. There were no supporting tests
-                                       // 2. There was no internal code relying on this
-                                       // 3. There was no documentation of an html argument
-                                       jQuery( this ).replaceWith( value.call( this, index, this ) );
-                               });
-                       }
+               var self = this,
+                       isFunc = jQuery.isFunction( value );
 
-                       if ( typeof value !== "string" ) {
-                               value = jQuery( value ).detach();
-                       }
+               // Make sure that the elements are removed from the DOM before they are inserted
+               // this can help fix replacing a parent with child elements
+               if ( !isFunc && typeof value !== "string" ) {
+                       value = jQuery( value ).detach();
+               }
 
-                       return this.each(function() {
-                               var next = this.nextSibling,
-                                       parent = this.parentNode;
+               this.each( function( i ) {
+                       var next = this.nextSibling,
+                               parent = this.parentNode,
+                               // HTML argument replaced by "this" element
+                               // 1. There were no supporting tests
+                               // 2. There was no internal code relying on this
+                               // 3. There was no documentation of an html argument
+                               val = !isFunc ? value : value.call( this, i, this );
+
+                       if ( isDisconnected( this ) ) {
+                               // for disconnected elements, we replace with the new content in the set. We use
+                               // clone here to ensure that each replaced instance is unique
+                               self[ i ] = jQuery( val ).clone()[ 0 ];
+                               return;
+                       }
 
-                               jQuery( this ).remove();
+                       jQuery( this ).remove();
 
-                               if ( next ) {
-                                       jQuery(next).before( value );
-                               } else {
-                                       jQuery(parent).append( value );
-                               }
-                       });
-               }
+                       if ( next ) {
+                               jQuery( next ).before( val );
+                       } else {
+                               jQuery( parent ).append( val );
+                       }
+               });
 
-               return this.length ?
-                       this.pushStack( jQuery(jQuery.isFunction(value) ? value() : value), "replaceWith", value ) :
-                       this;
+               return this;
        },
 
        detach: function( selector ) {

diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index ca3a2ef4036fc4d0e2fe06a51174798e3db57d2d..7b5fee19778043735dd63b2125b81ffdd7590435 100644 (file)
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -929,8 +929,8 @@ var testBefore = function(val) {
        equal( jQuery("#en").text(), expected, "Insert array of jQuery objects before" );
 
        var set = jQuery("<div/>").before("<span>test</span>");
-       equal( set[0].nodeName.toLowerCase(), "span", "Insert the element before the disconnected node." );
-       equal( set.length, 2, "Insert the element before the disconnected node." );
+       equal( set[0].nodeName.toLowerCase(), "div", "Insert before a disconnected node should be a no-op" );
+       equal( set.length, 1, "Insert the element before the disconnected node. should be a no-op" );
 };
 
 test("before(String|Element|Array&lt;Element&gt;|jQuery)", function() {
@@ -942,18 +942,35 @@ test("before(Function)", function() {
 });
 
 test("before and after w/ empty object (#10812)", function() {
-       expect(2);
+       expect(1);
 
        var res = jQuery( "#notInTheDocument" ).before( "(" ).after( ")" );
-       equal( res.length, 2, "didn't choke on empty object" );
-       equal( res.wrapAll("<div/>").parent().text(), "()", "correctly appended text" );
+       equal( res.length, 0, "didn't choke on empty object" );
 });
 
 test("before and after on disconnected node (#10517)", function() {
-       expect(2);
+       expect(6);
+       var expectedBefore = "This is a normal link: bugaYahoo",
+               expectedAfter = "This is a normal link: Yahoobuga";
+
+       equal( jQuery("<input type='checkbox'/>").before("<div/>").length, 1, "before() on disconnected node is no-op" );
+       equal( jQuery("<input type='checkbox'/>").after("<div/>").length, 1, "after() on disconnected node is no-op" );
+
+       QUnit.reset();
+       jQuery("#yahoo").add("<span/>").before("<b>buga</b>");
+       equal( jQuery("#en").text(), expectedBefore, "Insert String before with disconnected node last" );
+
+       QUnit.reset();
+       jQuery("<span/>").add("#yahoo").before("<b>buga</b>");
+       equal( jQuery("#en").text(), expectedBefore, "Insert String before with disconnected node first" );
+
+       QUnit.reset();
+       jQuery("#yahoo").add("<span/>").after("<b>buga</b>");
+       equal( jQuery("#en").text(), expectedAfter, "Insert String after with disconnected node last" );
 
-       equal( jQuery("<input type='checkbox'/>").before("<div/>").length, 2, "before() returned all elements" );
-       equal( jQuery("<input type='checkbox'/>").after("<div/>").length, 2, "after() returned all elements" );
+       QUnit.reset();
+       jQuery("<span/>").add("#yahoo").after("<b>buga</b>");
+       equal( jQuery("#en").text(), expectedAfter, "Insert String after with disconnected node first" );
 });
 
 test("insertBefore(String|Element|Array&lt;Element&gt;|jQuery)", function() {
@@ -1004,9 +1021,9 @@ var testAfter = function(val) {
        jQuery("#yahoo").after( val( [ jQuery("#first"), jQuery("#mark, #google") ] ) );
        equal( jQuery("#en").text(), expected, "Insert array of jQuery objects after" );
 
-       var set = jQuery("<div/>").after("<span>test</span>");
-       equal( set[1].nodeName.toLowerCase(), "span", "Insert the element after the disconnected node." );
-       equal( set.length, 2, "Insert the element after the disconnected node." );
+       var set = jQuery("<div/>").before("<span>test</span>");
+       equal( set[0].nodeName.toLowerCase(), "div", "Insert after a disconnected node should be a no-op" );
+       equal( set.length, 1, "Insert the element after the disconnected node should be a no-op" );
 };
 
 test("after(String|Element|Array&lt;Element&gt;|jQuery)", function() {
@@ -1155,6 +1172,41 @@ test("replaceWith(string) for more than one element", function(){
        equal(jQuery("#foo p").length, 0, "verify that all the three original element have been replaced");
 });
 
+test("replaceWith(string) for collection with disconnected element", function(){
+       expect(18);
+
+       var elem = jQuery("<div />"),
+               testSet, newSet;
+
+       QUnit.reset();
+       testSet = jQuery("#foo p").add(elem);
+       equal(testSet.length, 4, "ensuring that test data has not changed");
+
+       newSet = testSet.replaceWith("<span>bar</span>");
+       equal(testSet.length, 4, "ensure that we still have the same number of elements");
+       equal(jQuery("#foo span").length, 3, "verify that all the three original elements have been replaced");
+       equal(jQuery("#foo p").length, 0, "verify that all the three original elements have been replaced");
+       equal(testSet.filter("p").length, 3, "ensure we still have the original set of attached elements");
+       equal(testSet.filter("div").length, 0, "ensure the detached element is not in the original set");
+       equal(newSet.filter("p").length, 3, "ensure we still have the original set of attached elements in new set");
+       equal(newSet.filter("div").length, 0, "ensure the detached element has been replaced in the new set");
+       equal(newSet.filter("span").length, 1, "ensure the new element is in the new set");
+
+       QUnit.reset();
+       testSet = elem.add(jQuery("#foo p"));
+       equal(testSet.length, 4, "ensuring that test data has not changed");
+
+       testSet.replaceWith("<span>bar</span>");
+       equal(testSet.length, 4, "ensure that we still have the same number of elements");
+       equal(jQuery("#foo span").length, 3, "verify that all the three original elements have been replaced");
+       equal(jQuery("#foo p").length, 0, "verify that all the three original elements have been replaced");
+       equal(testSet.filter("p").length, 3, "ensure we still have the original set of attached elements");
+       equal(testSet.filter("div").length, 0, "ensure the detached element is not in the original set");
+       equal(newSet.filter("p").length, 3, "ensure we still have the original set of attached elements in new set");
+       equal(newSet.filter("div").length, 0, "ensure the detached element has been replaced in the new set");
+       equal(newSet.filter("span").length, 1, "ensure the new element is in the new set");
+});
+
 test("replaceAll(String|Element|Array&lt;Element&gt;|jQuery)", function() {
        expect(10);
        jQuery("<b id='replace'>buga</b>").replaceAll("#yahoo");