From 63c3af481c7010920bca68518c434cd27ab22cb2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Go=C5=82=C4=99biowski-Owczarek?= Date: Tue, 14 Feb 2023 11:42:29 +0100 Subject: [PATCH] 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 Closes gh-5207 Ref gh-5206 Ref gh-5098 Ref gh-5107 Ref w3c/csswg-drafts#7676 --- package.json | 2 +- src/selector.js | 83 +++++++++----------------------- test/unit/support.js | 112 ++++++++++++++++++++++--------------------- 3 files changed, 81 insertions(+), 116 deletions(-) diff --git a/package.json b/package.json index 1341ed4b3..517dad7f8 100644 --- a/package.json +++ b/package.json @@ -55,7 +55,7 @@ "karma-webkit-launcher": "2.1.0", "load-grunt-tasks": "5.1.0", "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.9.2", diff --git a/src/selector.js b/src/selector.js index 998700db5..147a2e25c 100644 --- a/src/selector.js +++ b/src/selector.js @@ -303,32 +303,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 9 - 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 ) ); @@ -575,33 +549,22 @@ function setDocument( node ) { return document.querySelectorAll( ":scope" ); } ); - // Support: IE 9 - 11+ - // IE doesn't support `CSS.supports( "selector(...)" )`; it will throw - // in this support test. - // - // 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 = assert( function() { - /* eslint-disable no-undef */ - - return CSS.supports( "selector(*)" ) && - - // Support: Firefox 78-81 only - // In old Firefox, `:is()` didn't use forgiving parsing. In that case, - // fail this test as there's no selector to test against that. - // `CSS.supports` uses unforgiving parsing - document.querySelectorAll( ":is(:jqfake)" ) && - - // `*` 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 */ + // Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ + // 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. + support.cssHas = assert( function() { + try { + document.querySelector( ":has(*,:jqfake)" ); + return false; + } catch ( e ) { + return true; + } } ); // ID filter and find @@ -752,14 +715,14 @@ function setDocument( node ) { } } ); - if ( !support.cssSupportsSelector ) { + if ( !support.cssHas ) { - // 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. + // Support: Chrome 105 - 110+, Safari 15.4 - 16.3+ + // 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/test/unit/support.js b/test/unit/support.js index cd14c2e43..d4955936d 100644 --- a/test/unit/support.js +++ b/test/unit/support.js @@ -64,7 +64,7 @@ testIframe( checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -83,20 +83,20 @@ testIframe( sortDetached: true, sortStable: true }, - ie_10_11: { + ie_9: { ajax: true, boxSizingReliable: false, checkClone: true, checkOn: true, clearCloneStyle: false, - cssSupportsSelector: false, - cors: true, + cssHas: true, + cors: false, createHTMLDocument: true, - disconnectedMatch: true, + disconnectedMatch: false, focusin: true, - getById: true, + getById: false, noCloneChecked: false, - option: true, + option: false, optSelected: false, pixelBoxStyles: true, pixelPosition: true, @@ -104,24 +104,24 @@ testIframe( reliableMarginLeft: true, reliableTrDimensions: false, scope: false, - scrollboxSize: true, + scrollboxSize: false, sortDetached: true, sortStable: true }, - ie_9: { + ie_10_11: { ajax: true, boxSizingReliable: false, checkClone: true, checkOn: true, clearCloneStyle: false, - cssSupportsSelector: false, - cors: false, + cssHas: true, + cors: true, createHTMLDocument: true, - disconnectedMatch: false, + disconnectedMatch: true, focusin: true, - getById: false, + getById: true, noCloneChecked: false, - option: false, + option: true, optSelected: false, pixelBoxStyles: true, pixelPosition: true, @@ -129,7 +129,7 @@ testIframe( reliableMarginLeft: true, reliableTrDimensions: false, scope: false, - scrollboxSize: false, + scrollboxSize: true, sortDetached: true, sortStable: true }, @@ -139,7 +139,7 @@ testIframe( checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: false, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -164,7 +164,7 @@ testIframe( checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: false, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -189,7 +189,7 @@ testIframe( checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: true, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -208,13 +208,13 @@ testIframe( sortDetached: true, sortStable: true }, - safari_9_10: { + firefox_60: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -223,23 +223,23 @@ testIframe( noCloneChecked: true, option: true, optSelected: true, - pixelBoxStyles: false, - pixelPosition: false, + pixelBoxStyles: true, + pixelPosition: true, radioValue: true, - reliableMarginLeft: true, + reliableMarginLeft: false, reliableTrDimensions: true, scope: true, scrollboxSize: true, sortDetached: true, sortStable: true }, - firefox: { + firefox_102: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: true, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -258,13 +258,13 @@ testIframe( sortDetached: true, sortStable: true }, - firefox_102: { + firefox: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -283,13 +283,13 @@ testIframe( sortDetached: true, sortStable: true }, - firefox_60: { + ios_7: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -298,33 +298,33 @@ testIframe( noCloneChecked: true, option: true, optSelected: true, - pixelBoxStyles: true, - pixelPosition: true, + pixelBoxStyles: false, + pixelPosition: false, radioValue: true, - reliableMarginLeft: false, + reliableMarginLeft: true, reliableTrDimensions: true, scope: true, scrollboxSize: true, sortDetached: true, sortStable: true }, - ios: { + ios_8: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, - createHTMLDocument: true, + createHTMLDocument: false, disconnectedMatch: true, focusin: false, getById: true, noCloneChecked: true, option: true, optSelected: true, - pixelBoxStyles: true, - pixelPosition: true, + pixelBoxStyles: false, + pixelPosition: false, radioValue: true, reliableMarginLeft: true, reliableTrDimensions: true, @@ -339,7 +339,7 @@ testIframe( checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -358,23 +358,23 @@ testIframe( sortDetached: true, sortStable: true }, - ios_8: { + ios_11_15_3: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, - createHTMLDocument: false, + createHTMLDocument: true, disconnectedMatch: true, focusin: false, getById: true, noCloneChecked: true, option: true, optSelected: true, - pixelBoxStyles: false, - pixelPosition: false, + pixelBoxStyles: true, + pixelPosition: true, radioValue: true, reliableMarginLeft: true, reliableTrDimensions: true, @@ -383,13 +383,13 @@ testIframe( sortDetached: true, sortStable: true }, - ios_7: { + ios: { ajax: true, boxSizingReliable: true, checkClone: true, checkOn: true, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: false, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -398,8 +398,8 @@ testIframe( noCloneChecked: true, option: true, optSelected: true, - pixelBoxStyles: false, - pixelPosition: false, + pixelBoxStyles: true, + pixelPosition: true, radioValue: true, reliableMarginLeft: true, reliableTrDimensions: true, @@ -414,7 +414,7 @@ testIframe( checkClone: false, checkOn: false, clearCloneStyle: true, - cssSupportsSelector: false, + cssHas: true, cors: true, createHTMLDocument: true, disconnectedMatch: true, @@ -446,7 +446,7 @@ testIframe( // Make the selector-native build pass tests. for ( browserKey in expectedMap ) { if ( !includesModule( "selector" ) ) { - delete expectedMap[ browserKey ].cssSupportsSelector; + delete expectedMap[ browserKey ].cssHas; delete expectedMap[ browserKey ].disconnectedMatch; delete expectedMap[ browserKey ].getById; delete expectedMap[ browserKey ].scope; @@ -457,10 +457,10 @@ testIframe( if ( /edge\//i.test( userAgent ) ) { expected = expectedMap.edge; - } else if ( /(msie 10\.0|trident\/7\.0)/i.test( userAgent ) ) { - expected = expectedMap.ie_10_11; } else if ( /msie 9\.0/i.test( userAgent ) ) { expected = expectedMap.ie_9; + } else if ( /(msie 10\.0|trident\/7\.0)/i.test( userAgent ) ) { + expected = expectedMap.ie_10_11; } else if ( /chrome/i.test( userAgent ) ) { // Catches Chrome on Android as well (i.e. the default @@ -476,12 +476,14 @@ testIframe( expected = expectedMap.firefox; } else if ( /android 4\.[0-3]/i.test( userAgent ) ) { expected = expectedMap.android; - } else if ( /iphone os (?:9|10)_/i.test( userAgent ) ) { - expected = expectedMap.ios_9_10; - } else if ( /iphone os 8_/i.test( userAgent ) ) { - expected = expectedMap.ios_8; } else if ( /iphone os 7_/i.test( userAgent ) ) { expected = expectedMap.ios_7; + } else if ( /iphone os 8_/i.test( userAgent ) ) { + expected = expectedMap.ios_8; + } else if ( /iphone os (?:9|10)_/i.test( userAgent ) ) { + expected = expectedMap.ios_9_10; + } else if ( /iphone os (?:1[1234]_|15_[0123])/i.test( userAgent ) ) { + expected = expectedMap.ios_11_15_3; } else if ( /(?:iphone|ipad);.*(?:iphone)? os \d+_/i.test( userAgent ) ) { expected = expectedMap.ios; } else if ( typeof URLSearchParams !== "undefined" && -- 2.39.5