#9630 closed bug (fixed)
foo.contents().hasClass() returns incorrect value
Reported by: | olov.lassus | Owned by: | Rick Waldron |
---|---|---|---|
Priority: | low | Milestone: | 1.6.3 |
Component: | attributes | Version: | 1.6.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
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.
Change History (12)
comment:1 Changed 12 years ago by
comment:2 Changed 12 years ago by
Component: | unfiled → attributes |
---|---|
Priority: | undecided → low |
Resolution: | → worksforme |
Status: | new → closed |
This is a non-issue.
comment:3 Changed 12 years ago by
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?
comment:4 Changed 12 years ago by
Right, but the add|removeClass methods only care about nodeType 1 ( https://developer.mozilla.org/en/nodeType )
Updated: http://jsfiddle.net/rwaldron/D5QgV/
comment:5 Changed 12 years ago by
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
comment:6 Changed 12 years ago by
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
Summary: | hasClass("undefined") returns incorrect value for text and comment nodes → foo.contents().hasClass() returns incorrect value |
comment:7 Changed 12 years ago by
Owner: | set to Rick Waldron |
---|---|
Status: | reopened → assigned |
comment:9 Changed 12 years ago by
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
comment:10 Changed 12 years ago by
Milestone: | 1.next → 1.7 |
---|
comment:11 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Merge pull request #419 from rwldrn/9630
Unit tests assert that .contents().hasClass() works as expected. Fixes #9630
Changeset: 6a3395afcdb958a25f9a7cba3e544fe10d4d123a
comment:12 Changed 11 years ago by
Milestone: | 1.7 → 1.6.3 |
---|
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.