addClass() called with an array doesn't add all of the classes if one of them is an empty string

[MatmaRex]

Description

When addClass() is called with an array, and one of the classes in the array is an empty string, it doesn't add all of the classes (it stops processing them after the empty string).

Minimal test case:

$( '<div>' ).addClass( [ 'a', '', 'b' ] ).attr( 'class' )

Expected output: a b
Actual output: a

Link to test case

https://jsbin.com/voxetucoho/edit?js,console

[mgol]

Thanks for the report. This is caused by this line:

jquery/src/attributes/classes.js

Line 41 in e24f2dc

while ( ( clazz = classes[ j++ ] ) ) {

I wonder if there's any concrete reason why we're not checking for length here. I know there have been some issues related to this property in old IE but perhaps not when we're dealing with a regular array.

[mgol]

Note, though, that an empty string is not a valid class name and if you pass it to the native classList.add, it will throw an error. Array input to addClass requires valid classes to be present in the input so the proposed input is invalid.

That said, the current behavior may still looks surprising so I'd be open to changing this.

[timmywil]

I'd be open to changing this as well. It makes sense to apply as many valid classes as possible, and I could see a case where a variable is passed that happens to be empty string.

[MatmaRex]

Note, though, that an empty string is not a valid class name and if you pass it to the native classList.add, it will throw an error. Array input to addClass requires valid classes to be present in the input so the proposed input is invalid.

It is not valid for sure, but jQuery is happy to ignore other invalid class names, such as ' ' (a space).

$( '<div>' ).addClass( [ ' ' ] ).attr( 'class' )

Output: undefined

It is also happy to clean up and tweak them to make them valid:

$( '<div>' ).addClass( [ 'a b', ' c ' ] ).attr( 'class' )

Output: a b c

If you'd like these to throw an exception, that'd be fine by me, but I'd expect all invalid class names to behave consistently.

I'd be open to changing this as well. It makes sense to apply as many valid classes as possible, and I could see a case where a variable is passed that happens to be empty string.

Yes, when I discovered this bug, the empty string was actually generated by a library very far away from the place where it was added as a class.

[mgol]

If you'd like these to throw an exception, that'd be fine by me, but I'd expect all invalid class names to behave consistently.

I think we're all leaning to making it work for an empty string as well. Would you like to submit a PR with a fix?

[MatmaRex]

I'd love to, but I find it uncomfortable to contribute to projects that require me to sign a CLA first.

(Also, your links to the CLA do not work, e.g. in CONTRIBUTING.md it links to https://contribute.jquery.org/cla/ which just redirects to https://openjsf.org/.)

[mgol]

Thanks for the info, we'll update the links. More information is available at https://openjsf.org/about/the-openjs-foundation-cla/.

I understand the CLA reservations but, unfortunately, we are not allowed to merge code from contributors that haven't signed it. If you don't want to do that, we'll fix it ourselves.

[mgol]

PR: #5003.