Skip to content

Commit

Permalink
Selector: Stop relying on CSS.supports( "selector(...)" )
Browse files Browse the repository at this point in the history 
`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
  • Loading branch information
@mgol
mgol committed Feb 9, 2023
1 parent 582785e commit bcb54ed
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 89 deletions.
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
80 changes: 21 additions & 59 deletions src/selector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<complex-selector>`, not
// a `<complex-selector-list>` 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 )
);
Expand Down Expand Up @@ -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 - 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.
//
// 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 */
// 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.
support.cssHas = assert( function() {
try {
document.querySelector( ":has(*,:jqfake)" );
return false;
} catch ( e ) {
return true;
}
} );

// ID filter and find
Expand Down Expand Up @@ -752,14 +715,13 @@ 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+
// 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" );
}

Expand Down
60 changes: 31 additions & 29 deletions test/unit/support.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -89,7 +89,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: false,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -114,7 +114,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: false,
cssSupportsSelector: false,
cssHas: true,
cors: false,
createHTMLDocument: true,
disconnectedMatch: false,
Expand All @@ -139,7 +139,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: false,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -158,13 +158,13 @@ testIframe(
sortDetached: true,
sortStable: true
},
safari: {
webkit: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -183,13 +183,13 @@ testIframe(
sortDetached: true,
sortStable: true
},
webkit: {
safari: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: true,
cssHas: false,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -208,13 +208,13 @@ testIframe(
sortDetached: true,
sortStable: true
},
safari_9_10: {
firefox: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -223,23 +223,23 @@ testIframe(
noCloneChecked: true,
option: true,
optSelected: true,
pixelBoxStyles: false,
pixelPosition: false,
pixelBoxStyles: true,
pixelPosition: true,
radioValue: true,
reliableMarginLeft: true,
reliableTrDimensions: true,
reliableTrDimensions: false,
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,
Expand All @@ -258,13 +258,13 @@ testIframe(
sortDetached: true,
sortStable: true
},
firefox_102: {
firefox_60: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -276,20 +276,20 @@ testIframe(
pixelBoxStyles: true,
pixelPosition: true,
radioValue: true,
reliableMarginLeft: true,
reliableTrDimensions: false,
reliableMarginLeft: false,
reliableTrDimensions: true,
scope: true,
scrollboxSize: true,
sortDetached: true,
sortStable: true
},
firefox_60: {
ios: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: false,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -301,20 +301,20 @@ testIframe(
pixelBoxStyles: true,
pixelPosition: true,
radioValue: true,
reliableMarginLeft: false,
reliableMarginLeft: true,
reliableTrDimensions: true,
scope: true,
scrollboxSize: true,
sortDetached: true,
sortStable: true
},
ios: {
ios_11_14: {
ajax: true,
boxSizingReliable: true,
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -339,7 +339,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -364,7 +364,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: false,
disconnectedMatch: true,
Expand All @@ -389,7 +389,7 @@ testIframe(
checkClone: true,
checkOn: true,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand All @@ -414,7 +414,7 @@ testIframe(
checkClone: false,
checkOn: false,
clearCloneStyle: true,
cssSupportsSelector: false,
cssHas: true,
cors: true,
createHTMLDocument: true,
disconnectedMatch: true,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -476,6 +476,8 @@ testIframe(
expected = expectedMap.firefox;
} else if ( /android 4\.[0-3]/i.test( userAgent ) ) {
expected = expectedMap.android;
} else if ( /iphone os (?:1[1234])_/i.test( userAgent ) ) {
expected = expectedMap.ios_11_14;
} else if ( /iphone os (?:9|10)_/i.test( userAgent ) ) {
expected = expectedMap.ios_9_10;
} else if ( /iphone os 8_/i.test( userAgent ) ) {
Expand Down

0 comments on commit bcb54ed

Please sign in to comment.