Closed Bug 1626428 Opened 5 years ago Closed 5 years ago

Replace IntersectionObserver lazy loading with native <img loading="lazy">

Categories

(Firefox :: New Tab Page, task, P2)

task

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mconley, Unassigned)

References

(Blocks 1 open bug)

Details

Bug 1542784 added support for <img loading="lazy">, which effectively does what the IntersectionObserver added in bug 1531933, bug 1573206 (and maybe other places) does.

We should see if we can swap out the IntersectionObserver code with the native lazy loading stuff. This will probably improve maintainability of the Discovery Stream code, and also makes it so that the cached about:home document generated for bug 1614351 includes the Pocket cards (otherwise, since the ReactDOMServer's notion of the page never gets "seen", the cards don't render to string).

<img loading="lazy"> support in Firefox is going out to the web in Firefox 75 in a week, so this is probably safe to land in Firefox 76 or 77.

Hey Scott, are you interested in trying this out? I could do it myself, but I suspect you could probably do it faster.

Once a patch is thrown together, we should probably ensure that it doesn't result in any regressions in our about:newtab loading times in Talos, and however else your team might be measuring that.

I think all you need to do is set loading="lazy" on each of the Discovery Stream Pocket images, and they'll get the lazy loading behaviour. See https://mathiasbynens.be/demo/img-loading-lazy for a demo.

Flags: needinfo?(sdowne)

It's worth looking into, but it's probably also worth mentioning there is more to newtab loading than just lazy images.

We use a combination of requestIdleCallback and intersection observer to do a bunch of overlapping this to try to mitigate this, not all related to images.

  1. Images above the fold are loaded immediately, and not lazily, I suspect loading="lazy" covers this, but this is what screen recordings would validate.
  2. We also load some below the fold react components lazily, using intersections observer and requestIdleCallback, which ever comes first. It's just prioritizing react to render the important bits first. We saw some load improvements with this, and was needed to initially get the last few m.s we needed to get in the green.
  3. Below the fold images within 540px of the scroll range start loading. This is behind the requestIdleCallback or intersectionObserver from step 2.
  4. If they are still scrolled to before loaded, we use a tiny transition to make it look less jarring.
  5. The requestIdleCallback on images is probably removable, and unnecessary in the current implementation.

Give the above cases, I propose:
Remove 5, it's probably dead code now.
Test/record lazy for support for 1, 3 and 4, and possibly move some/all functionality into 2.
In the end I think we'll reduce complexity in our image component a lot, and just have the lazy in there. Increase the complexity in card component a bit, but I think we'll have a net positive complexity reduction between the two components, and maybe a tiny performance improvement? If lazy is more efficient than setting up intersection observers/idles.

Flags: needinfo?(sdowne)
Priority: -- → P2

I've got a patch up to switch the <img> elements to native lazy loading in bug 1626721. We don't have anything similar at the native layer for non-images, so I guess we'll keep what we've got for the DSCard components. In bug 1614502, I've made it so that we fully render the cards if we're generating the cache, so I don't think we need this bug anymore.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.