fix(screenshot): don't show viewport size overlay #6824
Conversation
* Add test for a clip screenshot.
* Hide size overlay before screenshot.
src/common/Page.ts
Outdated
| @@ -1675,6 +1675,11 @@ export class Page extends EventEmitter { | |||
| }); | |||
| let clip = options.clip ? processClip(options.clip) : undefined; | |||
|
|
|||
| // Not hiding the size overlay will cause unwilling artefacts on the screenshot. | |||
| await this._client.send('Overlay.setShowViewportSizeOnResize', { | |||
There was a problem hiding this comment.
Probably we need to set it back to the original value afterwards? Btw, have you found why the viewport is being resized when you take a screenshot?
There was a problem hiding this comment.
Probably we need to set it back to the original value afterwards?
I initially thought this too, but then couldn't immediately think of any reason a Puppeteer script would need the overlays to render. Are you thinking of headful scenarios where the Puppeteer scripts does some set up on the page and then hands it over to the user for some manual interaction?
There was a problem hiding this comment.
Hm, I am not sure. What is the use case for running puppeteer with devtools: true? can you manipulate DevTools window as well? A hypothetical scenario of a test: resizing a window & evaluating that viewport overlay is rendered. Also, for DevTools it'd be probably helpful to capture screenshots with overlays in e2e tests at some point.
There was a problem hiding this comment.
DevTools doesn't set it back, and there is no method to get the initial value.
There was a problem hiding this comment.
I see. So do you know why the page is resized when taking a screenshot?
There was a problem hiding this comment.
Hm, I am not sure. What is the use case for running puppeteer with devtools: true?
We've build a complete test runner around puppeteer over at https://github.com/boxine/pentf/ . To make test inspection as easy as possible for our developers we have a -d, --debug flag that opens up the browser in headful mode with devtools already opened, with persistent logging enabled and a bunch of other stuff. They love it because it saves them quite a few clicks. This file (especially the newPage function) with the puppeteer enhancements might be worth a look: https://github.com/boxine/pentf/blob/master/src/browser_utils.js
I'm using the same runner to test the devtools extensions for Preact (frontend framework) https://github.com/preactjs/preact-devtools/ . Currently I'm faking the extension inside an iframe, but the long term goal is to test the extension pages inside the devtools panel directly. This would properly test it like the user would experience it and would allow me to verify things like "is my extension tab showing up?", syncing extension settings, etc.
|
Please try optimizing |
Co-authored-by: Mathias Bynens <mathias@qiwi.be>
It's not ready yet. I added reviewers too early, sorry. |
* Delete obsolete file.
It was optimised, but at the end there is no need in the file at all. |
mathiasbynens
force-pushed
the
main
branch
4 times, most recently
from
03a4ca1
to
d4b17bd
Compare
|
@sadym-chromium just a gentle ping to see if this is still relevant. If not, feel free to close this. Otherwise, let's get this landed :) |
|
As described in the comment, the suggested solution wouldn't work. Closing. |
Devtools resize viewport dimensions overlay included in screenshotsdeviceScaleFactoron the headful fullscreen screenshot #6823).