Fixed web elements at the bottom of the page shouldn't be rendered in the middle of a full page screenshot
Categories
(Core :: Graphics, enhancement, P3)
Tracking
()
People
(Reporter: whimboo, Unassigned)
References
Details
This request has been brought up via bug 1587845 which is going to implement full page screenshots for the Remote Protocol (our implementation of chrome dev-tools protocol).
It was also recently fixed in Chrome. See details at:
https://github.com/puppeteer/puppeteer/issues/5080
(In reply to Matt Woodrow (:mattwoodrow) from bug 1587845 comment #5)
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #3)
Here a sample full screenshot: https://snipboard.io/j6utLs.jpg
Matt, there is a cookie banner which is fixed at the bottom of the page. But in the full screenshot it appears at the bottom of the currently visible viewport. I assume this is a misrendering which needs to be fixed in drawSnapshot?
The current code for taking a full page screenshot is a bit of a hack, where it still has the normal viewport, we just don't clip to that rectangle (and omit the scrollbars).
If you want banners that are attached to the bottom edge of the viewport to be at the bottom of the page, then we'd have to re-layout using the page size as the viewport (and then un-do it since we don't want that layout to make it to the screen).
That's a fair big bit of new functionality to implement, so if it's needed, we should try schedule a meeting with the layout team in Berlin.
As Matt proposed a meeting would be great. Emilio, is it something you could take on?
|
||
Comment 1•5 years ago
|
||
We have some similar code for WebExtension popups, where we use getContentSizeConstrained to end up in PresShell::ResizeReflow here. I think getContentSize would do what you want?
There's the question of what happens with viewport units and such, I think they'd be pretty broken... How should viewport units / viewport media queries behave for this kind of stuff?
Can you point me to the current code we have for screenshots and such? Which APIs does it use?
|
Reporter | |
Comment 2•5 years ago
|
||
The content size is just fine and I have the exact area to screenshot. Retrieving the right values was done on bug 1588622 by the help of Botond.
The problem here is more the rendering. See my example screenshot from above (https://snipboard.io/j6utLs.jpg) which has this cookie banner shown at the very top - aligned with the current viewport height but not the bottom of the page. For screenshots I use the new drawSnapshot API, and full screen captures I currently implement on bug 1587845. I could have a WiP uploaded if you want in a couple of hours.
|
|
||
Comment 3•5 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)
The content size is just fine and I have the exact area to screenshot. Retrieving the right values was done on bug 1588622 by the help of Botond.
Well, sure, but for fixed pos elements you need to change the layout viewport size. So in theory if you change it you need to also change media queries and viewport units.
|
||
Updated•5 years ago
|
|
|
Reporter | |
Comment 4•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #2)
The content size is just fine and I have the exact area to screenshot. Retrieving the right values was done on bug 1588622 by the help of Botond.
Well, sure, but for fixed pos elements you need to change the layout viewport size. So in theory if you change it you need to also change media queries and viewport units.
So how would I change the layout viewport size? If there is no platform change necessary, and I just can do it via Javascript it would basically be done via bug 1544417 (Emulation.setDeviceMetricsOverride) and we could dupe this bug. Or would platform work still be necessary?
|
|
||
Comment 5•5 years ago
|
||
So how would I change the layout viewport size? If there is no platform change necessary, and I just can do it via Javascript it would basically be done via bug 1544417 (Emulation.setDeviceMetricsOverride) and we could dupe this bug. Or would platform work still be necessary?
Well depends on what Emulation.setDeviceMetricsOverride ends up doing. Where is that implemented?
|
|
Reporter | |
Comment 6•5 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)
Well depends on what
Emulation.setDeviceMetricsOverrideends up doing. Where is that implemented?
It's not implemented yet, but something I currently work on. So for now I got somewhat working by resizing the browser element like:
+ browser.style.setProperty("min-width", width + "px");
+ browser.style.setProperty("max-width", width + "px");
For the height it is similar. How would this be related to the media queries you mentioned above? What else would I have to do? Thanks.
|
|
||
Comment 7•5 years ago
|
||
Well, that will change how viewport units and media queries evaluate. Note that it will not change how device-width media queries evaluate though...
|
|
Reporter | |
Comment 8•5 years ago
|
||
With the landed patch on bug 1587845 this actually works perfectly now. After resizing the browser Puppeteer reloads the page, and uses the new height of the content as browser height. With that step absolutely positioned elements at the end of the page will be captured exactly at that location.
Not sure if it would make sense to also make it work for the screenshot command itself, so that eg. the command line argument --screenshot would also render those elements at the bottom of the page.
|
||
Updated•2 years ago
|
Description
•