Manipulation: Detect sneaky no-content replaceWith input

Fixes gh-2204 Ref 642e9a4 Closes gh-1752 Closes gh-2206
jquery · Apr 30, 2015 · 4b27ae1 · 4b27ae1
1 parent 1541664
commit 4b27ae1
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 22 deletions.
diff --git a/src/manipulation.js b/src/manipulation.js
@@ -201,7 +201,7 @@ jQuery.extend({
 		return clone;
 	},
 
-	buildFragment: function( elems, context, scripts, selection ) {
+	buildFragment: function( elems, context, scripts, selection, ignored ) {
 		var elem, tmp, tag, wrap, contains, j,
 			fragment = context.createDocumentFragment(),
 			nodes = [],
@@ -257,9 +257,11 @@ jQuery.extend({
 		i = 0;
 		while ( (elem = nodes[ i++ ]) ) {
 
-			// #4087 - If origin and destination elements are the same, and this is
+			// Skip elements already in the context collection (trac-4087)
-			// that element, do not do anything
 			if ( selection && jQuery.inArray( elem, selection ) > -1 ) {
+				if ( ignored ) {
+					ignored.push( elem );
+				}
 				continue;
 			}
 
@@ -446,28 +448,28 @@ jQuery.fn.extend({
 	},
 
 	replaceWith: function() {
-		var arg = arguments[ 0 ];
+		var ignored = [];
-
-		// Make the changes, replacing each context element with the new content
-		this.domManip( arguments, function( elem ) {
-			arg = this.parentNode;
 
-			jQuery.cleanData( getAll( this ) );
+		// Make the changes, replacing each non-ignored context element with the new content
+		return this.domManip( arguments, function( elem ) {
+			var parent = this.parentNode;
 
-			if ( arg ) {
+			if ( jQuery.inArray( this, ignored ) < 0 ) {
-				arg.replaceChild( elem, this );
+				jQuery.cleanData( getAll( this ) );
+				if ( parent ) {
+					parent.replaceChild( elem, this );
+				}
 			}
-		});
 
-		// Force removal if there was no new content (e.g., from empty arguments)
+		// Force callback invocation
-		return arg && (arg.length || arg.nodeType) ? this : this.remove();
+		}, ignored );
 	},
 
 	detach: function( selector ) {
 		return this.remove( selector, true );
 	},
 
-	domManip: function( args, callback ) {
+	domManip: function( args, callback, ignored ) {
 
 		// Flatten any nested arrays
 		args = concat.apply( [], args );
@@ -489,19 +491,20 @@ jQuery.fn.extend({
 				if ( isFunction ) {
 					args[ 0 ] = value.call( this, index, self.html() );
 				}
-				self.domManip( args, callback );
+				self.domManip( args, callback, ignored );
 			});
 		}
 
 		if ( l ) {
-			fragment = jQuery.buildFragment( args, this[ 0 ].ownerDocument, false, this );
+			fragment = jQuery.buildFragment( args, this[ 0 ].ownerDocument, false, this, ignored );
 			first = fragment.firstChild;
 
 			if ( fragment.childNodes.length === 1 ) {
 				fragment = first;
 			}
 
-			if ( first ) {
+			// Require either new content or an interest in ignored elements to invoke the callback
+			if ( first || ignored ) {
 				scripts = jQuery.map( getAll( fragment, "script" ), disableScript );
 				hasScripts = scripts.length;
 

diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
@@ -1279,22 +1279,38 @@ 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( "Empty replaceWith (#13401; #13596)", 8, function() {
+test( "Empty replaceWith (trac-13401; trac-13596; gh-2204)", function() {
-	var $el = jQuery( "<div/>" ),
+
+	expect( 25 );
+
+	var $el = jQuery( "<div/><div/>" ).html( "<p>0</p>" ),
+		expectedHTML = $el.html(),
 		tests = {
 			"empty string": "",
 			"empty array": [],
+			"array of empty string": [ "" ],
 			"empty collection": jQuery( "#nonexistent" ),
 
-       // in case of jQuery(...).replaceWith();
+			// in case of jQuery(...).replaceWith();
-			"empty undefined": undefined
+			"undefined": undefined
 		};
 
 	jQuery.each( tests, function( label, input ) {
 		$el.html( "<a/>" ).children().replaceWith( input );
 		strictEqual( $el.html(), "", "replaceWith(" + label + ")" );
 		$el.html( "<b/>" ).children().replaceWith(function() { return input; });
 		strictEqual( $el.html(), "", "replaceWith(function returning " + label + ")" );
+		$el.html( "<i/>" ).children().replaceWith(function( i ) { i; return input; });
+		strictEqual( $el.html(), "", "replaceWith(other function returning " + label + ")" );
+		$el.html( "<p/>" ).children().replaceWith(function( i ) {
+			return i ?
+				input :
+				jQuery( this ).html( i + "" );
+		});
+		strictEqual( $el.eq( 0 ).html(), expectedHTML,
+			"replaceWith(function conditionally returning context)" );
+		strictEqual( $el.eq( 1 ).html(), "",
+			"replaceWith(function conditionally returning " + label + ")" );
 	});
 });