Thank you Chris!
On Thu, Nov 30, 2023 at 10:57 AM Chris Harrelson <chris...@chromium.org> wrote: > LGTM3 > > On Thu, Nov 30, 2023 at 10:54 AM Mike Taylor <miketa...@chromium.org> > wrote: > >> LGTM2 >> On 11/30/23 9:33 AM, Traian Captan wrote: >> >> You're welcome Rick! >> >> >> On Thu, Nov 30, 2023 at 5:33 AM Rick Byers <rby...@chromium.org> wrote: >> >>> Thank you Traian! >>> >>> On Thu, Nov 30, 2023 at 7:27 PM Traian Captan <tcap...@chromium.org> >>> wrote: >>> >>>> Hi Rick, >>>> >>>> Yes. I uploaded a CL that increases the spacer size by 30px: >>>> https://chromium-review.googlesource.com/c/chromium/src/+/5075126 >>>> >>>> The tests are now failing as expected on Safari: >>>> >>>> https://wpt.fyi/results/html/semantics/embedded-content/the-img-element?label=pr_head&max-count=1&pr=43446 >>>> >>>> >>>> On Thu, Nov 30, 2023 at 12:14 AM Rick Byers <rby...@chromium.org> >>>> wrote: >>>> >>>>> Interesting. Could you try to improve the tests to capture the interop >>>>> difference and ensure passing reflects full conformance to the spec? We >>>>> rely on WPTs as our quantifiable measure of interoperability... >>>>> >>>>> Rick >>>>> >>>>> On Thu, Nov 30, 2023 at 5:07 PM Traian Captan <tcap...@chromium.org> >>>>> wrote: >>>>> >>>>>> Thank you Rick! >>>>>> >>>>>> I did some investigating into why WebKit is passing some of the new >>>>>> WPTs for lazy loaded images. >>>>>> I think it might be because WebKit is considering the edge as >>>>>> inclusive, while Blink and Gecko do not. >>>>>> In the following example if the spacer height is 100px Safari will >>>>>> report intersecting as true while Chrome and FireFox would report it as >>>>>> false. >>>>>> If the height is increased to 101px, all 3 browsers will report the >>>>>> intersection as false. >>>>>> <!DOCTYPE html> >>>>>> <style> >>>>>> #scroller { width: 100px; height: 100px; overflow: scroll; >>>>>> background-color: gray; } >>>>>> #spacer { width: 50px; height: 100px; } >>>>>> #target { width: 50px; height: 50px; background-color: green; } >>>>>> </style> >>>>>> <div id=scroller> >>>>>> <div id=spacer></div> >>>>>> <div id=target></div> >>>>>> </div> >>>>>> <script> >>>>>> let entries = []; >>>>>> window.onload = function() { >>>>>> const observer = new IntersectionObserver( >>>>>> callback, >>>>>> { >>>>>> rootMargin: "0px" >>>>>> } >>>>>> ); >>>>>> observer.observe(target); >>>>>> }; >>>>>> function callback(entries) { >>>>>> console.log(`isIntersecting = ${entries[0].isIntersecting}`); >>>>>> } >>>>>> </script> >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Nov 27, 2023 at 11:40 PM Rick Byers <rby...@chromium.org> >>>>>> wrote: >>>>>> >>>>>>> On Wed, Nov 22, 2023 at 2:37 PM Yoav Weiss <yoavwe...@chromium.org> >>>>>>> wrote: >>>>>>> >>>>>>>> Thanks, that sounds like a strict improvement. >>>>>>>> >>>>>>>> On Wed, Nov 22, 2023 at 6:25 AM Traian Captan <tcap...@chromium.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Yes, that's correct. >>>>>>>>> >>>>>>>>> >>>>>>>>> On Tue, Nov 21, 2023 at 9:18 PM Yoav Weiss <yoavwe...@chromium.org> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> Do I understand correctly that currently lazy-loaded images in >>>>>>>>>> CSS scrollers have suboptimal behavior and this would improve that >>>>>>>>>> without >>>>>>>>>> potentially harming other cases? >>>>>>>>>> >>>>>>>>>> On Wed, Nov 22, 2023 at 1:55 AM Traian Captan < >>>>>>>>>> tcap...@chromium.org> wrote: >>>>>>>>>> >>>>>>>>>>> Contact emails tcap...@chromium.org >>>>>>>>>>> >>>>>>>>>>> Explainer None >>>>>>>>>>> >>>>>>>>>> >>>>>>>> A short (inline) explainer would help reviewers to understand e.g. >>>>>>>> if this involves changes to the API surface that developers need to >>>>>>>> care >>>>>>>> about. >>>>>>>> Can you write a few words on the impact on developers? >>>>>>>> >>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Specification >>>>>>>>>>> https://html.spec.whatwg.org/#lazy-load-root-margin >>>>>>>>>>> >>>>>>>>>> >>>>>>>> The spec doesn't mention anything specific around root margins or >>>>>>>> scroll margins (other than the algorithm name). >>>>>>>> Are these concepts interoperable? >>>>>>>> >>>>>>> >>>>>>> I dug around a little to try to better understand this. The HTML >>>>>>> spec does mention >>>>>>> <https://html.spec.whatwg.org/#start-intersection-observing-a-lazy-loading-element> >>>>>>> setting >>>>>>> the "scrollMargin" on the IntersectionObserver, a new property we >>>>>>> recently >>>>>>> shipped (I2S >>>>>>> <https://groups.google.com/a/chromium.org/g/blink-dev/c/Ax8rTBusZ4s/m/nogj-s-eGQAJ?utm_medium=email&utm_source=footer> >>>>>>> ). >>>>>>> While WebKit and Gecko aren't yet passing the WPT tests >>>>>>> <https://wpt.fyi/results/intersection-observer?label=master&label=experimental&aligned&q=scroll-margin> >>>>>>> for this yet, interestingly WebKit is already passing >>>>>>> <https://wpt.fyi/results/html/semantics/embedded-content/the-img-element?label=master&label=experimental&aligned&q=image-loading-lazy-in-> >>>>>>> most of the newly added >>>>>>> <https://chromium-review.googlesource.com/c/chromium/src/+/5023396> >>>>>>> WPTs for lazy loaded images in particular. So perhaps their >>>>>>> implementation >>>>>>> already handled this? >>>>>>> >>>>>>> Seems reasonable to me - LGTM1 >>>>>>> >>>>>>> >>>>>>>>>>> >>>>>>>>>>> Summary >>>>>>>>>>> >>>>>>>>>>> Changes the lazy load intersection observer's init dictionary to >>>>>>>>>>> use a scrollMargin instead of a rootMargin. This allows lazy >>>>>>>>>>> loading images >>>>>>>>>>> contained inside CSS scrollers, like carousels, to load as expected >>>>>>>>>>> when >>>>>>>>>>> near the viewport instead of the current behavior where these >>>>>>>>>>> images load >>>>>>>>>>> when at least one pixel is intersecting the viewport. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Blink component Blink>Image >>>>>>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EImage> >>>>>>>>>>> >>>>>>>>>>> Search tags lazyload >>>>>>>>>>> <https://chromestatus.com/features#tags:lazyload>, scrollmargin >>>>>>>>>>> <https://chromestatus.com/features#tags:scrollmargin> >>>>>>>>>>> >>>>>>>>>>> TAG review None >>>>>>>>>>> >>>>>>>>>>> TAG review status Not applicable >>>>>>>>>>> >>>>>>>>>>> Risks >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Interoperability and Compatibility >>>>>>>>>>> >>>>>>>>>>> Overall low as scroll margin also applies to the root element >>>>>>>>>>> thus not affecting lazy loading images that are currently loading >>>>>>>>>>> with just >>>>>>>>>>> a root margin. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> *Gecko*: Positive ( >>>>>>>>>>> https://github.com/w3c/IntersectionObserver/issues/431) >>>>>>>>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1864794 >>>>>>>>>>> >>>>>>>>>>> *WebKit*: Positive ( >>>>>>>>>>> https://github.com/w3c/IntersectionObserver/issues/431#issuecomment-930602435 >>>>>>>>>>> ) https://bugs.webkit.org/show_bug.cgi?id=264864 >>>>>>>>>>> >>>>>>>>>>> *Web developers*: Positive ( >>>>>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1391989) >>>>>>>>>>> >>>>>>>>>>> *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? >>>>>>>>>>> >>>>>>>>>>> None >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Debuggability >>>>>>>>>>> >>>>>>>>>>> None >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> 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 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> https://wpt.fyi/results/html/semantics/embedded-content/the-img-element?label=master&label=experimental&aligned&q=image-loading-lazy-in- >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Flag name on chrome://flags LazyLoadScrollMargin >>>>>>>>>>> >>>>>>>>>>> Finch feature name None >>>>>>>>>>> >>>>>>>>>>> Non-finch justification >>>>>>>>>>> >>>>>>>>>>> This feature is behind an enabled-by-default flag that can be >>>>>>>>>>> disabled if needed. >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Requires code in //chrome? False >>>>>>>>>>> >>>>>>>>>>> Tracking bug >>>>>>>>>>> https://bugs.chromium.org/p/chromium/issues/detail?id=1391989 >>>>>>>>>>> >>>>>>>>>>> Estimated milestones >>>>>>>>>>> Shipping on desktop 121 >>>>>>>>>>> DevTrial on desktop 121 >>>>>>>>>>> Shipping on Android 121 >>>>>>>>>>> DevTrial on Android 121 >>>>>>>>>>> Shipping on WebView 121 >>>>>>>>>>> >>>>>>>>>>> Anticipated spec changes >>>>>>>>>>> >>>>>>>>>>> Open questions about a feature may be a source of future web >>>>>>>>>>> compat or interop issues. Please list open issues (e.g. links to >>>>>>>>>>> known >>>>>>>>>>> github issues in the project for the feature specification) whose >>>>>>>>>>> resolution may introduce web compat/interop risk (e.g., changing to >>>>>>>>>>> naming >>>>>>>>>>> or structure of the API in a non-backward-compatible way). >>>>>>>>>>> None >>>>>>>>>>> >>>>>>>>>>> Link to entry on the Chrome Platform Status >>>>>>>>>>> https://chromestatus.com/feature/5106926245642240 >>>>>>>>>>> >>>>>>>>>>> Links to previous Intent discussions Intent to prototype: >>>>>>>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFxahvtrmHkoOpTuD2eZYVOyFuAhP8ZFVoTuNBS8zYTVY%3DTaaQ%40mail.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/CAFxahvsUb0GEG9WNWRN7Akkowjm03gLj%2Biiq5rG8%2BzdAWMBTNA%40mail.gmail.com >>>>>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFxahvsUb0GEG9WNWRN7Akkowjm03gLj%2Biiq5rG8%2BzdAWMBTNA%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/CAL5BFfVhH_QxckxRLbR05jrN0CY48aQ-Ag3BypusnsyKoDTc0A%40mail.gmail.com >>>>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfVhH_QxckxRLbR05jrN0CY48aQ-Ag3BypusnsyKoDTc0A%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/CAFxahvuw19j2DwRAV4kGecr_V%2BZ2D89nW5PdSUJ1z43HG7JW8g%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFxahvuw19j2DwRAV4kGecr_V%2BZ2D89nW5PdSUJ1z43HG7JW8g%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/e34334bd-1d26-4f38-8e77-4540f2658fc5%40chromium.org >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/e34334bd-1d26-4f38-8e77-4540f2658fc5%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 blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFxahvsgZwM9JV0o1d8XQ%3DaOCCFyr4q2sTDSD9fX6535pDO%2B0g%40mail.gmail.com.
