Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect width and height of full page screenshot #14824

Closed
2 tasks done
kraten opened this issue Feb 23, 2023 · 3 comments · Fixed by #14920
Closed
2 tasks done

Incorrect width and height of full page screenshot #14824

kraten opened this issue Feb 23, 2023 · 3 comments · Fixed by #14920
Assignees
Labels

Comments

@kraten
Copy link
Contributor

kraten commented Feb 23, 2023

FAQ

URL

https://wp-rocket.me

What happened?

I generated a JSON report for this URL: https://wp-rocket.me

From the JSON report, I extracted the webp image buffer and converted it to a file.

While I was checking the file attributes, I noticed the width and height mentioned in the report are different compared to what the actual size is.

Actual Size: 412 × 8795 pixels
Size mentioned in report: 417 x 8902 pixels

What did you expect?

I expect the full-page screenshot image buffer to be the same size as mentioned in the report.

What have you tried?

I found this issue only for this particular URL. All other URLs I tried are working fine.
Also, I'm getting the correct width and height with lighthouse 9.6.6.

How were you running Lighthouse?

CLI, node

Lighthouse Version

10.0.1

Chrome Version

109.0.5414.119 (Official Build) (arm64)

Node Version

16.18.1

OS

Mac

Relevant log output

$ lighthouse https://wp-rocket.me/ --output json --output-path ./test.json
  LH:ChromeLauncher Waiting for browser. +0ms
  LH:ChromeLauncher Waiting for browser... +0ms
  LH:ChromeLauncher Waiting for browser..... +503ms
  LH:ChromeLauncher Waiting for browser.....✓ +1ms
  LH:status Connecting to browser +429ms
  LH:status Navigating to about:blank +5ms
  LH:status Benchmarking machine +14ms
  LH:status Preparing target for navigation mode +1s
  LH:status Navigating to about:blank +7ms
  LH:status Preparing target for navigation +6ms
  LH:status Cleaning origin data +21ms
  LH:status Cleaning browser cache +13ms
  LH:status Preparing network conditions +18ms
  LH:status Navigating to https://wp-rocket.me/ +39ms
  LH:artifacts:getArtifact DevtoolsLog +4s
  LH:artifacts:getArtifact Trace +0ms
  LH:artifacts:getArtifact DevtoolsLog +1ms
  LH:artifacts:getArtifact Trace +0ms
  LH:artifacts:getArtifact Accessibility +0ms
  LH:artifacts:getArtifact AnchorElements +229ms
  LH:artifacts:getArtifact ConsoleMessages +19ms
  LH:artifacts:getArtifact CSSUsage +1ms
  LH:artifacts:getArtifact Doctype +17ms
  LH:artifacts:getArtifact DOMStats +1ms
  LH:artifacts:getArtifact EmbeddedContent +3ms
  LH:artifacts:getArtifact FontSize +1ms
  LH:artifacts:getArtifact Inputs +17ms
  LH:artifacts:getArtifact GlobalListeners +2ms
  LH:artifacts:getArtifact ImageElements +1ms
  LH:artifacts:getArtifact InstallabilityErrors +223ms
  LH:status Get webapp installability errors +0ms
  LH:artifacts:getArtifact InspectorIssues +0ms
  LH:artifacts:getArtifact JsUsage +1ms
  LH:artifacts:getArtifact LinkElements +0ms
  LH:artifacts:getArtifact MainDocumentContent +3ms
  LH:artifacts:getArtifact MetaElements +2ms
  LH:artifacts:getArtifact NetworkUserAgent +3ms
  LH:artifacts:getArtifact OptimizedImages +0ms
  LH:artifacts:getArtifact ResponseCompression +0ms
  LH:artifacts:getArtifact RobotsTxt +0ms
  LH:artifacts:getArtifact ServiceWorker +184ms
  LH:artifacts:getArtifact Scripts +3ms
  LH:artifacts:getArtifact SourceMaps +0ms
  LH:artifacts:getArtifact Stacks +0ms
  LH:status Collect stacks +0ms
  LH:artifacts:getArtifact TagsBlockingFirstPaint +14ms
  LH:artifacts:getArtifact TapTargets +1ms
  LH:artifacts:getArtifact TraceElements +12ms
  LH:artifacts:getArtifact ViewportDimensions +34ms
  LH:artifacts:getArtifact WebAppManifest +1ms
  LH:status Get webapp manifest +0ms
  LH:artifacts:getArtifact devtoolsLogs +1ms
  LH:artifacts:getArtifact traces +1ms
  LH:artifacts:getArtifact FullPageScreenshot +0ms
  LH:artifacts:getArtifact BFCacheFailures +1s
  LH:status Analyzing and running audits... +256ms
  LH:status Auditing: Uses HTTPS +1ms
  LH:status Auditing: Registers a service worker that controls page and `start_url` +0ms
  LH:status Auditing: Has a `<meta name="viewport">` tag with `width` or `initial-scale` +0ms
  LH:status Auditing: First Contentful Paint +1ms
  LH:status Auditing: Largest Contentful Paint +6ms
  LH:status Auditing: First Meaningful Paint +2ms
  LH:status Auditing: Speed Index +1ms
  LH:status Auditing: Screenshot Thumbnails +163ms
  LH:status Auditing: Final Screenshot +1ms
  LH:status Auditing: Total Blocking Time +0ms
  LH:status Auditing: Max Potential First Input Delay +2ms
  LH:status Auditing: Cumulative Layout Shift +2ms
  LH:status Auditing: No browser errors logged to the console +1ms
  LH:status Auditing: Initial server response time was short +0ms
  LH:status Auditing: Time to Interactive +0ms
  LH:status Auditing: User Timing marks and measures +1ms
  LH:status Auditing: Avoid chaining critical requests +0ms
  LH:status Auditing: Avoid multiple page redirects +1ms
  LH:status Auditing: Web app manifest and service worker meet the installability requirements +0ms
  LH:status Auditing: Configured for a custom splash screen +1ms
  LH:status Auditing: Sets a theme color for the address bar. +1ms
  LH:status Auditing: Manifest has a maskable icon +0ms
  LH:status Auditing: Content is sized correctly for the viewport +0ms
  LH:status Auditing: Displays images with correct aspect ratio +1ms
  LH:status Auditing: Serves images with appropriate resolution +3ms
  LH:status Auditing: Fonts with `font-display: optional` are preloaded +0ms
  LH:status Auditing: Avoids deprecated APIs +0ms
  LH:status Auditing: Minimizes main-thread work +1ms
  LH:status Auditing: JavaScript execution time +3ms
  LH:status Auditing: Preload key requests +1ms
  LH:status Auditing: Preconnect to required origins +0ms
  LH:status Auditing: All text remains visible during webfont loads +0ms
  LH:status Auditing: Diagnostics +1ms
  LH:status Auditing: Network Requests +0ms
  LH:status Auditing: Network Round Trip Times +1ms
  LH:status Auditing: Server Backend Latencies +0ms
  LH:status Auditing: Tasks +0ms
  LH:status Auditing: Metrics +0ms
  LH:status Auditing: Performance budget +1ms
  LH:status Auditing: Timing budget +1ms
  LH:status Auditing: Keep request counts low and transfer sizes small +0ms
  LH:status Auditing: Minimize third-party usage +0ms
  LH:status Auditing: Lazy load third-party resources with facades +1ms
  LH:status Auditing: Largest Contentful Paint element +0ms
  LH:status Auditing: Largest Contentful Paint image was not lazily loaded +1ms
  LH:status Auditing: Avoid large layout shifts +0ms
  LH:status Auditing: Avoid long main-thread tasks +0ms
  LH:status Auditing: Avoids `unload` event listeners +1ms
  LH:status Auditing: Avoid non-composited animations +0ms
  LH:status Auditing: Image elements have explicit `width` and `height` +0ms
  LH:status Auditing: Page has valid source maps +1ms
  LH:status Auditing: Preload Largest Contentful Paint image +0ms
  LH:status Auditing: Ensure CSP is effective against XSS attacks +0ms
  LH:status Auditing: Script Treemap Data +1ms
  LH:status Auditing: Site works cross-browser +0ms
  LH:status Auditing: Page transitions don't feel like they block on the network +1ms
  LH:status Auditing: Each page has a URL +0ms
  LH:status Auditing: `[accesskey]` values are unique +0ms
  LH:status Auditing: `[aria-*]` attributes match their roles +0ms
  LH:status Auditing: `button`, `link`, and `menuitem` elements have accessible names +0ms
  LH:status Auditing: `[aria-hidden="true"]` is not present on the document `<body>` +1ms
  LH:status Auditing: `[aria-hidden="true"]` elements do not contain focusable descendents +0ms
  LH:status Auditing: ARIA input fields have accessible names +0ms
  LH:status Auditing: ARIA `meter` elements have accessible names +0ms
  LH:status Auditing: ARIA `progressbar` elements have accessible names +0ms
  LH:status Auditing: `[role]`s have all required `[aria-*]` attributes +0ms
  LH:status Auditing: Elements with an ARIA `[role]` that require children to contain a specific `[role]` have all required children. +0ms
  LH:status Auditing: `[role]`s are contained by their required parent element +0ms
  LH:status Auditing: `[role]` values are valid +1ms
  LH:status Auditing: ARIA toggle fields have accessible names +0ms
  LH:status Auditing: ARIA `tooltip` elements have accessible names +0ms
  LH:status Auditing: ARIA `treeitem` elements have accessible names +0ms
  LH:status Auditing: `[aria-*]` attributes have valid values +0ms
  LH:status Auditing: `[aria-*]` attributes are valid and not misspelled +0ms
  LH:status Auditing: Buttons have an accessible name +0ms
  LH:status Auditing: The page contains a heading, skip link, or landmark region +1ms
  LH:status Auditing: Background and foreground colors have a sufficient contrast ratio +0ms
  LH:status Auditing: `<dl>`'s contain only properly-ordered `<dt>` and `<dd>` groups, `<script>`, `<template>` or `<div>` elements. +0ms
  LH:status Auditing: Definition list items are wrapped in `<dl>` elements +0ms
  LH:status Auditing: Document has a `<title>` element +0ms
  LH:status Auditing: `[id]` attributes on active, focusable elements are unique +0ms
  LH:status Auditing: ARIA IDs are unique +0ms
  LH:status Auditing: No form fields have multiple labels +1ms
  LH:status Auditing: `<frame>` or `<iframe>` elements have a title +0ms
  LH:status Auditing: Heading elements appear in a sequentially-descending order +0ms
  LH:status Auditing: `<html>` element has a `[lang]` attribute +0ms
  LH:status Auditing: `<html>` element has a valid value for its `[lang]` attribute +0ms
  LH:status Auditing: Image elements have `[alt]` attributes +0ms
  LH:status Auditing: `<input type="image">` elements have `[alt]` text +0ms
  LH:status Auditing: Form elements have associated labels +0ms
  LH:status Auditing: Links have a discernible name +0ms
  LH:status Auditing: Lists contain only `<li>` elements and script supporting elements (`<script>` and `<template>`). +1ms
  LH:status Auditing: List items (`<li>`) are contained within `<ul>`, `<ol>` or `<menu>` parent elements +0ms
  LH:status Auditing: The document does not use `<meta http-equiv="refresh">` +0ms
  LH:status Auditing: `[user-scalable="no"]` is not used in the `<meta name="viewport">` element and the `[maximum-scale]` attribute is not less than 5. +0ms
  LH:status Auditing: `<object>` elements have alternate text +0ms
  LH:status Auditing: No element has a `[tabindex]` value greater than 0 +0ms
  LH:status Auditing: Cells in a `<table>` element that use the `[headers]` attribute refer to table cells within the same table. +1ms
  LH:status Auditing: `<th>` elements and elements with `[role="columnheader"/"rowheader"]` have data cells they describe. +0ms
  LH:status Auditing: `[lang]` attributes have a valid value +0ms
  LH:status Auditing: `<video>` elements contain a `<track>` element with `[kind="captions"]` +0ms
  LH:status Auditing: Custom controls have associated labels +0ms
  LH:status Auditing: Custom controls have ARIA roles +0ms
  LH:status Auditing: User focus is not accidentally trapped in a region +0ms
  LH:status Auditing: Interactive controls are keyboard focusable +0ms
  LH:status Auditing: Interactive elements indicate their purpose and state +0ms
  LH:status Auditing: The page has a logical tab order +0ms
  LH:status Auditing: The user's focus is directed to new content added to the page +0ms
  LH:status Auditing: Offscreen content is hidden from assistive technology +0ms
  LH:status Auditing: HTML5 landmark elements are used to improve navigation +0ms
  LH:status Auditing: Visual order on the page follows DOM order +0ms
  LH:status Auditing: Uses efficient cache policy on static assets +0ms
  LH:status Auditing: Avoids enormous network payloads +1ms
  LH:status Auditing: Defer offscreen images +0ms
  LH:status Auditing: Eliminate render-blocking resources +1ms
  LH:status Auditing: Minify CSS +1ms
  LH:status Auditing: Minify JavaScript +5ms
  LH:status Auditing: Reduce unused CSS +2ms
  LH:status Auditing: Reduce unused JavaScript +1ms
  LH:status Auditing: Serve images in next-gen formats +1ms
  LH:status Auditing: Efficiently encode images +1ms
  LH:status Auditing: Enable text compression +1ms
  LH:status Auditing: Properly size images +1ms
  LH:status Auditing: Use video formats for animated content +3ms
  LH:status Auditing: Remove duplicate modules in JavaScript bundles +1ms
  LH:status Auditing: Avoid serving legacy JavaScript to modern browsers +1ms
  LH:status Auditing: Page has the HTML doctype +8ms
  LH:status Auditing: Properly defines charset +0ms
  LH:status Auditing: Avoids an excessive DOM size +0ms
  LH:status Auditing: Avoids requesting the geolocation permission on page load +1ms
  LH:status Auditing: No issues in the `Issues` panel in Chrome Devtools +0ms
  LH:status Auditing: Avoids `document.write()` +0ms
  LH:status Auditing: Detected JavaScript libraries +0ms
  LH:status Auditing: Avoids requesting the notification permission on page load +0ms
  LH:status Auditing: Allows users to paste into input fields +0ms
  LH:status Auditing: Use HTTP/2 +0ms
  LH:status Auditing: Uses passive listeners to improve scrolling performance +1ms
  LH:status Auditing: Document has a meta description +0ms
  LH:status Auditing: Page has successful HTTP status code +0ms
  LH:status Auditing: Document uses legible font sizes +0ms
  LH:status Auditing: Links have descriptive text +2ms
  LH:status Auditing: Links are crawlable +1ms
  LH:status Auditing: Page isn’t blocked from indexing +0ms
  LH:status Auditing: robots.txt is valid +1ms
  LH:status Auditing: Tap targets are sized appropriately +0ms
  LH:status Auditing: Document has a valid `hreflang` +0ms
  LH:status Auditing: Document avoids plugins +1ms
  LH:status Auditing: Document has a valid `rel=canonical` +0ms
  LH:status Auditing: Structured data is valid +0ms
  LH:status Auditing: Page didn't prevent back/forward cache restoration +0ms
  LH:status Generating results... +0ms
  LH:Printer json output written to ./test.json +11ms
  LH:ChromeLauncher Killing Chrome instance 74957 +0ms
