Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOMTokenList contains implementation seems wrong #36

Open
sandersky opened this issue Aug 26, 2014 · 4 comments
Open

DOMTokenList contains implementation seems wrong #36

sandersky opened this issue Aug 26, 2014 · 4 comments

Comments

@sandersky
Copy link

I believe your contains method for DOMTokenList in classList.js is incorrect. It is checking the Element's className instead of the array of classes. In the event you check if a DOMTokenList contains a string which is a substring of a class, it would return true when I believe it should return false. Instead it should be something like:

  contains: function(token) {
    return prototype.indexOf.call(this, token) != -1;
  },
@jerone
Copy link

jerone commented Aug 26, 2014

Apparently this was previous the way it worked before removing it after having problems with IE8's fireEvent: 898b546
Array.prototype.indexOf() is only supported since IE 9.

@sandersky
Copy link
Author

Well I still think the current implementation is wrong. If I have an element with the class items and check that the element contains the class item it will return true because item is a substring of items while the class item doesn't actually exist on the element.

@jerone
Copy link

jerone commented Aug 26, 2014

@sandersky commented on 26 aug. 2014 21:09 CEST:

Well I still think the current implementation is wrong. If I have an element with the class items and check that the element contains the class item it will return true because item is a substring of items while the class item doesn't actually exist on the element.

Seems a valid bug, you're right.

Wish above commit had a bug report attached to it; searching the Internet didn't find any similar issues with Array.prototype.indexOf() and IE's fireEvent.

Edit:
Found the corresponding PR, but no real explanation.

@sandersky
Copy link
Author

For me I only care about IE9 and above since IE8 is no longer supported by Microsoft (since April when they stopped XP support). With this being said I opted for the indexOf solution in my project. If I find time I'll submit an IE8 friendly fix to the issue I addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants