-
Notifications
You must be signed in to change notification settings - Fork 647
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[selectors] The forgiving nature of :has breaks jQuery when used with a complex :has selector #7676
Comments
It's worth noting this affects all jQuery versions from 1.3 released in January, 2009 to the current 3.6.1 released a few days ago. |
This did not break with Safari shipping, because WebKit did not implement using forgiving selector list per spec: https://bugs.webkit.org/show_bug.cgi?id=244708 |
Even if we make the |
The current WebKit behavior (not by design) is to use forgiving parsing but fail the selector if the selector list ends up empty. That is
is valid and parses to
but
Perhaps this is actually the best option considering the jQuery issue. |
Shouldn't jQuery check |
I agree that this is the correct way to validate Now I'm trying to merge a fix to Chrome to avoid existing JQuery conflict by following the WebKit's behavior. But I think that we need a way to handle this situation properly. The one way I can imagine is to rename the pseudo class name in the spec so that there will not be any conflict with existing JQuery implementation ( |
Is there any real-world site broken by the change? How frequent do we expect it to be to use jQuery-specific selectors inside |
Also, is this a "recent version of jQuery" kinda thing? Or would this change cause issues on older jQuery versions? |
If there's no real-world breakage, it might be worth just getting jQuery fixed by using |
In Chrome 105,
As I wrote in #7676 (comment), this affects virtually all jQuery versions used in the wild.
I am unable to answer this question. But it did both break our test suite and get us a report from a user at jquery/jquery#5098. @Rinzwind did you encounter this issue just in manual testing or did it actually break anything for you on a live site? |
Note that the WebKit behavior still breaks some jQuery selectors. True, it doesn't break:
but it still breaks:
Maybe that latter kind of selectors is not used as often in the wild. |
Workaround to avoid JQuery :has() issue: - w3c/csswg-drafts#7676 This CL follows the current WebKit behavior: Parse :has() as forgiving, but make it invalid when all the arguments are dropped. - csswg-drafts/issues/7676#issuecomment-1235450787 Bug: 1358953 Change-Id: Ib6bf08a6c3320585eb80e1761e852e530c369bd2 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3868781 Commit-Queue: Byungwoo Lee <blee@igalia.com> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/main@{#1042628}
Opened issue for CSS.supports()/@supports: https://bugs.chromium.org/p/chromium/issues/detail?id=1359396 |
There is a real-world breakage reported here: https://crbug.com/1359246 |
In our case, none of the selectors used in the application itself seem to be affected by this problem, only the ones in our automated tests (see: jquery/jquery#5098 (comment)). |
Importantly, this is not specific to So our options are:
All three options suck! Unless the breakage gets fairly bad, tho, I do lean toward 3. The |
BTW, re:
What use cases exist that make it useful to have The fact that in WebKit |
Because the jQuery selector engine currently works via a binary switch - first, check qSA; if it throws, use the jQuery custom traversal - and because jQuery never implemented Does Google have any way to collect metrics of how often |
@mgol We could query HTTP Archive. What are we looking for?
|
Any jQuery-specific pseudo inside of
There are other native ones also implemented by jQuery but those are probably fine? EDIT: Updated based on the comment from @SelenIT |
@mgol isn't |
@tabatkins What about the following? 2b. Don't give up on forgiving-selector-list entirely, but spec the current Safari behavior. I.e. make :has() invalid if all the arguments are invalid. ... provided that it avoids enough problems to be worth it. (As I understand it doesn't fully solve the issue). |
That seems rather unfortunate fwiw, I'd much rather make them behave consistently regardless of the number of valid arguments... If we had something like that I think I'd prefer making it unforgiving iff the number of provided arguments is exactly one, rather than making it depend on the number of valid arguments. But still unfortunate IMHO |
So making:
invalid instead of never matching wouldn't be too bad, but the consequence of dropping other fully valid selectors from a list, which would happen with
is more problematic. |
Resolved in <w3c/csswg-drafts#7676>
Automatic update from web-platform-tests Make :has() unforgiving Apply the issue resolution of making :has() unforgiving: w3c/csswg-drafts#7676 (comment) Bug: 1399744 Change-Id: Ibb499e251ecce7ba22bd454ea94b2c8c8b1d8afb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4090967 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Byungwoo Lee <blee@igalia.com> Cr-Commit-Position: refs/heads/main@{#1081452} -- wpt-commits: 37e131cadfd27444fa4223b826edbd6bed29c356 wpt-pr: 37423
Apply the issue resolution of making :has() unforgiving: w3c/csswg-drafts#7676 (comment) Bug: 1399744 Change-Id: Ibb499e251ecce7ba22bd454ea94b2c8c8b1d8afb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4090967 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Byungwoo Lee <blee@igalia.com> Cr-Commit-Position: refs/heads/main@{#1081452}
jQuery has followed the following logic for selector handling for ages: 1. Modify the selector to adhere to scoping rules jQuery mandates. 2. Try `qSA` on the modified selector. If it succeeds, use the results. 3. If `qSA` threw an error, run the jQuery custom traversal instead. It worked fine so far but now CSS has a concept of forgiving selector lists that some selectors like `:is()` & `:has()` use. That means providing unrecognized selectors as parameters to `:is()` & `:has()` no longer throws an error, it will just return no results. That made browsers with native `:has()` support break selectors using jQuery extensions inside, e.g. `:has(:contains("Item"))`. Detecting support for selectors can also be done via: ```js CSS.supports( "selector(SELECTOR_TO_BE_TESTED)" ) ``` which returns a boolean. There was a recent spec change requiring this API to always use non-forgiving parsing: w3c/csswg-drafts#7280 (comment) However, no browsers have implemented this change so far. To solve this, two changes are being made: 1. In browsers supports the new spec change to `CSS.supports( "selector()" )`, use it before trying `qSA`. 2. Otherwise, add `:has` to the buggy selectors list. Ref jquerygh-5098 Ref jquerygh-5107 Ref jquery/sizzle#486 Ref w3c/csswg-drafts#7676
Automatic update from web-platform-tests Make :has() unforgiving Apply the issue resolution of making :has() unforgiving: w3c/csswg-drafts#7676 (comment) Bug: 1399744 Change-Id: Ibb499e251ecce7ba22bd454ea94b2c8c8b1d8afb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4090967 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Byungwoo Lee <blee@igalia.com> Cr-Commit-Position: refs/heads/main@{#1081452} -- wpt-commits: 37e131cadfd27444fa4223b826edbd6bed29c356 wpt-pr: 37423
https://bugs.webkit.org/show_bug.cgi?id=249914 rdar://103733208 Reviewed by Antti Koivisto. Following CSSWG resolution: w3c/csswg-drafts#7676 (comment) In order to unbreak jQuery. * LayoutTests/imported/w3c/web-platform-tests/css/selectors/parsing/parse-has-expected.txt: * Source/WebCore/css/parser/CSSSelectorParser.cpp: (WebCore::CSSSelectorParser::consumeRelativeSelectorList): (WebCore::CSSSelectorParser::consumePseudo): (WebCore::CSSSelectorParser::consumeForgivingRelativeSelectorList): Deleted. * Source/WebCore/css/parser/CSSSelectorParser.h: Canonical link: https://commits.webkit.org/258712@main
…st-child() unforgiving. w3c#7676
…ngParsing Enable these two features by default. CSSAtSupportsAlwaysNonForgivingParsing: - Apply the csswg issue resolution: change '@supports' selector parsing. - w3c/csswg-drafts#7280 (comment) CSSPseudoHasNonForgivingParsing: - Apply the csswg issue resolution: make ':has()' non-forgiving. - w3c/csswg-drafts#7676 (comment) Intent-to-ship: https://groups.google.com/u/1/a/chromium.org/g/blink-dev/c/RJrIxJA9LYw Bug: 1359396, 1399744 Change-Id: I179f22074f5cf6c9c8d56e36a4e3360bfe892256 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4224863 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Byungwoo Lee <blee@igalia.com> Cr-Commit-Position: refs/heads/main@{#1102139}
I have some doubts about the generic recommendation of wrapping with |
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes jquerygh-5194 Ref jquerygh-5098 Ref jquerygh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes jquerygh-5194 Ref jquerygh-5098 Ref jquerygh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes jquerygh-5194 Ref jquerygh-5098 Ref jquerygh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. Fixes jquery/jquery#5194 Ref jquery/jquery#5098 Ref jquerygh-486 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes jquerygh-5194 Ref jquerygh-5098 Ref jquerygh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes jquerygh-5194 Ref jquerygh-5098 Ref jquerygh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. Fixes jquery/jquery#5194 Ref jquery/jquery#5098 Ref jquerygh-486 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes gh-5194 Closes gh-5206 Ref gh-5098 Ref gh-5107 Ref w3c/csswg-drafts#7676 Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. The PR also updates `playwright-webkit` so that we test against a version of WebKit that already has non-forgiving `:has()`. Fixes gh-5194 Closes gh-5207 Ref gh-5206 Ref gh-5098 Ref gh-5107 Ref w3c/csswg-drafts#7676
`CSS.supports( "selector(...)" )` has different semantics than selectors passed to `querySelectorAll`. Apart from the fact that the former returns `false` for unrecognized selectors and the latter throws, `qSA` is more forgiving and accepts some invalid selectors, auto-correcting them where needed - for example, mismatched brackers are auto-closed. This behavior difference is breaking for many users. To add to that, a recent CSSWG resolution made `:is()` & `:where()` the only pseudos with forgiving parsing; browsers are in the process of making `:has()` parsing unforgiving. Taking all that into account, we go back to our previous try-catch approach without relying on `CSS.supports( "selector(...)" )`. The only difference is we detect forgiving parsing in `:has()` and mark the selector as buggy. Fixes jquery/jquery#5194 Closes gh-493 Ref jquery/jquery#5098 Ref jquery/jquery#5206 Ref jquery/jquery#5207 Ref gh-486 Ref w3c/csswg-drafts#7676
The fact that the native
:has
pseudo-class:https://drafts.csswg.org/selectors/#relational
takes
forgiving-relative-selector-list
as an argument:https://drafts.csswg.org/selectors/#forgiving-selector
means the contents are not validated.
jQuery has supported the
:has
pseudo-class for ages. However, its support is more powerful; in particular, it allows for jQuery extensions like:contains
to appear within:has
.The way the jQuery selector engine works (and have worked for a long time), the selector is tried against
querySelectorAll
with minor modifications and if it throws, it goes through the internal selector engine. It's either-or.Since selectors like
ul:has(li:contains('Item'))
no longer throw in Chrome, jQuery defers toquerySelectorAll
and such a selector returns 0 results even if the jQuery path should match something.We can of course try to patch it in jQuery, perhaps defaulting selectors containing
:has
to use the jQuery selector engine. But it won't help countless apps using older jQuery versions. So I wanted to bring it for consideration for the standards committee. I understand if the decision is going to be "no changes in the spec are planned" but it'd be good to have this discussion and the decision made knowing the consequences.As you can see, it didn't take long after the Chrome 105 release for us to get a bug report about this breakage:
jquery/jquery#5098
Implementing
:has
according to the spec makes the browser break the jQuery test suite right now.The text was updated successfully, but these errors were encountered: