[Bug]: Multiple bugs in V (className handling)

This issue has been tracked since 2022-11-20.

What happened?

I stumbled across a very weird bug and after a few hours of debugging I found out that there are multiple issues with the way V is handling class names.

hasClass, addClass, removeClass and toggleClass aren't trimming the className argument, which can lead to edge cases in which the methods won't work as expected. For example, adding a class foo (note the white space at the beginning) and then trying to remove foo won't work.

Demo: https://codesandbox.io/s/addclass-add-bug-g4jvbv?file=/src/index.js

Repro steps:

  1. Move the mouse over the link. The link should animate it's dasharray property
  2. Move the mouse out. The link should switch back to it's original form, but it won't.

Fix:

Remove the white space from the beginning of the class name at line 73 and repeat the steps.

After having a look at the other methods of V, I'd say removeAttr, attr and findParentByClass are a possible candidates for the same bug.

Proposed solution:

I think a good way to deal with this bug is trimming the className argument.

Version

3.6.2

What browsers are you seeing the problem on?

Chrome

What operating system are you seeing the problem on?

Mac

kumilingus wrote this answer on 2022-12-03

Hi, would this work for you?

alexandernst wrote this answer on 2022-12-03

Yes, that will fix my particular issue, but imho hasClass should also be able to handle a single class name with whitespaces at the beginning / end.

Bonus points: Maybe even it could handle multiple classes, in which case it could return true if all the classes are present in the element.

kumilingus wrote this answer on 2022-12-03

Yes, that will fix my particular issue, but imho hasClass should also be able to handle a single class name with whitespaces at the beginning / end.

It's a good point about the leading and trailing whitespaces. I have updated the PR.

Bonus points: Maybe even it could handle multiple classes, in which case it could return true if all the classes are present in the element.

I thought about it and decided not to do that, as it would not be clear whether it means contains ALL or contains ANY.
As it is now, It's also aligned with the tokenlist specification.

alexandernst wrote this answer on 2022-12-03

I have updated the PR.

Awesome \o/

I thought about it and decided not to do that, as it would not be clear whether it means contains ALL or contains ANY.

Fair enough! :)

Thank you for the hard work!!

More Details About Repo
Owner Name clientIO
Repo Name joint
Full Name clientIO/joint
Language JavaScript
Created Date 2009-09-11
Updated Date 2022-12-06
Star Count 3717
Watcher Count 155
Fork Count 817
Issue Count 53

YOU MAY BE INTERESTED

Issue Title Created Date Updated Date