search

HTTPArchive / legacy.httparchive.org

<<THIS REPOSITORY IS DEPRECATED>> The HTTP Archive provides information about website performance such as # of HTTP requests, use of gzip, and amount of JavaScript. This information is recorded over time revealing trends in how the Internet is performing. Built using Open Source software, the code and data are available to everyone allowing researchers large and small to work from a common base.
https://legacy.httparchive.org
Other
328 stars 84 forks source link

Updated Images custom metric to use IntersectionObserver #225

Closed kevinfarrugia closed 3 years ago

kevinfarrugia commented 3 years ago

Updated the Images custom metric inViewport property to use InteresctionObserver to improve reliability and support for:

Inspired by: https://github.com/WPO-Foundation/webpagetest-docs/pull/36

Using IntersectionObserver adds some more complexity because of its async nature, however I tried to keep the custom metric as similar to its previous implementation as possible.

Sample WPT https://webpagetest.org/custom_metrics.php?test=210716_AiDc99_be18dd00ed31c02ef0277471152b0fa8&run=1&cached=0

[
  {
    "url": "https://placekitten.com/304/304",
    "width": 304,
    "height": 304,
    "naturalWidth": 304,
    "naturalHeight": 304,
    "loading": "lazy",
    "decoding": null,
    "inViewport": true
  },
  {
    "url": "https://placekitten.com/305/305",
    "width": 305,
    "height": 305,
    "naturalWidth": 0,
    "naturalHeight": 0,
    "loading": "lazy",
    "decoding": null,
    "inViewport": false
  }
]
kevinfarrugia commented 3 years ago

Is there a guarantee from IntersectionObserver that all observed elements will be delivered in one callback? I have not studied the specification deep enough to be sure of that. If there's no guarantee, you might miss some images compared to the previous version.

Converted to draft until I am able to clarify/confirm this.

rviscomi commented 3 years ago

@kevinfarrugia any update on this?

kevinfarrugia commented 3 years ago

@rviscomi I had manually tested it on a number of pages and the results are correct. I couldn't find anything detailing whether all entries should be included in the first callback or not, so I suspect it may be up to the browser.

rviscomi commented 3 years ago

Ok thanks. Is this PR ready for review then? If so I'll try to get it reviewed and merged in time for the September crawl.

kevinfarrugia commented 3 years ago

Yes it is ready for review but I am wary of merging it until I have a definite confirmation on the behavior of IntersectionObserver.

rviscomi commented 3 years ago

Ok so let's leave this in draft until you confirm.

rviscomi commented 3 years ago

Closing for now. Feel free to reopen when ready.