Bug Tracker

Ticket #11539 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

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

Reported by: theprecognition@… Owned by: gibson042
Priority: low Milestone: 1.8
Component: traversing Version: 1.7.1
Keywords: Cc:
Blocking: Blocked by:

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. =\

Change History

comment:1 Changed 3 years ago by mendesjuan@…

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

comment:2 Changed 3 years ago by mendesjuan@…

I think a better description of the problem is that

$.has

doesn't support passing in a text node

comment:3 Changed 3 years ago by theprecognition@…

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/

comment:4 Changed 3 years ago by theprecognition@…

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

comment:5 Changed 3 years ago by gibson042

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

comment:6 Changed 3 years ago by dmethvin

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.

comment:7 Changed 3 years ago by gibson042

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).

comment:8 Changed 3 years ago by dmethvin

#11538 is a duplicate of this ticket.

comment:9 follow-up: ↓ 13 Changed 3 years ago by 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.

comment:10 follow-up: ↓ 11 Changed 3 years ago by 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()

comment:11 in reply to: ↑ 10 Changed 3 years ago by gibson042

Replying to 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?

comment:12 Changed 3 years ago by theprecognition@…

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

comment:13 in reply to: ↑ 9 Changed 3 years ago by gibson042

Replying to 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() )).

comment:14 Changed 2 years ago by dmethvin

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to traversing
  • Milestone changed from None to 1.8

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.

comment:15 Changed 2 years ago by gibson042

  • Owner set to gibson042
  • Status changed from open to assigned

comment:17 Changed 2 years ago by timmywil

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

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

Changeset: eecd8aef671d6a8720b4f66ee22b3d01ef1b951b

Note: See TracTickets for help on using tickets.