Sounds like a pragmatic yet likely safe plan to me. LGTM2

On Mon, Jan 16, 2023 at 5:39 AM Yoav Weiss <[email protected]> wrote:

> LGTM1 to ship in M111 (while monitoring the relevant UMAs from M110) after
> merging back the use counters.
>
> On Thu, Jan 12, 2023 at 4:40 PM Rune Lillesveen <[email protected]>
> wrote:
>
>>
>>
>> On Thu, Jan 12, 2023 at 1:00 PM Yoav Weiss <[email protected]>
>> wrote:
>>
>>>
>>>
>>> On Wed, Jan 11, 2023 at 7:53 AM Yoav Weiss <[email protected]>
>>> wrote:
>>>
>>>> thanks! :)
>>>>
>>>> On Wed, Jan 11, 2023 at 6:42 AM Byungwoo Lee <[email protected]> wrote:
>>>>
>>>>> 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.
>>>>>
>>>>
>>> Just to clarify - 0.5% is about 2 orders of magnitude higher than the
>>> levels of breakage we're typically comfortable with.
>>> But from our discussion it seems that the levels of actual breakage are
>>> likely to be significantly smaller.
>>>
>>> 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.
>>>>>
>>>>
>>> That'd be great to land and merge back.
>>> +Rune Lillesveen <[email protected]> - can you help make that happen?
>>>
>>
>> Yes.
>>
>> Once we have that in place, I'd be comfortable with turning on the
>>> feature on M111, and carefully watching the (internal) UMA use counter
>>> stats for Beta as it rolls out, and revert it if we'd see that actual
>>> breakage is likely to be larger than expected.
>>>
>>> Rune, Byungwoo - what do you think?
>>>
>>
>> Sgtm.
>>
>>
>>>
>>>>>
>>>>> 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>
>>>>>> .
>>>>>>
>>>>>
>>
>> --
>> Rune Lillesveen
>>
>> --
> 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/CAL5BFfVruS-25V_eUJFUQ%3D%2BuojRyumyExkL9YzEKLfTj4qHC1Q%40mail.gmail.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfVruS-25V_eUJFUQ%3D%2BuojRyumyExkL9YzEKLfTj4qHC1Q%40mail.gmail.com?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/CAFUtAY-egorwJA-9sSCTfHNsEs3%2BihbDuRjP6FHS7YkFmG8OTw%40mail.gmail.com.

Reply via email to