Skip to content

Commit

Permalink
Make sure that the previous element is removed from the page before t…
Browse files Browse the repository at this point in the history 
…he next is inserted, in replaceWith. Using a variation of the patch by snaury. Fixes #2697.
  • Loading branch information
@jeresig
jeresig committed Dec 5, 2009
1 parent aae0617 commit 2a6de9a
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
12 changes: 11 additions & 1 deletion src/manipulation.js
View file
Expand Up @@ -210,7 +210,17 @@ jQuery.fn.extend({

replaceWith: function( value ) {
if ( this[0] && this[0].parentNode ) {
return this.after( value ).remove();
return this.each(function(){
var next = this.nextSibling, parent = this.parentNode;

jQuery(this).remove();

if ( next ) {
jQuery(next).before( value );
} else {
jQuery(parent).append( value );
}
});
} else {
return this.pushStack( jQuery(jQuery.isFunction(value) ? value() : value), "replaceWith", value );
}
Expand Down
9 changes: 8 additions & 1 deletion test/unit/manipulation.js
View file
Expand Up @@ -466,7 +466,7 @@ test("insertAfter(String|Element|Array<Element>|jQuery)", function() {
});

var testReplaceWith = function(val) {
expect(12);
expect(14);
jQuery('#yahoo').replaceWith(val( '<b id="replace">buga</b>' ));
ok( jQuery("#replace")[0], 'Replace element with string' );
ok( !jQuery("#yahoo")[0], 'Verify that original element is gone, after string' );
Expand All @@ -491,6 +491,13 @@ var testReplaceWith = function(val) {
var set = jQuery("<div/>").replaceWith(val("<span>test</span>"));
equals( set[0].nodeName.toLowerCase(), "span", "Replace the disconnected node." );
equals( set.length, 1, "Replace the disconnected node." );

var $div = jQuery("<div class='replacewith'></div>").appendTo("body");
$div.replaceWith("<div class='replacewith'></div><script>" +
"equals(jQuery('.replacewith').length, 1, 'Check number of elements in page.');" +
"</script>");
equals(jQuery('.replacewith').length, 1, 'Check number of elements in page.');
jQuery('.replacewith').remove();
}

test("replaceWith(String|Element|Array&lt;Element&gt;|jQuery)", function() {
Expand Down

5 comments on commit 2a6de9a

@foxbunny
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just gone trough the bug report, and also the dev group's archives. Since there is no way I can post a comment to either, I'll do it here. This patch does not fix all the cases. Im my case, I do replaceWith once, and it works. The second time I do it, it just appends new content right after the old one. And on all subsequent occasions. For example, let's say I have a

with ID of 'foo'

<p id='foo'></p>

On first $("#foo").replaceWith('<p id='foo'></p>'); I get the same thing:

<p id='foo'></p>

Next time I run $("#foo").replaceWith('<p id='foo'></p>'); I have:

<p id='foo'></p>
<p id='foo'></p>

And so on.

@jeresig
Copy link
Member Author

@jeresig jeresig commented on 2a6de9a Oct 9, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to have been fixed: 51283d9

@foxbunny
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that commit released? I'm seeing the above error in currently released version of jQuery.

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@foxbunny: I'll need you to provide a better test case then (one that works within the context of the jQuery test suite - or one that runs on jsfiddle, or similar, because I'm unable to reproduce it.

@foxbunny
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okeydokey. As soon as I have some time, I'll take a look at the test case and see what I can do.

Please sign in to comment.