Skip to content

Conversation

@johannesodland
Copy link
Contributor

@johannesodland johannesodland commented Apr 6, 2025

19ed0f7

Use scrollMargin for lazy load intersection observer
https://bugs.webkit.org/show_bug.cgi?id=264864

Reviewed by Tim Nguyen.

Update to follow step 2 (options) in "start intersection-observing a lazy loading element":
- switch from using rootMargin to using scrollMargin in the IntersectionObserverInit dictionary.
- remove document from the IntersectionObserverInit dictionary

https://html.spec.whatwg.org/#start-intersection-observing-a-lazy-loading-element

This will allow images in scroll containers to load when they are near the viewport,
in stead of when they are intersecting the viewport.

This PR does not change the lazy load scroll margin, that will stay at 100%.

* Source/WebCore/html/LazyLoadImageObserver.cpp:
(WebCore::LazyLoadImageObserver::intersectionObserver):

Canonical link: https://commits.webkit.org/293552@main

49ceac5

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792
Copy link
Contributor

@johannesodland - I am syncing the-img-element, which has few lazy loading tests, just thinking if it progresses any of them. Once land, can you rebase and test it out?

Ref - #43713

Copy link
Member

@nt1m nt1m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test that shows the behavior difference that would fail with the previous code?

@johannesodland
Copy link
Contributor Author

Is there a test that shows the behavior difference that would fail with the previous code?

Hopefully the image-loading-lazy-in-scroller-* tests that @Ahmad-S792 is importing in #43713 will show the difference.

@Ahmad-S792
Copy link
Contributor

@johannesodland - I just merged tests after green EWS, you can now rebase and see if it progresses any test case. :-)

@johannesodland
Copy link
Contributor Author

@johannesodland - I just merged tests after green EWS, you can now rebase and see if it progresses any test case. :-)

Thanks 🙏

I've rebased locally, but it's not passing the tests the way I expected, so I'll investigate before I push an update.

@johannesodland johannesodland force-pushed the eng/Use-scrollMargin-for-lazy-load-intersection-observer branch from 99b8c96 to 49ceac5 Compare April 10, 2025 18:49
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 10, 2025
@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Apr 10, 2025
@Ahmad-S792
Copy link
Contributor

@johannesodland - Thanks! Good to see some WPT progressions. @nt1m - now it is up for your review or Simon.

@Ahmad-S792 Ahmad-S792 added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Apr 10, 2025
@webkit-ews-buildbot webkit-ews-buildbot added merge-queue Applied to send a pull request to merge-queue and removed safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Apr 11, 2025
@webkit-ews-buildbot
Copy link
Collaborator

Safe-Merge-Queue: Build #54084.

https://bugs.webkit.org/show_bug.cgi?id=264864

Reviewed by Tim Nguyen.

Update to follow step 2 (options) in "start intersection-observing a lazy loading element":
- switch from using rootMargin to using scrollMargin in the IntersectionObserverInit dictionary.
- remove document from the IntersectionObserverInit dictionary

https://html.spec.whatwg.org/#start-intersection-observing-a-lazy-loading-element

This will allow images in scroll containers to load when they are near the viewport,
in stead of when they are intersecting the viewport.

This PR does not change the lazy load scroll margin, that will stay at 100%.

* Source/WebCore/html/LazyLoadImageObserver.cpp:
(WebCore::LazyLoadImageObserver::intersectionObserver):

Canonical link: https://commits.webkit.org/293552@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Use-scrollMargin-for-lazy-load-intersection-observer branch from 49ceac5 to 19ed0f7 Compare April 11, 2025 00:20
@webkit-commit-queue
Copy link
Collaborator

Committed 293552@main (19ed0f7): https://commits.webkit.org/293552@main

Reviewed commits have been landed. Closing PR #43712 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 19ed0f7 into WebKit:main Apr 11, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants