Closed Bug 1571659 Opened Last month Closed Last month

Simplify the code for taking full screenshots

Categories

(Testing :: Marionette, defect, P1)

Version 3
defect

Tracking

(firefox69 unaffected, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 --- unaffected
firefox70 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

Details

Attachments

(2 files)

As just noticed on bug 1571341, my patch from bug 1377335 caused a regression so that full screenshots don't work anymore. Any content outside of the current viewport actually gets clipped (and replaced with a gray area).

So there is a strange behavior I cannot get fixed with bug 1377335 still in the tree. As such I voted for a backout. Once we switched over to drawSnapshot() we can try again to get this working. See also bug 1571341 which is the blocker for that.

Instead I will use this bug to improve the screenshot logic for fullscreen, which is currently kinda confusing and hard to follow. Also I will add a comment along the broken code to cover the latest investigation.

Keywords: regression
No longer regressed by: 1377335
Summary: Capture code shouldn't set DRAWWINDOW_DRAW_VIEW flag for full screenshots → Simplify the code for taking full screenshots

The patch makes the code for handling screenshots in the driver and
listener module easier to understand. Also it sets the default values
for optional arguments once, which doesn't require duplicated code
for the listener anymore.

Further there was a bug which treated the "full" parameter with a
higher priority as an element, if both are defined.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6db420df3a35
[marionette] Simplify logic for fullscreen screenshots. r=webdriver-reviewers,ato
https://hg.mozilla.org/integration/autoland/rev/15243603aac5
[marionette] Explain why certain flags for drawWindow() cannot be used right now. r=webdriver-reviewers,jgraham
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
You need to log in before you can comment on or make changes to this bug.