Bug Tracker

Ticket #9630 (closed bug: fixed)

Opened 3 years ago

Last modified 3 years ago

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

Reported by: olov.lassus Owned by: rwaldron
Priority: low Milestone: 1.6.3
Component: attributes Version: 1.6.1
Keywords: Cc:
Blocking: Blocked by:

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

comment:2 Changed 3 years ago by rwaldron

  • Priority changed from undecided to low
  • Resolution set to worksforme
  • Status changed from new to closed
  • Component changed from unfiled to attributes

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

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 3 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 3 years ago by rwaldron

  • Status changed from closed to reopened
  • Resolution worksforme deleted
  • Summary changed from hasClass("undefined") returns incorrect value for text and comment nodes to foo.contents().hasClass() returns incorrect value

comment:7 Changed 3 years ago by rwaldron

  • Owner set to rwaldron
  • Status changed from reopened to assigned

comment:9 Changed 3 years ago by olov.lassus

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

Version 0, edited 3 years ago by olov.lassus (next)

comment:10 Changed 3 years ago by ajpiano

  • Milestone changed from 1.next to 1.7

comment:11 Changed 3 years ago by Dave Methvin

  • Status changed from assigned to closed
  • Resolution set to fixed

Merge pull request #419 from rwldrn/9630

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

Changeset: 6a3395afcdb958a25f9a7cba3e544fe10d4d123a

comment:12 Changed 3 years ago by dmethvin

  • Milestone changed from 1.7 to 1.6.3
Note: See TracTickets for help on using tickets.