Skip to content

Commit

Permalink
Refactored before/after/replaceWith to not pushStack. Fixes #12664, c…
Browse files Browse the repository at this point in the history 
…loses gh-987
  • Loading branch information
@rwaldron
Sai Wong authored and rwaldron committed Oct 16, 2012
1 parent 51feba6 commit 0c1cea3
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 58 deletions.
85 changes: 38 additions & 47 deletions src/manipulation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ) {
Expand Down
74 changes: 63 additions & 11 deletions test/unit/manipulation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 0c1cea3

Please sign in to comment.