Something wrong with the citation style of the previous mail. I'll send the reply again.
>>> OK, so for where we're risking seeing more fallbacks than before, but according to your manual inspection, that seems fine? >> Yes, I think so. >> Based on the usage metrics, only about 0.5 % of page loads could be affected by this feature. Considering the manual investigation on the top pages (only 1 of of 10 is for `:where()`, and the rest are for `:has()`. no urls for `:is()`), the ratio of the `:where()` is likely to be much less than 0.5 %. > In the manual inspection, how many function calls had a mix of valid and invalid selectors? (that would be impacted by this change) There is no mix of valid and invalid in the manual inspection for the top URLs. :has() and :where() are used only with empty argument or valid argument. >> Will it be better to add a feature for this change and add some metrics (something like, how many page loads use :has() with both valid and invalid selector) before releasing it to stable? > Adding a feature (including a base_feature) to the `:has` change would be good. Would you be able to merge that back to 110? > I think we should tie the `:has` change to this intent. The risk profile seems similar. I made a CL that adds 'CSSPseudoHasNonForgivingParsing' feature ( https://chromestatus.com/feature/6177049203441664) for the change: - https://chromium-review.googlesource.com/c/chromium/src/+/4151453 The CL also adds two metrics so that we can get usecounter of the cases that the change affects: - CSSPseudoHasContainsMixOfValidAndInvalid : ':has(a, :foo)' - CSSPseudoIsWhereContainsMixOfValidAndInvalid : ':is(a, :foo)', ':where(a, :foo)' I'll try to merge the CL to 110 branch after it landed. 2023년 1월 11일 수요일 오후 2시 33분 10초 UTC+9에 Byungwoo Lee님이 작성: > > > 2023년 1월 9일 월요일 오후 7시 36분 34초 UTC+9에 [email protected]님이 작성: > On Mon, Jan 9, 2023 at 11:12 AM Byungwoo Lee <[email protected]> wrote: > Thanks! I replied again. :) > > 2023년 1월 6일 금요일 오후 7시 50분 43초 UTC+9에 [email protected]님이 작성: > On Thu, Jan 5, 2023 at 4:59 PM Byungwoo Lee <[email protected]> wrote: > Added missing links. > > 2023년 1월 6일 금요일 오전 12시 52분 25초 UTC+9에 Byungwoo Lee님이 작성: > > Thanks for asking! > > > > Is this change covered by a base feature flag? > > This is behind 'CSSAtSupportsAlwaysNonForgivingParsing' flag, and the flag > doesn't have 'base_feature' field yet. I'll add the field to the feature > before enable it. > > > > Can you clarify if the ':has()' behavior will change here or not? This > last sentence seems to contradict the original message of the intent. > > > Can you confirm that both these cases won't break? > > There's a bit of twisted history here, so it would be better to answer > these two questions at once. (Sorry for the long answer!) > > > 1. What can this feature change? > > After this feature enabled, `@supports selector()` can return different > result when it checks forgiving-parsing pseudo class. > > For example, `@supports selector(:where(:foo, a))` returns true now > (forgiving parsing drops invalid `:foo` inside `:where()`, so the > `:where(:foo, a)` becomes a valid selector `:where(a)` after forgiving > parsing), but it will return false after this feature enabled > (`:where(:foo, a)` will be invalid inside `@supports selector()`). > > OK, so for where we're risking seeing more fallbacks than before, but > according to your manual inspection, that seems fine? > > Yes, I think so. > Based on the usage metrics, only about 0.5 % of page loads could be > affected by this feature. Considering the manual investigation on the top > pages (only 1 of of 10 is for `:where()`, and the rest are for `:has()`. no > urls for `:is()`), the ratio of the `:where()` is likely to be much less > than 0.5 %. > > In the manual inspection, how many function calls had a mix of valid and > invalid selectors? (that would be impacted by this change) > There is no mix of valid and invalid in the manual inspection for the top > URLs. :has() and :where() are used only with empty argument or valid > argument. > > > > But I cannot say that this feature will not affect at all, or that will be > the exact numbers that this feature actually affects after 110(unforgiving > `:has()`) released. > > I think we can get the number at about Apr (the next month after the 110 > released). > > Will it be better to wait more so that we can see the number only for > `:where()` and `:is()`? > > > 2. How is this feature related to `:has()`? > > This `@supports` behavior change was applied to the spec [1] while > resolving an issue of `:has()` [2]. At that time, `:has()` was a > forgiving-parsing pseudo class. So this feature was able to change the > result of `@supports selector(:has(:foo, a))` at first. > > But it is not true now since `:has()` is changed to unforgiving while > resolving the jQuery `:has()` conflict issue [3]. > > Hmm, so the behavior change to `:has` landed > <https://chromium-review.googlesource.com/c/chromium/src/+/4090967> in > M110 without a feature flag nor an intent. How confident are we that this > is safe? > ^^ +Rune Lillesveen > > I think it would not make a critical issue since, > 1. the change only affects `:has()` validity if the `:has()` contains both > valid and invalid arguments (e.g. `:has(:foo, a) { ... }`), and it will not > be used often in the wild. > I got a comment saying something similar while landing the jQuery > workaround - > https://github.com/w3c/csswg-drafts/issues/7676#issuecomment-1235724730 > 2. the change fixes the inconsistency in the existing :has() validity > logic. > - Currently, `:has()`, `:has(:foo)` and `:has(:foo, :bar)` are > invalid, but `:has(:foo, a)` is valid. > - After the change merged, all the above are invalid selector. > 3. Basically, the conflict from the change(making `:has()` unforgiving) > can be easily fixed by changing the selector. (e.g. change `:has(:foo, a) > {...}` to `:has(:where(:foo, a)) {...}` or `where(:has(:foo), :has(a)) > {...}`), > > Will it be better to add a feature for this change and add some metrics > (something like, how many page loads use :has() with both valid and invalid > selector) before releasing it to stable? > > Adding a feature (including a base_feature) to the `:has` change would be > good. Would you be able to merge that back to 110? > > I think we should tie the `:has` change to this intent. The risk profile > seems similar. > > I made a CL that adds 'CSSPseudoHasNonForgivingParsing' feature ( > https://chromestatus.com/feature/6177049203441664) for the change: > - https://chromium-review.googlesource.com/c/chromium/src/+/4151453 > > The CL also adds two metrics so that we can get usecounter of the cases > that the change affects: > - CSSPseudoHasContainsMixOfValidAndInvalid : ':has(a, :foo)' > - CSSPseudoIsWhereContainsMixOfValidAndInvalid : ':is(a, :foo)', > ':where(a, :foo)' > > I'll try to merge the CL to 110 branch after it landed. > > > > > Now this feature doesn't change the `@supports selector(:has(:foo, a))` > result. `@supports selector(:has(:foo, a))` returns always false regardless > of this feature since `:has(:foo, a)` is an invalid selector both inside > and outside of `@supports selector()`. > > > 3. The history about empty `:has()` > > This is a tricky part. > When the 105(the first `:has()` enabled version) is released to stable, a > workaround was merged [4] to avoid the jQuery conflict issue. > > At that time, `:has()` was a forgiving-parsing pseudo class, so > `:has(:foo)` and `:has()` should be a valid selector. > > But the workaround changed the behavior - make `:has()` invalid when all > the arguments are dropped. > - `:has()` is invalid because it doesn't have any argument. > - `:has(:foo)` is invalid because it doesn't have any argument after the > invalid argument `:foo` is dropped. > - `:has(:foo, a)` is valid because it has a valid argument `a` after the > invalid argument `:foo` is dropped. > > Last December, the jQuery conflict issue was resolved [3] and it was > applied to 110 [5] - make `:has()` unforgiving. > - `:has()` is invalid because it doesn't have any argument. > - `:has(:foo)` is invalid because it has an invalid argument `:foo`. > - `:has(:foo, a)` is invalid because it has an invalid argument `:foo`. > > Due to this, the result of `@supports selector(:has())` has been false > since 105. > OK, so the `:has` change only differs from currently shipped behavior if > there's a mix of invalid and valid arguments as part of the supports > statement. And given the fact that the M110 shipped behavior is stricter, > what we may see is more sites fallback if they have such :has supports > statements, but we wouldn't expect real breakage, because presumably the > fallbacks are reasonable? > Yes, exactly. As I replied at above, the 110 change fixes the > inconsistency of :has() validity, and the selector expression can be simply > fixed if it creates actual problem on a site. > > In the perspective of the jQuery `:has()` conflict issue, this change > fixes remaining bugs that the jQuery workaround doesn't fix. > Currently jQuery has a bug on `$(':has(span, :contains(abc))')` since > `@supports selector()` returns true for the selector and > `querySelectorAll()` doesn't throw invalid selector exception. > After making `:has()` unforgiving, jQuery can do its custom traversal for > `:contains()` since `@supports selector()` returns false and > `querySelectorAll()` throws exception. > > > > 4. Why does this feature not affect URLs that use WordPress yootheme? > > Because it checks with empty `:has()` - `@supports not selector(:has())`. > > `@supports not selector(:has())` has been always true since 105, and it > will still be true after this feature enabled because this feature doesn't > affect unforgiving parsing. > > The strange point is that the statement is useless(because it is always > true) and semantically incorrect [6]. > > > 5. Why does this feature not affect URLs that use jQuery `:has()`? > > Because the jQuery `has()` conflict issue was already resolved by making > `:has()` unforgiving [3], [5], and this feature doesn't affect unforgiving > parsing. > > > 6. In short, > > This feature will not affect `:has()` inside `@supports selector()`. > > This feature can affects `:is()` or `where()` inside `@supports > selector()`. (only when its argument is empty or invalid) > > > Hope that this has clarified the question. > -------- > > [1] > https://github.com/w3c/csswg-drafts/commit/3a2efb33d12f6667d6142e89609a982978b49223 > > [2] https://github.com/w3c/csswg-drafts/issues/7280 > > [3] > https://github.com/w3c/csswg-drafts/issues/7676#issuecomment-1341347244 > > [4] > https://chromium.googlesource.com/chromium/src/+/2b818b338146d89e524c4fabc2c6f7fd7776937a > > [5] > https://chromiumdash.appspot.com/commit/7278cf3bf630c7791ba4b4885eb7da64dc16eab2 > [6] It uses `@supports` like this: > @supports not selector(:has()) { > .woocommerce:has(> .woocommerce-MyAccount-navigation){ > display:flex; > justify-content:space-between > } > } > I'm not sure but the `not` seems to be a workaround to make the block > works. > > > 2023년 1월 5일 목요일 오후 7시 8분 7초 UTC+9에 [email protected]님이 작성: > Thanks!! > > A couple of questions below, plus another one: Is this change covered by a > base > feature > <https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/renderer/platform/RuntimeEnabledFeatures.md#generate-a-instance-from-a-blink-feature> > flag? > > On Wed, Jan 4, 2023 at 12:34 PM Byungwoo Lee <[email protected]> wrote: > I checked the top URLs in the ChromeStatus page. (TL;DR - this feature > looks not affect the existing behavior of the top URLs) > > I was able to categorize the URLs as below. > > 1. Checking `:has()` support > - Most of the URLs use `@supports` to check `:has()` support. > - `@support` behavior will not be changed for `:has()` (We can ignore this > case since `:has()` will be unforgiving after 110 released) > > Can you clarify if the ':has()' behavior will change here or not? This > last sentence seems to contradict the original message of the intent. > > - There are 2 sub cases: > - URLs using WordPress yootheme [1] > - URLs using jQuery `has()` [2] > > Can you confirm that both these cases won't break? > > > 2. Checking `:where()` support > - Only one URL(https://learn.ooznest.co.uk/) uses `@supports` to check > `:where()` support. > - `@supports` behavior will be changed for `:where()` after this feature > enabled, but it will not affect the behavior of the web page since the page > handles both support and not support case[3]. > > The only problem that I can see from the top URLs is checking `:where()` > support, but it looks very rare case and it may be already handled like > learn.ooznest.co.uk. > (I was able to see some incorrect usages while checking[4], but I think it > is another discussion of checking empty `:where()`, `:has()`) > > I think that this feature does not have critical impact on the existing > behavior. And as shared previously, Safari and Firefox already changed > their implementations. > > How about shipping this? > > ------------ > > [1] 6 URLs (6/10): > - https://lavalmore.gr/ > - https://www.kussenwereld.nl/ > - https://thelocustgroveflowers.com/ > - https://shop.bmgi.com.au/ > - https://badaptor.com/ > - https://suicidprev.se/ > 'theme1.css' of yootheme contains `@supports not selector(:has()) > {...}` statement. > (e.g. > https://thelocustgroveflowers.com/wp-content/themes/locust-ff/css/theme.1.css?ver=1669913762 > ) > The `@supports not...` statement looks weird since the conditional > block contains rules using `:has()`. > > [2] 2 URLs (2/10): > - https://www.midroog.co.il/ > - https://whadam.tistory.com/ > > [3] A stylesheet file has `@supports selector(:where()) {...}` and > `@supports not selector(:where()) {...}` statement. > ( > https://d3015z1jd0uox2.cloudfront.net/Assets/Guide/black/guide-all-j81VMtmAdGEcl2BaHR40jA.css > ) > > [4] Passing empty `:has()` or `:where()` to `@supports selector()` to > check whether a browser supports the pseudo class. > (e.g. `@supports not selector(:has())`, `@supports selector(:where())`) > > 2023년 1월 3일 화요일 오후 6시 18분 31초 UTC+9에 Byungwoo Lee님이 작성: > Hello Yoav, > > Chrome status shows the number from stable now. > > I checked these metrics. > - https://chromestatus.com/metrics/feature/timeline/popularity/4361 > (CSSAtSupportsDropInvalidWhileForgivingParsing) > - https://chromestatus.com/metrics/feature/timeline/popularity/976 > (CSSAtRuleSupports) > > According to the above metrics, some pages will be affected by this > feature but it seems to be a relatively small fraction: > - Only 0.50 % of page loads are dropping invalid selector while parsing > forgiving selector inside '@supports selector()'. > - 41.10% of page loads are using '@supports', but only 1.21% (0.5/41.1) of > the page loads are dropping invalid selector while parsing forgiving > selector inside '@supports selector()'. > - Less than 0.01 % of top sites are dropping invalid selector while > parsing forgiving selector inside '@supports selector()'. > - 50.89% of top URLs are using '@supports', but less than 0.02% > (0.01/50.89) of the URLs are dropping invalid selector while parsing > forgiving selector inside '@supports selector()'. > > Can we move forward based on this? Or should I check another number? > > 2022년 12월 10일 토요일 오전 1시 26분 57초 UTC+9에 Byungwoo Lee님이 작성: > Hello, > > The 108 branch is shipping to stable now, but the numbers from stable > doesn't seems to be applied to the ChromeStatus stats yet. It seems that > the stable numbers will be applied at Jan. 1st. > - https://chromestatus.com/metrics/feature/timeline/popularity/4361 > > I'll reschedule the feature release to 112 so that we can revisit this > thread when we can get the numbers from stable. > > Thank you! > > p.s. 1 > This feature is not related to :has() anymore since :has() is now > unforgiving: > - Issue resolution: > https://github.com/w3c/csswg-drafts/issues/7676#issuecomment-1341347244 > - CL : https://chromium-review.googlesource.com/c/chromium/src/+/4090967 > This feature only affects :is()/:where() inside @supports. > > p.s. 2 > Once I get the stable number, I'll provide a comparison of these two > numbers that I can get from the ChromeStatus stats: > - Percentage of page loads that drop invalid while forgiving parsing > inside @supports selector > (https://chromestatus.com/metrics/feature/timeline/popularity/4361) > - Percentage of page loads that use @supports rule > (https://chromestatus.com/metrics/feature/timeline/popularity/976) > > The comparison doesn't prove anything, but I think we can at least guess > how much the @supports change affects the existing behavior: > Assuming the current numbers in the above links are from stable, about 40% > of the loaded pages use @supports rule, but only 0.002% of the loaded pages > hit the case of dropping invalid selector while forgiving selector parsing > inside @supports. By simply comparing the numbers, I think we can say that > 1/20000 of the @supports rule usages will be affected by the feature. > > 2022년 10월 10일 월요일 오후 11시 18분 41초 UTC+9에 Byungwoo Lee님이 작성: > To continue this thread after getting the stable Chrome's use counter, I > changed the estimated milestone of this feature from 109 to 111. > I'll share the use counter after the version 108 released. > > Thank you! > > 2022년 9월 29일 목요일 오전 11시 52분 43초 UTC+9에 Byungwoo Lee님이 작성: > > > On 9/27/22 10:07, Byungwoo Lee wrote: > > > On 9/24/22 00:40, Yoav Weiss wrote: > > > On Fri, Sep 23, 2022 at 5:25 PM Byungwoo Lee <[email protected]> wrote: > > Hello Yoav and Mike, > > Thanks for the feedback! I replied inline. > On 9/23/22 22:18, Yoav Weiss wrote: > > > On Fri, Sep 23, 2022 at 3:15 PM Mike Taylor <[email protected]> wrote: > Hi Byungwoo, > > On 9/23/22 4:34 AM, Byungwoo Lee wrote: > Contact emails [email protected] > > Specification > https://drafts.csswg.org/css-conditional-4/#support-definition-ext > > Summary > > Some functional selectors are parsed forgivingly. (e.g. :is(), :has()) If > an argument of the functional selectors is unknown or invalid, the argument > is dropped but the selector itself is not invalidated. To provide a way of > detecting the unknown or invalid arguments in those functional selectors, > this feature applies the CSS Working Group issue resolution: - @supports > uses non-forgiving parsing for all selectors ( > https://github.com/w3c/csswg-drafts/issues/7280#issuecomment-1143852187) > Am I understanding correctly that content that now uses a functional > selector argument that's invalid may break as a result of this? > If so, do we have usecounters to that effect? > > Yes it can change the previous behavior. > > This changes the selector parsing behavior only for the selectors inside > @supports selector(). > > So if authors expected true for '@supports > selector(:is(:some-invalid-selector))', this feature will break it because > the return value will be changed to false after this feature is enabled. > > I'm not sure that we have the usecounters of the case: counting drop of > invalid selector inside @supports selector. > > If it doesn't exists but it is needed, I think we can add it. Will it be > better to add it to get use counters before ship it? > > Yeah, knowing the order of magnitude of potential breakage would be good. > I landed a CL to add the use counter: > > https://chromiumdash.appspot.com/commit/d060459d174c468a78d69d4e2a12925e0e7ab216 > > > It counts the drop of invalid selector while forgiving selector parsing > inside @supports selector(). We can see the stats with > CSSAtSupportsDropInvalidWhileForgivingParsing(4361): > https://chromestatus.com/metrics/feature/timeline/popularity/4361 > > This will be in 108 version so hopefully we can get the use counter after > the version is released. > > - beta (Oct 27) > - stable (Nov 29) > > I'll share the stats when it released. > > Thanks! > > > > Blink component Blink > <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink> > > TAG review > > TAG review status Not applicable > > Can you expand on why you think a TAG review is not needed here? > > I thought that we don't need TAG review and the reason was > > - This is already specified in the spec: > https://drafts.csswg.org/css-conditional-4/#support-definition-ext > <https://drafts.csswg.org/css-conditional-4/#support-definition-ext> > > - Firefox already landed it: > https://bugzilla.mozilla.org/show_bug.cgi?id=1789248 > > Will it be better to change the TAG review status to 'Pending'? > > > Risks > > > Interoperability and Compatibility > > *Gecko*: Shipped/Shipping > https://bugzilla.mozilla.org/show_bug.cgi?id=1789248 > > *WebKit*: Positive > > *Web developers*: Positive > Can you please link to these signals? > > > WebKit: > > - Explained about this in a blog post: > https://webkit.org/blog/13096/css-has-pseudo-class/ > > Web developers: > > - Thumbs ups in the CSSWG issue: > https://github.com/w3c/csswg-drafts/issues/7280 > > - jQuery applied the spec: > https://github.com/jquery/jquery/pull/5107 > > > Rego let me know what I missed (Thanks!), so I'm updating this. > > This specification change has been implemented in WebKit as well as > Firefox: > https://bugs.webkit.org/show_bug.cgi?id=244808 > > I updated the 'Safari views' and 'Tag review' in the chromestatus > accordingly. > > > *WebKit:* Shipped/Shipping https://bugs.webkit.org/show_bug.cgi?id=244808 > > > *Tag review* > No TAG review > > > - This is already specified in the spec: > https://drafts.csswg.org/css-conditional-4/#support-definition-ext > > - Firefox and WebKit already implemented it: > https://bugzilla.mozilla.org/show_bug.cgi?id=1789248 > https://bugs.webkit.org/show_bug.cgi?id=244808 > > > *Tag review status* > pending > > > Could this update affect the shipping decisions? > > thanks, > Mike > > > -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/b03b90af-3911-40b4-dd6f-b12764826cf1%40chromium.org > > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/b03b90af-3911-40b4-dd6f-b12764826cf1%40chromium.org?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/475dcfe0-e296-4418-ba9f-2486884b4542n%40chromium.org.
