From 4b27ae16a2b911f75b341b56d9d939bc65a9657a Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 13 Apr 2015 16:05:48 -0400 Subject: [PATCH] Manipulation: Detect sneaky no-content replaceWith input Fixes gh-2204 Ref 642e9a45579cfa90861b8ea71a95dd077775caaf Closes gh-1752 Closes gh-2206 --- src/manipulation.js | 39 +++++++++++++++++++++------------------ test/unit/manipulation.js | 24 ++++++++++++++++++++---- 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/src/manipulation.js b/src/manipulation.js index 16a5a5208..160b9ec79 100644 --- 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 - // that element, do not do anything + // Skip elements already in the context collection (trac-4087) 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; + // Make the changes, replacing each non-ignored context element with the new content + return this.domManip( arguments, function( elem ) { + var parent = this.parentNode; - jQuery.cleanData( getAll( this ) ); - - if ( arg ) { - arg.replaceChild( elem, this ); + if ( jQuery.inArray( this, ignored ) < 0 ) { + jQuery.cleanData( getAll( this ) ); + if ( parent ) { + parent.replaceChild( elem, this ); + } } - }); - // Force removal if there was no new content (e.g., from empty arguments) - return arg && (arg.length || arg.nodeType) ? this : this.remove(); + // Force callback invocation + }, 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 index 5a41e9dad..1fd19a2c1 100644 --- a/test/unit/manipulation.js +++ b/test/unit/manipulation.js @@ -1279,15 +1279,20 @@ 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() { - var $el = jQuery( "
" ), +test( "Empty replaceWith (trac-13401; trac-13596; gh-2204)", function() { + + expect( 25 ); + + var $el = jQuery( "
" ).html( "

0

" ), + expectedHTML = $el.html(), tests = { "empty string": "", "empty array": [], + "array of empty string": [ "" ], "empty collection": jQuery( "#nonexistent" ), - // in case of jQuery(...).replaceWith(); - "empty undefined": undefined + // in case of jQuery(...).replaceWith(); + "undefined": undefined }; jQuery.each( tests, function( label, input ) { @@ -1295,6 +1300,17 @@ test( "Empty replaceWith (#13401; #13596)", 8, function() { strictEqual( $el.html(), "", "replaceWith(" + label + ")" ); $el.html( "" ).children().replaceWith(function() { return input; }); strictEqual( $el.html(), "", "replaceWith(function returning " + label + ")" ); + $el.html( "" ).children().replaceWith(function( i ) { i; return input; }); + strictEqual( $el.html(), "", "replaceWith(other function returning " + label + ")" ); + $el.html( "

" ).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 + ")" ); }); });