Bug 148582 – classList.toggle(name, force) treats undefined `force` argument as false

• Component: DOM
• Status: RESOLVED FIXED

[Michał Gołębiowski-Owczarek]

Test case:

var div = document.createElement('div');
div.classList.toggle('a', undefined); // <div></div>

`toggle` should add the class and not treat `undefined` as false.

Chrome & Edge also share this bug. Firefox implements it correctly.

Chrome bug report: https://code.google.com/p/chromium/issues/detail?id=489665

[Michał Gołębiowski-Owczarek]

Edge bug report: https://connect.microsoft.com/IE/feedbackdetail/view/1725606/

[Chris Dumez]

Yes, this is not specific to this API but a general bug in our JS bindings code. undefined gets converted to 0 / false instead of being treated as no argument when the argument is optional. I will try and address this in the near future.

[Michał Gołębiowski-Owczarek]

(In reply to comment #2)
> I will try and address this in the near future.

Great to hear that!

This is very useful when you need to proxy the method. I'm developing an experimental jQuery plugin that replaces its class manipulation code with one utilizing classList and I had to write something like:

if (force !== undefined) {
    this.classList.toggle(clazz, force);
} else {
    this.classList.toggle(clazz);
}

instead of just passing the value. This is the full source:
https://github.com/mzgol/jquery.classList/blob/3a7018254bf0545c54f6ff7d73eb21aadcfb8ec9/src/jquery.class_list.js#L108-L119

[Chris Dumez]

rdar://problem/22545600

[Chris Dumez]

Created attachment 261474 [details]
Patch

[Chris Dumez]

Created attachment 261490 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 261490 [details]
Patch

Does this match the behavior of other browsers?

[Chris Dumez]

(In reply to comment #7)
> Comment on attachment 261490 [details]
> Patch
> 
> Does this match the behavior of other browsers?

It matches Firefox and the specification.

[Chris Dumez]

Created attachment 261503 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 261503 [details]
Patch

Clearing flags on attachment: 261503

Committed r189969: <http://trac.webkit.org/changeset/189969>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michał Gołębiowski-Owczarek]

Thanks! \o/

Do you also plan to fix the more general issue about WebIDL bindings mistreating undefined?

[Chris Dumez]

(In reply to comment #12)
> Thanks! \o/
> 
> Do you also plan to fix the more general issue about WebIDL bindings
> mistreating undefined?

Yes, I am fixing a lot of those via:
https://bugs.webkit.org/show_bug.cgi?id=149263
https://bugs.webkit.org/show_bug.cgi?id=149331

If you know about more cases, please let me know.