@alexnj
Copy link
Member

alexnj commented Mar 22, 2023

Running a bisect identifies #14418 to be the change that caused a regression.

git bisect start HEAD v9.6.6 --
git bisect run node ../spikes/lighthouse-bisect-fullpagescreenshot/index.js

...
  LH:status Generating results... +0ms
This version is: Bad.
 Actual dimensions: 360x6988.
 Declared dimensions: 477x9259.

501133d70035dc1ea5b910efcd655abd2545d3c6 is the first bad commit
    core(fps): use observed metrics for screenshot dimensions (#14418)

bisect run cli here.

@alexnj
Copy link
Member

alexnj commented Mar 22, 2023

I spent some more time with it — I think our full page screenshot dimension calculations could be improved.

The reason why this specific site's audit is producing a difference between declared screenshot dimensions (that's in LHR) and actual dimensions (that of WebP image) is due to the page's horizontal overflow. Any page that has a horizontal overflow (a horizontal scrollbar for that matter) should be able to reproduce the inconsistency.

The root cause of this inconsistency comes from the way we calculate screenshot area. For a page with horizontal overflow, Chromium produces a window.innerWidth that's larger than window.outerWidth. Per spec, this seems correct behavior as window.innerWidth is the width of the layout viewport, i.e., what is available to view. However, prior to taking a a screenshot, we resize the viewable viewport to a width of zero, that retains the width of the viewport to the emulated screen width. This forces the screenshot to clip horizontal overflow, which also seems to be the correct behavior as we like to take the screenshot of what the user is actually experiencing. However, this introduces the discrepancy in dimensions, with a WebP image dimension that's smaller than declared in LHR.

