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 <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- <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 becauseyou 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 aresubscribed 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.
