core(fps): use observed metrics for screenshot dimensions #14418
Conversation
| width: document.documentElement.clientWidth, | ||
| height: document.documentElement.clientHeight, | ||
| width: window.outerWidth, | ||
| height: window.outerHeight, |
There was a problem hiding this comment.
Creating the smoke test helped me discover that document.documentElement.client* does not represent the actual device size.
| screenshot: { | ||
| height: 1742, | ||
| width: 980, | ||
| }, |
There was a problem hiding this comment.
these exact value display raw confidence :P
hopefully we can be this exact in all environments
| ViewportDimensions: { | ||
| innerWidth: 980, | ||
| // This value can vary slightly, depending on the display. | ||
| // https://bugs.chromium.org/p/chromium/issues/detail?id=1346355 |
|
from referenced issue mobile: (is the blurriness expected?) mobile, but with desktop: cnn.com looks good. |
I assume so. The actual text is tiny and then we add compression on top of that. |
| const documentSize = | ||
| await context.driver.executionContext.evaluate(getDocumentSize, { | ||
| const screenshotAreaSize = | ||
| await context.driver.executionContext.evaluate(getScreenshotAreaSize, { |
There was a problem hiding this comment.
This should usually be the same as the document size. In some cases (e.g. infinite scroll) the document element could grow larger than the viewport after we expand the viewport size.
There was a problem hiding this comment.
This sounds like a good comment to add.
3 participants
Closes #14415
The device metric dimensions only match the element bounding boxes if the page scale is 1. This PR takes the dimensions of the document element after the viewport is expanded to ensure the element bounding boxes line up with the screenshot.