full-page-screenshot: handle elements beyond MAX_SCREENSHOT_HEIGHT

[paulirish]

ref #10716 (comment) and #11118

i made https://output.jsbin.com/koqamuh/4/quiet as a test page for us. it has a (CSS) pixel ruler running down the side.

Here's a report of the page: https://marked-cable.surge.sh/color-contrast-over16000px-koqamuh.html

The elem screenshots start failing between 6200 and 6300 px

---

DPR stuff..

6300 is roughly 16384 / 2.625.. so it's to do with some DPR adjustments.. (I'm capturing this on my imacpro with a native dpr of 2.)

FWIW i tried removing the / dpr in maxScreenshotHeight:

lighthouse/lighthouse-core/gather/gatherers/full-page-screenshot.js

Line 75 in 06d3e1b

const maxScreenshotHeight = Math.floor(MAX_SCREENSHOT_HEIGHT / deviceScaleFactor);

and after doing that, it cut out at 16300 . (right before 16384, the real max)

that much seems good.
of course, the placement of the markers are off because of some other dpr math I didnt account for.

[connorjclark]

that much seems good.

I did the same, and I noticed that while yes, the highlight now shows for >6200px, the image itself is repeating the content starting at 6200px. I tried tweaking multiple parameters in the screenshot protocol method, but nothing resulted in an image that didn't repeat (w/o tweaking the DPR, which as you pointed out, we probably don't want to do). I think this may be a limitation of the protocol implementation.

It seems the original behavior is what we want, except for in the report, we should simply not show the element image if out of bounds of screenshot?

[patrickhulce]

w/o tweaking the DPR, which as you pointed out, we probably don't want to do

I'm still a little unclear why this is off limits but making a page with a 16000px tall scroll window is fine 🤔 (If it doesn't help solve this problem at all, probably not worth discussing but it sounds like it might help based on both of your findings)

In my limited experience, infinite scroll mutations and visibility observers that mutate are far more common than fundamental changes to the page based on DPR-specific art direction media queries.

The most likely repercussion of a DPR change to me seems to be a lower resolution image being swapped in, which would likely look pretty similar on a super low quality JPEG screenshot like ours even if our logic manages to wait for the new one to load in off the network.

[paulirish]

I believe this is fixed after #11538 and #11688. cc @connorjclark for confirmation