Per #5236 (comment) , my understanding was that we only want to start observing in the update the rendering. But maybe I was overthinking it. Should the "update the image data" add lazy images to an intersection observer directly?

Per #5236 (comment) , my understanding was that we only want to start observing in the update the rendering.

I think from updating-the-image-data if loading=lazy, we always want to observe the image, so that the update-the-rendering resumes the updating-the-image-data algorithm.

I'm commenting on how we observe the image, not so much when. My understanding from #5236 is that we don't want to create a per-document image list, and add a new step to update-the-rendering. Instead, from updating-the-image-data, we want to create and attach an Intersection Observer, and not add anything to the updating-the-image-data. That way the next time update-the-rendering runs, we just delegate to the Intersection Observer steps, instead of our own custom logic. Does that make sense? Please see @domenic's comment: #5236 (comment)

I'm happy to take on this approach, as it is pretty complicated and interesting to me. Or if you want to adjust this PR to do so, that's fine too.

Thanks for the explanation! It makes sense to me now. I'll update the PR.

OK, please take a look.

I set the root to the document, and left rootMargin and threshold UA-defined.

I realized now though that IntersectionObserver can only initialize rootMargin, and then it's read-only. Per #5408 (comment) , I think it needs to be changeable somehow?

I realized now though that IntersectionObserver can only initialize rootMargin, and then it's read-only. Per #5408 (comment) , I think it needs to be changeable somehow?

Filed w3c/IntersectionObserver#428

cc @rwlbuis @smfr

There are no implementation bugs to be filed, since we're just aligning the spec to what implementations are already doing, so that can probably be checked

http://software.hixie.ch/utilities/js/live-dom-viewer/saved/8073

It seems in both Chromium and Gecko, root is the implicit root. Is that what we want? This PR sets it to the image's node document. Using the implicit root takes away the rootMargin if the origins aren't similar-origin, per IntersectionObserver spec (for images in nested browsing contexts).

Yes, that's what you want since otherwise you load images inside iframes that are out of view.

I think I've addressed all major reviews in this with my latest commit, would appreciate if another owner took a look (@annevk ?)

Thanks @domfarolino, LGTM

we should not add new points where we instantiate JavaScript objects or call their methods.

Although I agree in general, I'm not sure whether we should block this work on that. It seems like it would require a large refactoring of the IntersectionObserver spec.

A middle ground might be to add awkward disclaimers everywhere, e.g. "new IntersectionObserver instance (using the original value of the IntersectionObserver constructor)", "For each entry in entries (using a method of iteration which does not trigger developer-modifiable array accessors or iteration hooks)", "if entry.isIntersecting is true (using the original value of the isIntersecting getter)", etc.

But, I guess if we could pull off the refactoring, that would be much nicer...

I agree with the comments around using the DOM-exposed method names and objects. This was originally brought up in #5510 (comment), and we filed w3c/IntersectionObserver#427 for it.

But yeah, until that gets addressed I agree with making notes in the spec that explain what we're trying to do, and pointing to that issue to track the IO refactoring. For now I've added one around using the IntersectionObserver constructor, and I'll add more around the other parts @domenic mentioned.

I've added some more <p class="XXX">'s describing that dom-IntersectionObserver* references should use the original values etc. I think this is ready for review

Thanks, @domenic! I've addressed your comments.

Follow-up cleanup tasks will be:

• Fix Setting loading=eager should be less COMEFROM-ey #5547
• Fix Define referenceable algorithms for IntersectionObserver methods w3c/IntersectionObserver#427 and fix the "XXX" issues introduced in this PR.

@domenic can you do the honors? 🙂