On Mon, Aug 1, 2022 at 12:11 PM Yoav Weiss <yoavwe...@chromium.org> wrote:
> > > On Mon, Jul 25, 2022 at 7:44 PM Khushal Sagar <khushalsa...@chromium.org> > wrote: > >> >> >> On Wed, Jul 20, 2022 at 5:17 AM Yoav Weiss <yoavwe...@chromium.org> >> wrote: >> >>> Thanks Khushal! :) >>> >>> The breakage seems potentially significant (at worst, makes the site >>> visually broken and unusable), and the percentage of breakage seems above >>> the threshold we typically consider safe. >>> At the same time this seems like a positive change, and our friends at >>> Mozilla consider it "worth prototyping". >>> >>> Would it be possible to consider this as a deprecation of the old >>> behavior, and run the console issue (+deprecation warnings and outreach to >>> affected sites) for a few milestones, to try and drive the usage down >>> before flipping this change to be on by default? Maybe also get some >>> documentation out there and work with devrel folks to make sure folks are >>> aware of this coming change? >>> >>> Does that sound reasonable? >>> >> >> Thanks for the suggestion Yoav. This sounds reasonable. I've reached out >> to the devrel folks to do more outreach about this change. I'll update this >> thread with the plan for it. The console issue and deprecation warnings >> have been added in M105. >> >> I looked into adding a use counter for sites which have real breakage, >> since the current metric tracks whether the computed style *could* permit >> allow but not whether there is actual overflow at paint time. And >> unfortunately computing potential overflow is not easy to add. The CT >> analysis I ran earlier did this by turning the feature on and tracking >> actual overflow generated by the element. So a couple of questions for >> moving forward: >> >> - Would it be okay to turn the feature on in beta and 1% stable (in M105) >> to collect metrics for the sites with real breakage and the extent of this >> breakage (how many pixels of overflow do we see). This should be lower than >> the counter of 0.017%. >> > > That sounds like a great way to gather data! (assuming the relevant Chrome > processes are followed) > Would be good to gather a histogram of overflowed pixels, to get a sense > of "small breakage" vs. "large breakage". > > >> >> - What's the number (in terms of page loads affected) we should be >> targeting before this would be safe? >> > > From my perspective, I'd be comfortable shipping this if we're seeing less > than 0.003% of page loads in the "large breakage" bucket (say more than > ~5000 overflowing pixels, assuming that number makes sense). > > +Andre Bandarra <andre...@google.com> - for devrel opinions on this. > Currently we're recording a UseCounter when any breakage would happen, i.e, there is any pixel overflow. If it helps to have a UseCounter for overflow above a threshold, I'm happy to add that too. We do have an UMA metric which measures the number of overflowing pixels when there is overflow but it's for all images rendered. I can tie it to the UseCounter so we can also know the number of affected page loads with large breakage. I'll wait till the end of the week for more feedback, otherwise assume 5k is a good threshold for large breakage. > > >> >> >>> >>> Cheers, >>> Yoav >>> >>> On Thursday, July 14, 2022 at 5:03:54 PM UTC+2 Khushal Sagar wrote: >>> >>>> 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/CAMLuWUy5zyHePkt06pjbtAE_vu1VjbybK5VXURhuSETyR%2Bu54g%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAMLuWUy5zyHePkt06pjbtAE_vu1VjbybK5VXURhuSETyR%2Bu54g%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/CAMLuWUwzkromcXgTLpt8DDFUJm6ENQ%2BYd3VmEf%2B_48BF5fNXGw%40mail.gmail.com.