Similarly, the height calculation is a scaled approximation between device height, content height and viewport client height. I haven't fully understood the reasoning behind this calculation, but it seems to produce enough shortage from actual content height in most cases and results in a screenshot that's shorter than full height of the page. As a side effect of this, all element screenshot coordinate calculations are off for the page.

FWIW, I propose that we simplify this calculations to the following:

// To obtain a full page screenshot, we resize the emulated viewport to
// (1) a height equal to the maximum of visual-viewport height and document height.
// (2) a width equal to emulated visual-viewport width (we choose to clip overflow on x-axis).
// Finally, we cap the viewport to a maximum size allowance of WebP format.
const fullHeight = Math.max(deviceMetrics.height, metrics.cssContentSize.height);
const height = Math.min(fullHeight, MAX_WEBP_SIZE);
const width = Math.min(deviceMetrics.width, MAX_WEBP_SIZE);

Once we do that, we could avoid reading window.innerWidth and rely on cssVisualViewport.client{width|height} returned by Page.getLayoutMetrics. The dimensions of what's captured and what's calculated as screenshot area should match at that point.

I'm testing this out in a branch and it seems to work, assuming our dpr and other corner case tests are exhaustive.

Thoughts? Are there cases where I might have had an oversight?

@adamraine
Copy link
Member

I haven't fully digested everything about horizontal overflow, but it does appear that the placement of our element bounding boxes are off if there is horizontal overflow. Your branch seems to fix this issue at least so I think it's close to a solution.

Similarly, the height calculation is a scaled approximation between device height, content height and viewport client height. I haven't fully understood the reasoning behind this calculation, but it seems to produce enough shortage from actual content height in most cases and results in a screenshot that's shorter than full height of the page. As a side effect of this, all element screenshot coordinate calculations are off for the page.

It's important to point out that deviceMetrics.height is not necessarily on the same scale as metrics.cssContentSize.height. The fullHeight calculation is incorrect for the fps-scaled smoke test in your branch.

This is the reasoning behind the current fullHeight calculation. We know how much we want to scale the window.innerHeight by, but we can only control the window.outerHeight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants