Skip to content

Commit

Permalink
Fix #12411, .removeClass(undefined) is a chaining no-op. Close gh-913.
Browse files Browse the repository at this point in the history
.removeClass() //removes all classes, as documented
.removeClass(window.nonExistentVariable) // removes nothing
  • Loading branch information
matjae authored and dmethvin committed Oct 22, 2012
1 parent 23d125a commit 227c49a
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/attributes.js
Expand Up @@ -78,7 +78,7 @@ jQuery.fn.extend({
jQuery( this ).removeClass( value.call(this, j, this.className) );
});
}
if ( (value && typeof value === "string") || value === undefined ) {
if ( (value && typeof value === "string") || !arguments.length ) {
removes = ( value || "" ).split( core_rspace );

for ( i = 0, l = this.length; i < l; i++ ) {
Expand Down
9 changes: 9 additions & 0 deletions test/unit/attributes.js
Expand Up @@ -1202,6 +1202,15 @@ test( "removeClass() removes duplicates", function() {
ok( !$div.hasClass("x"), "Element with multiple same classes does not escape the wrath of removeClass()" );
});

test("removeClass(undefined) is a no-op", function() {
expect( 1 );

var $div = jQuery("<div class='base second'></div>");
$div.removeClass( undefined );

ok( $div.hasClass("base") && $div.hasClass("second"), "Element still has classes after removeClass(undefined)" );

This comment has been minimized.

Copy link
@Krinkle

Krinkle Oct 25, 2012

Member

Maybe additionally assert that the return value is strictEqual to $div? That way it doesn't just assert it being a no-op, but it also actually being chainable.

edit: Done in pull #1008.

});

var testToggleClass = function(valueObj) {
expect( 17 );

Expand Down

0 comments on commit 227c49a

Please sign in to comment.