Open
Bug 1434313
Opened 7 years ago
Updated 2 years ago
Full screenshot captures document element, but should include initial viewport dimensions
Categories
(Remote Protocol :: Marionette, defect, P3)
Remote Protocol
Marionette
Tracking
(Not tracked)
REOPENED
People
(Reporter: code_mozilla, Unassigned)
References
Details
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36
Steps to reproduce:
1. use selenium
2. use a patched geckodriver (https://github.com/mguentner/geckodriver/commit/df37bf4d1837fb6b049f0a1819fadbb59bf4d0ea)
3. take a screenshot of a page with an absolute element *under* the bounding box of <html></html>
which means that it's outside the dimensions of win.document.documentElement
Example: https://gist.github.com/mguentner/c975c0d1aa12b1c6a846d615fb292484
4: take a screenshot of the same page using
$ firefox -headless -screenshot
-> I know this usecase is *currently* not supported but if https://bugzilla.mozilla.org/show_bug.cgi?id=1431148 makes progress, this will become an issue.
Actual results:
3: The screenshot obtained through selenium / webdriver / marionette only includes the <html>, but not the <div> that is "position: absolute" under it.
Relevant Code:
https://hg.mozilla.org/mozilla-central/file/9746e0a0a81c/testing/marionette/driver.js#l2865
4: the screenshot includes everything
Album: https://imgur.com/a/9OBXS
Expected results:
steps 3 and 4 produce identical results.
Comment 1•7 years ago
|
||
As far as I can tell from the screenshot attached generated using
"firefox -headless -screenshot", it takes a screenshot of the viewport.
If the document element expands beyond the viewport, is that included?
If that is the case it means that the -screenshot flag uses the
dimensions of the initial viewport as the minimum capture area and
that it grows beyond that. That’s not the behaviour currently
defined in Marionette, so I’m not sure why you think that will
be an issue when bug 1431148 gets implemented.
Conversely, you will have the same problem as you describe above if
the absolutely positioned element falls below the document element
as it expands beyond the bottom edge of the viewport because it
will not be visible. Maybe this is intended, but it seems like
what you are actually asking for here is _a change_ to the way
Marionette takes full document screenshots.
Updated•7 years ago
|
OS: Unspecified → All
Hardware: Unspecified → All
Summary: marionette → Full screenshot captures document element, but should include initial viewport dimensions
Reporter | ||
Comment 2•7 years ago
|
||
Thank you for clarifying :).
Basically, I am asking that the marionette command produces the same pixels as `firefox -headless -screenshot`
I think that when taking a "fullscreen" screenshot, using `win.document.documentElement` is wrong.
This can be demonstrated using the inspector using the above example html: https://imgur.com/a/7ZlxR
There are some (poorly) written pages where the content is "position: absolute; top: x" where x is
where the header (navbar, breadcrumbs etc.) stops vertically.
Taking a screenshot of such a page using the selector `win.document.documentElement` results in just
the header being included in the screenshot but not the actual content.
In the above html example, the green element would be the header and the red element the content.
Also, I just tried to move the absolute div outside the viewport by changing the top value from 200 to 2200.
`firefox -headless -screenshot` produces this (this is what I expect from marionette when bug 1431148 is implemented)
https://imgur.com/a/RjioK
As for the actual code in firefox, I am completely new to the code base. The closest thing I found is this:
https://hg.mozilla.org/mozilla-central/file/9746e0a0a81c/gfx/wrench/src/main.rs#l590
Comment 3•7 years ago
|
||
I tend to sympathise we should have the same behaviour as -screenshot
in Marionette.
The implementation of -screenshot in Firefox can be found here:
https://searchfox.org/mozilla-central/rev/97cb0aa64ae51adcabff76fb3b5eb18368f5f8ab/browser/components/shell/HeadlessShell.jsm#141-162
They appear to use these dimensions:
> let width = fullWidth ? contentWindow.innerWidth + contentWindow.scrollMaxX - contentWindow.scrollMinX
> : contentWindow.innerWidth;
> let height = fullHeight ? contentWindow.innerHeight + contentWindow.scrollMaxY - contentWindow.scrollMinY
> : contentWindow.innerHeight;
fullWidth and fullHeight are booleans, which if true (default),
will select the content window’s innerWidth/innerHeight +
scrollMaxX/scrollMaxY - scrollMinX/scrollMinY. That seems reasonable
to me.
Since taking a full document screenshot is not a WebDriver defined
concept we can change the semantics of WebDriver:TakeScreenshot
when passed {full: true} without ramifications.
That said, the command is awfully overloaded and we might want to
review it and split it up into separate commands. Whether that is
done as part of this bug or separately I don’t particularly have
an opinion about.
Reporter | ||
Comment 4•7 years ago
|
||
I have started to write with a patch, however scrollM{in,ax}{X,Y} return 0 for the `win`
obtained through `getCurrentWindow()`.
Any ideas why that might be, or how to get the scroll context for a ChromeWindow?
Comment 5•7 years ago
|
||
On further investigation it looks like the getWindowDimensions function
in devtools/shared/layout/utils.js might do exactly what we want [1].
WindowProxy.{scrollMinY,scrollMinX} appear to only be exposed
to chrome [2] which makes it tricky to get at these values from
the content script. I wonder though if it might not be possible
to capture the screenshot of the content window entirely from
chrome space. It looks like it might be possible.
This would entail a lot more work than just updating the algorithm
for what the full document is, however.
[1] https://searchfox.org/mozilla-central/rev/f41017f5cd7b5f3835d282fbe011d2899574fc2b/devtools/shared/layout/utils.js#661-685
[2] https://searchfox.org/mozilla-central/source/dom/webidl/Window.webidl#265-268
Updated•7 years ago
|
Priority: -- → P3
Comment 6•5 years ago
|
||
See also https://bugzilla.mozilla.org/show_bug.cgi?id=1559592 about
drawWindow
going away.
See Also: → 1559592
Updated•5 years ago
|
Status: UNCONFIRMED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Comment 8•5 years ago
|
||
I think duping this bug was a mistake. Reopening for now.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: DUPLICATE → ---
Updated•2 years ago
|
Severity: normal → S3
Updated•2 years ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•