From 8b5ef0e07df2ff128fe2f82e478fe01077bbac5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Thu, 9 Feb 2023 15:16:19 +0100 Subject: [PATCH 1/3] Selector: Stop relying on CSS.supports( "selector(...)" ) `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 Ref gh-5098 Ref gh-5107 Ref w3c/csswg-drafts#7676 --- package.json | 2 +- src/selector.js | 27 --------------------------- src/selector/rbuggyQSA.js | 15 +++++++-------- src/selector/support.js | 33 +++++++++++++-------------------- test/unit/support.js | 16 ++++++++-------- 5 files changed, 29 insertions(+), 64 deletions(-) diff --git a/package.json b/package.json index 153eb8d6a7..31166d31f1 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "load-grunt-tasks": "5.1.0", "multiparty": "4.2.3", "native-promise-only": "0.8.1", - "playwright-webkit": "1.29.2", + "playwright-webkit": "1.30.0", "promises-aplus-tests": "2.1.2", "q": "1.5.1", "qunit": "2.10.1", diff --git a/src/selector.js b/src/selector.js index 328eca45ff..c995a65fea 100644 --- a/src/selector.js +++ b/src/selector.js @@ -22,7 +22,6 @@ import selectorError from "./selector/selectorError.js"; import unescapeSelector from "./selector/unescapeSelector.js"; import tokenize from "./selector/tokenize.js"; import toSelector from "./selector/toSelector.js"; -import support from "./selector/support.js"; // The following utils are attached directly to the jQuery object. import "./selector/escapeSelector.js"; @@ -189,32 +188,6 @@ function find( selector, context, results, seed ) { } try { - - // `qSA` may not throw for unrecognized parts using forgiving parsing: - // https://drafts.csswg.org/selectors/#forgiving-selector - // like the `:is()` pseudo-class: - // https://drafts.csswg.org/selectors/#matches - // `CSS.supports` is still expected to return `false` then: - // https://drafts.csswg.org/css-conditional-4/#typedef-supports-selector-fn - // https://drafts.csswg.org/css-conditional-4/#dfn-support-selector - if ( support.cssSupportsSelector && - - // `CSS.supports( "selector(...)" )` requires the argument to the - // `selector` function to be a ``, not - // a `` which our selector may be. Wrapping with - // `:is` works around the issue and is supported by all browsers - // we support except for IE which will fail the support test anyway. - // eslint-disable-next-line no-undef - !CSS.supports( "selector(:is(" + newSelector + "))" ) ) { - - // Support: IE 11+ - // Throw to get to the same code path as an error directly in qSA. - // Note: once we only support browser supporting - // `CSS.supports('selector(...)')`, we can most likely drop - // the `try-catch`. IE doesn't implement the API. - throw new Error(); - } - push.apply( results, newContext.querySelectorAll( newSelector ) ); diff --git a/src/selector/rbuggyQSA.js b/src/selector/rbuggyQSA.js index 709fb0f63f..29dd93d1b7 100644 --- a/src/selector/rbuggyQSA.js +++ b/src/selector/rbuggyQSA.js @@ -21,14 +21,13 @@ if ( isIE ) { ); } -if ( !support.cssSupportsSelector ) { - - // Support: Chrome 105+, Safari 15.4+ - // `:has()` uses a forgiving selector list as an argument so our regular - // `try-catch` mechanism fails to catch `:has()` with arguments not supported - // natively like `:has(:contains("Foo"))`. Where supported & spec-compliant, - // we now use `CSS.supports("selector(:is(SELECTOR_TO_BE_TESTED))")`, but - // outside that we mark `:has` as buggy. +if ( !support.cssHas ) { + + // Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ + // In some browsers, `:has()` uses a forgiving selector list as an argument, + // so our regular `try-catch` mechanism fails to catch `:has()` with arguments + // not supported natively, like `:has(:contains("Foo"))`. The spec now requires + // `:has()` parsing to be non-forgiving but browsers have not adjusted yet. rbuggyQSA.push( ":has" ); } diff --git a/src/selector/support.js b/src/selector/support.js index 387c41bfea..95d22c4e56 100644 --- a/src/selector/support.js +++ b/src/selector/support.js @@ -1,27 +1,20 @@ +import document from "../var/document.js"; import support from "../var/support.js"; -// Support: IE 11+ -// IE doesn't support `CSS.supports( "selector(...)" )`; it will throw -// in this support test. +// Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ +// Make sure forgiving mode is not used in `:has()`. +// `*` is needed as Safari & newer Chrome implemented something in between +// for `:has()` - it throws in `qSA` if it only contains an unsupported +// argument but multiple ones, one of which is supported, are fine. +// We want to play safe in case `:is()` gets the same treatment. +// +// Note that we don't need to detect the complete lack of support for `:has()` +// as then the `qSA` path will throw and fall back to jQuery traversal anyway. try { - /* eslint-disable no-undef */ - - // Support: Chrome 105+, Firefox <106, Safari 15.4+ - // Make sure forgiving mode is not used in `CSS.supports( "selector(...)" )`. - // - // `:is()` uses a forgiving selector list as an argument and is widely - // implemented, so it's a good one to test against. - support.cssSupportsSelector = CSS.supports( "selector(*)" ) && - - // `*` is needed as Safari & newer Chrome implemented something in between - // for `:has()` - it throws in `qSA` if it only contains an unsupported - // argument but multiple ones, one of which is supported, are fine. - // We want to play safe in case `:is()` gets the same treatment. - !CSS.supports( "selector(:is(*,:jqfake))" ); - - /* eslint-enable */ + document.querySelector( ":has(*,:jqfake)" ); + support.cssHas = false; } catch ( e ) { - support.cssSupportsSelector = false; + support.cssHas = true; } export default support; diff --git a/test/unit/support.js b/test/unit/support.js index aa3386ef83..abe915646a 100644 --- a/test/unit/support.js +++ b/test/unit/support.js @@ -59,31 +59,31 @@ testIframe( userAgent = window.navigator.userAgent, expectedMap = { ie_11: { - cssSupportsSelector: false, + cssHas: true, reliableTrDimensions: false }, chrome: { - cssSupportsSelector: false, + cssHas: false, reliableTrDimensions: true }, safari: { - cssSupportsSelector: false, + cssHas: false, reliableTrDimensions: true }, webkit: { - cssSupportsSelector: true, + cssHas: true, reliableTrDimensions: true }, firefox_102: { - cssSupportsSelector: false, + cssHas: true, reliableTrDimensions: false }, firefox: { - cssSupportsSelector: true, + cssHas: true, reliableTrDimensions: false }, ios: { - cssSupportsSelector: false, + cssHas: false, reliableTrDimensions: true } }; @@ -91,7 +91,7 @@ testIframe( // Make the selector-native build pass tests. for ( browserKey in expectedMap ) { if ( !includesModule( "selector" ) ) { - delete expectedMap[ browserKey ].cssSupportsSelector; + delete expectedMap[ browserKey ].cssHas; } } From 5817baa575e79d6474362553fa9a00ccc0cf7458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Tue, 14 Feb 2023 00:25:51 +0100 Subject: [PATCH 2/3] Selector: Reword a few comments Co-authored-by: Richard Gibson --- src/selector/rbuggyQSA.js | 9 +++++---- src/selector/support.js | 16 ++++++++-------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/selector/rbuggyQSA.js b/src/selector/rbuggyQSA.js index 29dd93d1b7..674598e839 100644 --- a/src/selector/rbuggyQSA.js +++ b/src/selector/rbuggyQSA.js @@ -24,10 +24,11 @@ if ( isIE ) { if ( !support.cssHas ) { // Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ - // In some browsers, `:has()` uses a forgiving selector list as an argument, - // so our regular `try-catch` mechanism fails to catch `:has()` with arguments - // not supported natively, like `:has(:contains("Foo"))`. The spec now requires - // `:has()` parsing to be non-forgiving but browsers have not adjusted yet. + // Our regular `try-catch` mechanism fails to detect natively-unsupported + // pseudo-classes inside `:has()` (such as `:has(:contains("Foo"))`) + // in browsers that parse the `:has()` argument as a forgiving selector list. + // https://drafts.csswg.org/selectors/#relational now requires the argument + // to be parsed unforgivingly, but browsers have not yet fully adjusted. rbuggyQSA.push( ":has" ); } diff --git a/src/selector/support.js b/src/selector/support.js index 95d22c4e56..396e0e046f 100644 --- a/src/selector/support.js +++ b/src/selector/support.js @@ -2,14 +2,14 @@ import document from "../var/document.js"; import support from "../var/support.js"; // Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ -// Make sure forgiving mode is not used in `:has()`. -// `*` is needed as Safari & newer Chrome implemented something in between -// for `:has()` - it throws in `qSA` if it only contains an unsupported -// argument but multiple ones, one of which is supported, are fine. -// We want to play safe in case `:is()` gets the same treatment. -// -// Note that we don't need to detect the complete lack of support for `:has()` -// as then the `qSA` path will throw and fall back to jQuery traversal anyway. +// Make sure the the `:has()` argument is parsed unforgivingly. +// We include `*` in the test to detect buggy implementations that are +// _selectively_ forgiving (specifically when the list includes at least +// one valid selector). +// Note that we treat complete lack of support for `:has()` as if it were +// spec-compliant support, which is fine because use of `:has()` in such +// environments will fail in the qSA path and fall back to jQuery traversal +// anyway. try { document.querySelector( ":has(*,:jqfake)" ); support.cssHas = false; From e53a28beaa56c37ad15d6d393968e7f22683eff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Tue, 14 Feb 2023 00:43:19 +0100 Subject: [PATCH 3/3] Selector: Correctly declare support test results for older iOS --- test/unit/support.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/support.js b/test/unit/support.js index abe915646a..2bf43112f6 100644 --- a/test/unit/support.js +++ b/test/unit/support.js @@ -82,6 +82,10 @@ testIframe( cssHas: true, reliableTrDimensions: false }, + ios_14_15_3: { + cssHas: true, + reliableTrDimensions: true + }, ios: { cssHas: false, reliableTrDimensions: true @@ -105,6 +109,8 @@ testIframe( expected = expectedMap.firefox_102; } else if ( /firefox/i.test( userAgent ) ) { expected = expectedMap.firefox; + } else if ( /iphone os (?:14_|15_[0123])/i.test( userAgent ) ) { + expected = expectedMap.ios_14_15_3; } else if ( /(?:iphone|ipad);.*(?:iphone)? os \d+_/i.test( userAgent ) ) { expected = expectedMap.ios; } else if ( typeof URLSearchParams !== "undefined" &&