Bug Tracker

Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#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:


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:2 Changed 12 years ago by Rick Waldron

Component: unfiledattributes
Priority: undecidedlow
Resolution: worksforme
Status: newclosed

comment:3 Changed 12 years ago by 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?

comment:4 Changed 12 years ago by 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/

comment:5 Changed 12 years ago by 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

comment:6 Changed 12 years ago by Rick Waldron

Resolution: worksforme
Status: closedreopened
Summary: hasClass("undefined") returns incorrect value for text and comment nodesfoo.contents().hasClass() returns incorrect value

comment:7 Changed 12 years ago by Rick Waldron

Owner: set to Rick Waldron
Status: reopenedassigned

comment:9 Changed 12 years ago by 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

Last edited 12 years ago by olov.lassus (previous) (diff)

comment:10 Changed 12 years ago by ajpiano

Milestone: 1.next1.7

comment:11 Changed 12 years ago by Dave Methvin

Resolution: fixed
Status: assignedclosed

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 dmethvin

Note: See TracTickets for help on using tickets.