Hey folks,

I'm summarizing the steps to mitigate the compat risk with this feature
based on the feedback:

   - Add a warning to the console when a developer style would permit
   replaced elements to overflow. The patch to add that is here
   <https://chromium-review.googlesource.com/c/chromium/src/+/3763640> and
   documentation to help developers debug, which is referenced in the console
   warning, is here
   <https://github.com/WICG/shared-element-transitions/pull/166/files>. We
   can pre-emptively add this warning to M105 and ship the feature in M106 to
   have a one release window before the behaviour changes.

   - Send an email to the webmaster of sites surfaced in the CT analysis
   which already have the styles that trigger the warning above.

In addition to the above, the feature can be turned off with a server-side
config using finch if there is any severe breakage.

Please let me know if the above suffices.

Thanks.
Khushal

On Wed, Jul 13, 2022 at 4:11 PM Khushal Sagar <khushalsa...@chromium.org>
wrote:

>
>
> On Wed, Jul 13, 2022 at 3:44 PM Mike Taylor <miketa...@chromium.org>
> wrote:
>
>> On 7/13/22 3:04 PM, Khushal Sagar wrote:
>>
>>
>>
>> On Wed, Jul 13, 2022 at 6:12 AM Yoav Weiss <yoavwe...@chromium.org>
>> wrote:
>>
>>>
>>>
>>> On Wednesday, July 13, 2022 at 3:54:28 AM UTC+2 Khushal Sagar wrote:
>>>
>>>> On Mon, Jul 11, 2022 at 11:40 AM Yoav Weiss <yoavwe...@chromium.org>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, Jul 8, 2022 at 7:22 PM Khushal Sagar <
>>>>> khushalsa...@chromium.org> wrote:
>>>>>
>>>>>> Contact emails khushalsa...@chromium.org, vmp...@chromium.org
>>>>>>
>>>>>> Explainer
>>>>>> https://github.com/WICG/shared-element-transitions/blob/main/overflow-on-replaced-elements.md
>>>>>> https://github.com/w3c/csswg-drafts/issues/7058
>>>>>>
>>>>>> Specification
>>>>>> https://drafts.csswg.org/css-overflow/#overflow-properties
>>>>>>
>>>>>> Summary
>>>>>>
>>>>>> This change allows developers to use the existing `overflow` property
>>>>>> with replaced elements that paint outside the content-box. Paired with
>>>>>> `object-view-box` this can be used to create an image with a custom glow 
>>>>>> or
>>>>>> shadow applied, with proper ink-overflow behavior like a CSS shadow would
>>>>>> have.
>>>>>>
>>>>>> Blink component Blink>CSS
>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3ECSS>
>>>>>>
>>>>>> TAG review https://github.com/w3ctag/design-reviews/issues/750
>>>>>>
>>>>>> TAG review status Pending
>>>>>>
>>>>>> Risks
>>>>>> Interoperability and Compatibility
>>>>>>
>>>>>> This feature changes the behaviour of the existing overflow property
>>>>>> on replaced elements (img, video, canvas). Currently `overflow:visible` 
>>>>>> in
>>>>>> a developer stylesheet on such elements is ignored during paint and the
>>>>>> content is clipped to the element's content-box. With this feature,
>>>>>> `overflow:visible` will result in content outside the element's 
>>>>>> content-box
>>>>>> to paint as ink overflow. We've collected use counter data to measure the
>>>>>> number of sites which could be affected by this. The use counter data
>>>>>> collected over 1 week of a stable release (M102) is as follows. We
>>>>>> collected 2 different counters explained below. * The first measures any
>>>>>> instance where overflow is explicitly set from developer styles to 
>>>>>> visible.
>>>>>> The percentage of page loads with this is 2.16%. * The second measures 
>>>>>> the
>>>>>> above instances but only includes the cases with object-fit set to cover 
>>>>>> or
>>>>>> none or object-position set to any value other than the default (50% 
>>>>>> 50%).
>>>>>> The rationale behind this counter is to exclude cases which can not cause
>>>>>> overflow (such as object-fit:contain), even if overflow is set to 
>>>>>> visible.
>>>>>> The percentage of page loads with this is 0.017%.
>>>>>>
>>>>>
>>>>> That's not nothing. Any idea what breakage may look like?
>>>>> Can we maybe collect histograms on *how much* overflow would occur in
>>>>> those cases? (maybe with ClusterTelemetry initially, to get a rough idea 
>>>>> in
>>>>> the lab)
>>>>>
>>>>
>>>> I ran an analysis on CT using top 100k sites for desktop and top 10k
>>>> sites on mobile. The raw numbers are here: desktop
>>>> <https://docs.google.com/spreadsheets/d/1kKWq8kqZOfCXqiHaiamYNDdTs5x1_YJfDTnAgXOIwaE/edit#gid=0>
>>>> and mobile
>>>> <https://docs.google.com/spreadsheets/d/1SrNyrEe4yzCOIxqNOlNgCk8O58NqqoBlBTd4Wn_gKCc/edit#gid=0>,
>>>> and the rough patch
>>>> <https://chromium-review.googlesource.com/c/chromium/src/+/3749485> to
>>>> collect this data. The highlights from the analysis are below:
>>>>
>>>>    - The number of sites which override the default CSS to allow
>>>>    overflow *and* also had overflow during painting was 13 out of 10k on
>>>>    mobile and 39 out of 63k on desktop (only 63k sites yielded results out 
>>>> of
>>>>    100k).
>>>>
>>>>    - I measured the percentage of area painted outside the content box
>>>>    out of the total painted area. The highest was 88% on desktop and 70% on
>>>>    mobile.
>>>>
>>>>
>>> I'm not sure what that means in practice. Can you elaborate? Have you
>>> looked at extreme cases to see the impact there?
>>>
>>
>> Sorry, I should've added more details. :) I was looking for breakages
>> with 2 numbers: sites with the largest number of overflowing pixels (such
>> that other content could be occluded); sites with the largest percentage of
>> image content outside the content box. But I realize the former is probably
>> better to identify breakages.
>>
>> Looking at the top 10 sites, the worst affected is liveops.com. This has
>> cases which use object-fit:cover so the image scales to a size bigger than
>> its content rect and developer CSS overrides overflow to visible.
>> Unfortunately, interacting more with this site, I did see images which are
>> drawing above other content (screenshot attached) as you scroll down. This
>> pattern showed up on the rest of the sites with high overflow numbers too
>> (another example with screenshot attached).
>>
>> This kind of breakage is expected with this change. I'm not sure where to
>> put the cutoff to identify sites with significant breakage, but there are
>> at least 30 sites (out of 63k) that have images with more than 100px of
>> overflow. The fix for the developer is to trace down the source of the
>> overflow override and remove it.
>>
>> I'm not sure what's the recommended way to progress with a behaviour
>> change like this given these numbers, the instances of affected sites seems
>> low. Since the fix is simple, but hard to diagnose, one option could be to
>> add a warning to the console: "overflow:visible now causes images to draw
>> outside their bounds. Please make sure this style is intentional" for a few
>> releases (credits to @Vladimir Levin <vmp...@chromium.org> for the
>> idea). Would that suffice?
>>
>> This feels like a fairly challenging bug to diagnose out without a DevTools
>> issue <https://developer.chrome.com/docs/devtools/issues/> (or similar
>> console message that links to some useful docs, as you proposed). Would we
>> have the ability to conditionally create these for some overflow threshold
>> value?
>>
> Sure, we can do this conditionally based on the area of overflow. I'd be
> inclined to always do it if the UA style is overridden to allow overflow
> for at least a few releases. Since it's likely that existing sites are not
> overriding this style intentionally (because currently it's a no-op). Just
> to catch cases which don't show up in local testing because scaling of the
> image (with properties like object-fit) can depend on the form factor of
> the device.
>
>>
>>
>>>
>>>> I manually went through ~10 sites on both desktop and mobile. For the
>>>> ones which repro-ed, the breakage was losing rounded corners because
>>>> border-radius doesn't clip the content if 'overflow' is 'visible'. In fact
>>>> a few sites had the same code, likely coming from customerly
>>>> <https://www.customerly.io/> based on class names and the UX. There
>>>> was one case where an image (used in the background) had object-fit:cover
>>>> and overflowed outside the content box now. I've attached screenshots for
>>>> both of these.
>>>>
>>>> Overall I didn't see any case where the overflow occluded any other
>>>> content on the page.
>>>>
>>>
>>> That's reassuring! :)
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> *Gecko*: No signal (
>>>>>> https://github.com/mozilla/standards-positions/issues/659)
>>>>>>
>>>>>> *WebKit*: No signal (
>>>>>> https://lists.webkit.org/pipermail/webkit-dev/2022-June/032317.html)
>>>>>>
>>>>>> *Web developers*: No signals
>>>>>>
>>>>>> *Other signals*:
>>>>>>
>>>>>> WebView application risks
>>>>>>
>>>>>> Does this intent deprecate or change behavior of existing APIs, such
>>>>>> that it has potentially high risk for Android WebView-based applications?
>>>>>>
>>>>>> Debuggability
>>>>>>
>>>>>> This is a CSS property which can be debugged in the devtools style
>>>>>> panel similar to other CSS properties.
>>>>>>
>>>>>> Will this feature be supported on all six Blink platforms (Windows,
>>>>>> Mac, Linux, Chrome OS, Android, and Android WebView)? Yes
>>>>>>
>>>>>> Is this feature fully tested by web-platform-tests
>>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>
>>>>>> ? Yes
>>>>>>
>>>>>> Flag name CSSOverflowForReplacedElements
>>>>>>
>>>>>> *Note: Because of the compat risk with this feature, this flag can be
>>>>>> controlled via Finch. This will allow us to rollback with a server-side
>>>>>> config change if needed.*
>>>>>>
>>>>>> Requires code in //chrome? False
>>>>>>
>>>>>> Tracking bug
>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1321217
>>>>>>
>>>>>> Estimated milestones
>>>>>>
>>>>>> M105
>>>>>>
>>>>>> Anticipated spec changes
>>>>>>
>>>>>> N/A
>>>>>>
>>>>>> Link to entry on the Chrome Platform Status
>>>>>> https://chromestatus.com/feature/5137515594383360
>>>>>>
>>>>>> Links to previous Intent discussions Intent to prototype:
>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUykJWEAqVzcUy15fpBNdA68508Mny_1z--FCBKXRTZOFQ%40mail.gmail.com
>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/camluwuykjweaqvzcuy15fpbnda68508mny_1z--fcbkxrtz...@mail.gmail.com>
>>>>>>
>>>>>>
>>>>>> This intent message was generated by Chrome Platform Status
>>>>>> <https://chromestatus.com/>.
>>>>>> --
>>>>>> 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 blink-dev+unsubscr...@chromium.org.
>>>>>> To view this discussion on the web visit
>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUze8JV6twLfhPBwkXj_UBMGApU048OdY33hYQn_KDj2rA%40mail.gmail.com
>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUze8JV6twLfhPBwkXj_UBMGApU048OdY33hYQn_KDj2rA%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 blink-dev+unsubscr...@chromium.org.
>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUz9cutp%2BinEc3%2B7sdv%2B2TPoBbEeFCZjZFExBHSOL1p47A%40mail.gmail.com
>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUz9cutp%2BinEc3%2B7sdv%2B2TPoBbEeFCZjZFExBHSOL1p47A%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 blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUzoOPyY-%2BdxZ-vkBVvR89oYfJ25uyPrGWMYfCi4_yte3Q%40mail.gmail.com.

Reply via email to