foo.contents().hasClass() returns incorrect value

[olov.lassus]

hasClass("undefined") returns true incorrectly for elements that do no have the className property (as opposed to having it set to ""), such as text and comment nodes.

Verified with Chrome 12 and Firefox 5.

Pull request for patch and test case to follow.

[olov.lassus]

Submitted pull request ​https://github.com/jquery/jquery/pull/418.

Reference to forum post ​http://forum.jquery.com/topic/i-made-jquery-restrict-mode-clean-and-i-think-i-found-a-tiny-bug-doing-so.

[Rick Waldron]

This is a non-issue.

​http://jsfiddle.net/rwaldron/D5QgV/

[olov.lassus]

As I demonstrated in the test case included in the pull request this is working as expected for DIV nodes (I included that expectation for completeness) but not for text and comment nodes.

The jsFiddle test you linked to just shows that this is a non-issue for DIV nodes, doesn't it?

[Rick Waldron]

Right, but the add|removeClass methods only care about nodeType 1 ( ​https://developer.mozilla.org/en/nodeType )

Updated: ​http://jsfiddle.net/rwaldron/D5QgV/

[olov.lassus]

That's fine and I see the nodeType guards in addClass/removeClass, but what about hasClass? Should one be able to rely on jQuery("div").contents().hasClass or not? The existing hasClass implementation contains no guards and is used in that manner in your existing unit tests.

​http://jsfiddle.net/olov/sMceh/ is reduced to make it clearer

[Rick Waldron]

​https://github.com/jquery/jquery/pull/419

[olov.lassus]

Do you consider the patch in my pull request adequate for fixing this issue?

edit: ​https://github.com/jquery/jquery/pull/419 is now updated with a correct test case and the bugfix so landing that will fix this ticket

[Dave Methvin]

Merge pull request #419 from rwldrn/9630

Unit tests assert that .contents().hasClass() works as expected. Fixes #9630

Changeset: 6a3395afcdb958a25f9a7cba3e544fe10d4d123a