From 0c1cea376196d24acc73c64dbdff1880a2dfd006 Mon Sep 17 00:00:00 2001 From: Sai Wong Date: Tue, 16 Oct 2012 16:50:30 -0400 Subject: [PATCH] Refactored before/after/replaceWith to not pushStack. Fixes #12664, closes gh-987 --- src/manipulation.js | 85 +++++++++++++++++---------------------- test/unit/manipulation.js | 74 +++++++++++++++++++++++++++++----- 2 files changed, 101 insertions(+), 58 deletions(-) diff --git a/src/manipulation.js b/src/manipulation.js index 11b6cc430..7d18d04c9 100644 --- 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 ca3a2ef40..7b5fee197 100644 --- 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("
").before("test"); - 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<Element>|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("
").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("").before("
").length, 1, "before() on disconnected node is no-op" ); + equal( jQuery("").after("
").length, 1, "after() on disconnected node is no-op" ); + + QUnit.reset(); + jQuery("#yahoo").add("").before("buga"); + equal( jQuery("#en").text(), expectedBefore, "Insert String before with disconnected node last" ); + + QUnit.reset(); + jQuery("").add("#yahoo").before("buga"); + equal( jQuery("#en").text(), expectedBefore, "Insert String before with disconnected node first" ); + + QUnit.reset(); + jQuery("#yahoo").add("").after("buga"); + equal( jQuery("#en").text(), expectedAfter, "Insert String after with disconnected node last" ); - equal( jQuery("").before("
").length, 2, "before() returned all elements" ); - equal( jQuery("").after("
").length, 2, "after() returned all elements" ); + QUnit.reset(); + jQuery("").add("#yahoo").after("buga"); + equal( jQuery("#en").text(), expectedAfter, "Insert String after with disconnected node first" ); }); test("insertBefore(String|Element|Array<Element>|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("
").after("test"); - 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("
").before("test"); + 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<Element>|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("
"), + testSet, newSet; + + QUnit.reset(); + testSet = jQuery("#foo p").add(elem); + equal(testSet.length, 4, "ensuring that test data has not changed"); + + newSet = testSet.replaceWith("bar"); + 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("bar"); + 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<Element>|jQuery)", function() { expect(10); jQuery("buga").replaceAll("#yahoo"); -- 2.39.5