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. 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/CAL5BFfXdVLM5NC-5jVmws7axd1xQ4jajdEWu%3DCHZMPRToVuNZQ%40mail.gmail.com.

Reply via email to