report: add full-page-screenshot to experimental config

[connorjclark]

demo

Summary

New gatherer: full-page-screenshot changes the screen emulation to be as large as the content height and snaps a JPEG, backing off as necessary to meet size thresholds. After: if Lighthouse is in control of the emulation, we easily set it back to the emulated state. Otherwise, if emulation is outside of our control, we do our best to put it back. In practice this probably doesn't matter, since this is the afterPass of the very last gatherer, and we'd expect any programatic usage of Lighthouse to not reuse the page after the LH run.

New details: for the node detail type, an optional boundingRect represents the location of the node on the screen.

New audit: full-page-screenshot, simply exposes the FullPageScreenshot artifact.

New renderer: element-screenshot-renderer, for node details with a boundingRect, display a clipped image of the full page screenshot. On click, use the same renderer to paint the clipped image in a larger viewport in an overlay. The overlay is enabled in report-ui-features.

---

old

(before many refactors)

emulation

The previous PR wired emulation overrides through the driver + emulation files. Instead, I moved the presets out of emulation.js and into driver.js, meaning the overrides only go to Driver.beginEmulation. I think moving presets out of emulation.js makes the file better encapsulated.

on click

currently if you click on a node image you'll get a shadowbox overlay over the entire report. I see there is some code for instead showing the image in the corner of the page. I suppose we could also do a tooltip hover thing. We should sort out exactly what we want to do.

size

I added a script json-sizes so we can get a feel for how much larger this will make the LHR.

cat lhr.json | jq .audits | node lighthouse-core/scripts/json-size.js

theverge.com:

total                              100	897127
full-page-screenshot               38	337865
network-requests                   27	239008
screenshot-thumbnails              11	96481
final-screenshot                   4	37873
uses-long-cache-ttl                4	36034
tap-targets                        2	13667
network-server-latency             1	13084
network-rtt                        1	10443
third-party-summary                1	10294
uses-rel-preconnect                1	8864
bootup-time                        1	6958
unused-javascript                  1	6323
user-timings                       0	3702
...snip...

example.com:

total                              100	155237
screenshot-thumbnails              43	66273
final-screenshot                   10	15533
full-page-screenshot               9	13700
resource-summary                   1	1278
metrics                            1	1231
network-requests                   1	1210
mainthread-work-breakdown          1	1069
dom-size                           1	1006
html-has-lang                      1	969
critical-request-chains            1	946
bootup-time                        1	848
font-size                          0	739
...snip...

---

Cont: #8797

Note: do not use "land when green". Add Co-authored-by: Matt Zeunert <[email protected]> to commit body.

[paulirish]

spent some time in the renderer. in general it's dealing with the complexity quite well. :)

the var names make reading it a little tough so i have a few suggestions below that should help (and shouldn't be too contentious)

i'm also gonna do a followup PR with a slightly more contentious naming tweak

[paulirish]

I believe we can simplify this, but I'll attempt it in a followup.

[paulirish]

on my 2dpr screen i see a 1px edge on the right where the mask doesn't extend. not sure why, though.

not a blocker for shipping, obv.

[paulirish]

since this method is clearly working in screenshot coordinate scale, can we drop InScreenshotCoords from all the var names within?

[patrickhulce]

I would like to push back a little on the idea that anything anywhere here is clearly working in any particular scale :)

it's definitely clearer what's happening within this function but in absence of comments getScreenshotPositions could easily be a name for a function that returns somethingScreenshotCords based on somethingDisplayCoords.

in my experience variable names are less easily ignored when reading a function to figure out what it's doing than function overview. I'm ok with removing the suffix, just didn't want it to die without a fight :)

[paulirish]

aiight well if you're already feelin this, lemme pitch you on what I was going to put in a separate PR. basically abbreviating all the suffixes:

• elementRectInScreenshotCoords => elementRect_SC
• maxRenderSizeInDisplayCoords => maxRenderSize_DC

On my first pass looking at this renderer code i was really confused because the var names suffix was "Coords" but their types were Rect/Size. But then realized the different coords spaces and i'm pleased the names indicate this signal. But still, it just takes up so much visual space that I think it current hampers readability.

So yeah. suffix abbreviation. Connor's on board. so if you're into it, we could just do this.

edit: (Oh last bit... I already wrote a lil description for the top.. and we'd add in the abbv's in thar:

/**
 * @overview These functions define {Rect}s and {Size}s using two different coordinate spaces:
 *   1. Screenshot coords (SC): where 0,0 is the top left of the screenshot image
 *   2. Display coords (DC): that match the CSS pixel coordinate space of the LH report's page.
 */

)

[patrickhulce]

this plus the fileoverview explaining the two coordinate spaces (and what the abbreviation means) SGTM 👍

[brendankenny]

Just no underscores, please :)

[connorjclark]

So yeah. suffix abbreviation. Connor's on board. so if you're into it, we could just do this.

plz do this in followup pr

addressed review by moving to experimental config