Skip to main content

Bug Tracker

Side navigation

#11539 closed bug (fixed)

Opened March 30, 2012 05:16PM UTC

Closed June 26, 2012 08:40PM UTC

All version of jQuery don't support .has() on $([text Element].parentNode).has?(other_element)

Reported by: theprecognition@gmail.com Owned by: gibson042
Priority: low Milestone: 1.8
Component: traversing Version: 1.7.1
Keywords: Cc:
Blocked by: Blocking:
Description

http://jsfiddle.net/4hV6c/4/

works in ff, chrome, safari, opera, but not IE8.

stackoverflow discussion: http://stackoverflow.com/questions/9947182/jquery-doesnt-support-has-in-ie8-what-is-a-work-around#comment12701455_9947182

Currently working on a solution. =\\

Attachments (0)
Change History (17)

Changed March 30, 2012 05:29PM UTC by mendesjuan@gmail.com comment:1

The jsfiddle above contains some noise, it is not a complete reduction of the problem. See the following jsFiddles for smaller examples of the problem

Using jQuery (and Sizzle) http://jsfiddle.net/mendesjuan/4hV6c/8/

Just with Node.contains http://jsfiddle.net/4hV6c/10/

The problem is really in Sizzle.contains which uses Node.contains, but that is broken in IE when you pass in a text node

Changed March 30, 2012 05:56PM UTC by mendesjuan@gmail.com comment:2

I think a better description of the problem is that

$.has

doesn't support passing in a text node

Changed March 30, 2012 08:14PM UTC by theprecognition@gmail.com comment:3

Here is a work around for my specific issue in IE, the conditions for the other ways to use .has() will have to be added. http://jsfiddle.net/8F57r/12/

Changed March 30, 2012 08:33PM UTC by theprecognition@gmail.com comment:4

Used jquery to handle the .parent() http://jsfiddle.net/8F57r/13/

Changed April 02, 2012 03:44PM UTC by gibson042 comment:5

I looked at this a little over the weekend, but couldn't find a solution that added less than 14 bytes gzipped (and really, the most performant added 16). Others are welcome to take a swing, but I feel like that's probably it.

https://github.com/gibson042/sizzle/commit/439b16792ef089a248059bed7fe89ce2247ca621

Changed April 02, 2012 03:54PM UTC by dmethvin comment:6

This feels a lot like #11540 to me. Text nodes are not generally valid inputs for jQuery collections and/or methods. If we start to make exceptions and say, "Oh but for this API and that one we'll allow it" then everyone will (somewhat rightly) expect that all our APIs fully support text nodes. That would be a lot of work/size for something that hasn't caused a lot of issues over 5 years.

Changed April 02, 2012 04:02PM UTC by gibson042 comment:7

This might be a minority opinion, but I feel like we ''should'' generally support text nodes (except where doing so is prohibitively heavy or slow, in which case those reasons deserve documentation).

Changed April 04, 2012 12:56AM UTC by dmethvin comment:8

#11538 is a duplicate of this ticket.

Changed April 09, 2012 11:10AM UTC by addyosmani comment:9

My opinion is that text nodes shouldn't be supported for just one API when we're not going to be implementing this for others (at least not without some significant weight being added). I'd vote for a wontfix.

Changed April 09, 2012 01:31PM UTC by theprecognition@gmail.com comment:10

I never meant for this to be a discussion about textNodes

http://jsfiddle.net/8F57r/13/

proves (in IE8) that the parent of a textNode (i.e.: an actual node) has a broken .has()

Changed April 09, 2012 02:19PM UTC by gibson042 comment:11

Replying to [comment:10 theprecognition@…]:

I never meant for this to be a discussion about textNodes http://jsfiddle.net/8F57r/13/ proves (in IE8) that the parent of a textNode (i.e.: an actual node) has a broken .has()

The parent has no relevance here, it's your text node argument to .has that ultimately gets passed to Node.contains via Sizzle.contains. I've updated your fiddle to demonstrate: http://jsfiddle.net/8F57r/14/. See?

Changed April 09, 2012 02:28PM UTC by theprecognition@gmail.com comment:12

ah. But semantically, can you seer where I'm coming from? this functionality feels like it should work across all browsers. =\\

Changed April 09, 2012 03:46PM UTC by gibson042 comment:13

Replying to [comment:9 addyosmani]:

My opinion is that text nodes shouldn't be supported for just one API when we're not going to be implementing this for others (at least not without some significant weight being added). I'd vote for a wontfix.

In this case the relevant code defers directly to Sizzle, which tends to be much less hostile towards non-elements (e.g., compare $("<p>A<br/>B</p>").contents().filter(":not(*)") to $("<p>A<br/>B</p>").contents(":not(*)") or $.filter( ":not(*)", $("<p>A<br/>B</p>").contents() )).

Changed May 11, 2012 05:45PM UTC by dmethvin comment:14

component: unfiledtraversing
milestone: None1.8
priority: undecidedlow
status: newopen

I can't envision us opening up Pandora's box and treating text nodes as first-class citizens in jQuery. Given that much of our semantics are based on CSS selectors which treat text nodes as a lower caste if not an outright untouchable, I don't think we're making up crazy rules here.

For .has() I can see how it might be expected to accept a text node as its parameter. I'll mark this open for now but don't want it to become a rallying cry for "you accepted text nodes there, so why not here" arguments. In that case it would be better to close this wontfix.

Changed June 11, 2012 04:36PM UTC by gibson042 comment:15

owner: → gibson042
status: openassigned

Changed June 12, 2012 03:14AM UTC by gibson042 comment:16

Changed June 26, 2012 08:40PM UTC by timmywil comment:17

resolution: → fixed
status: assignedclosed

Allow document to be passed to Sizzle.contains. Fixes #11539.

Changeset: eecd8aef671d6a8720b4f66ee22b3d01ef1b951b