Skip to content
Permalink
Browse files
Fix #12120. Always stack .before/.after, and fix disconnected nodes.
  • Loading branch information
@dmethvin
dmethvin committed Aug 24, 2012
1 parent 2263134 commit e2eac3f4d2e7f47b67635704df8e3b6675a91ed6
Showing 2 changed files with 9 additions and 13 deletions.
11 src/manipulation.js
View file
@@ -148,10 +148,7 @@ jQuery.fn.extend({
});
}

if ( arguments.length ) {
var set = jQuery.clean( arguments );
return this.pushStack( jQuery.merge( set, this ), "before", this.selector );
}
return this.pushStack( jQuery.merge( jQuery.clean( arguments ), this ), "before", this.selector );
},

after: function() {
@@ -161,10 +158,7 @@ jQuery.fn.extend({
});
}

if ( arguments.length ) {
var set = jQuery.clean( arguments );
return this.pushStack( jQuery.merge( this, set ), "after", this.selector );
}
return this.pushStack( jQuery.merge( this.toArray(), jQuery.clean( arguments ) ), "after", this.selector );
},

// keepData is for internal use only--do not document
@@ -721,6 +715,7 @@ jQuery.extend({

// Fix #11356: Clear elements from safeFragment
if ( div ) {
div.innerHTML = "";
safe.removeChild( div );
elem = div = safe = null;
}
11 test/unit/manipulation.js
View file
@@ -909,12 +909,13 @@ test("before(Function)", function() {
testBefore(manipulationFunctionReturningObj);
});

test("before and after w/ empty object (#10812)", function() {
expect(2);
test("before and after w/ empty object (#10812, #12120)", function() {
expect(3);

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" );
var res = jQuery("#notInTheDocument").before("<span>(</span>").after("<span>)</span>");
equal( res.length, 2, "added two elements to the empty object" );
equal( res.text(), "()", "correctly appended text" );
equal( res.end().text(), "(", "stacked the previous value" );
});

test("before and after on disconnected node (#10517)", function() {

3 comments on commit e2eac3f

@lafriks
Copy link

@lafriks lafriks commented on e2eac3f Aug 24, 2012

Choose a reason for hiding this comment

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

This seems to brake something in IE9. After this change it now throws error "Invalid argument" when using prepend in some cases like space before first tag in html string.
test case: http://jsfiddle.net/xhZJz/7/

@pimvdb
Copy link

@pimvdb pimvdb commented on e2eac3f Aug 24, 2012

Choose a reason for hiding this comment

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

The original fiddle is still reproducible: http://jsfiddle.net/3qqXG/.

The issue is not arguments.length, but !isDisconnected( this[0] ) that makes the if fail in case of an empty set. So a stack is only added in case of an empty set, which makes .end unusable.

@dmethvin
Copy link
Member Author

@dmethvin dmethvin commented on e2eac3f Aug 24, 2012

Choose a reason for hiding this comment

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

Yes, IE of all vintages doesn't like this patch. It seemed to work locally but it was a caching problem. So there's more work to do.

@pimvdb the problem was/is that the elements created by string arguments like "<p>hello</p>" are falsely claimed to be connected, because their parentNode points to the div we used to create them. Yes, the issue in your test case wasn't arguments.length, but we still need to stack for that case if we want to be consistent. And I'll add another test case.

Please sign in to comment.