Bug Tracker

Opened 6 years ago

Closed 6 years ago

Last modified 6 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 6 years ago by Rick Waldron

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

comment:3 Changed 6 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 6 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 6 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 6 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 6 years ago by Rick Waldron

Owner: set to Rick Waldron
Status: reopenedassigned

comment:9 Changed 6 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 6 years ago by olov.lassus (previous) (diff)

comment:10 Changed 6 years ago by ajpiano

Milestone: 1.next1.7

comment:11 Changed 6 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 6 years ago by dmethvin

Note: See TracTickets for help on using tickets.