1604506 - Implement print command in marionette

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, task)

Description

[James Graham [:jgraham]]

This will implement the WebDriver print-to-pdf command in marionette, following the proposed WebDriver spec.

Comment 1

[James Graham [:jgraham]]

Attachment: Bug 1604506 - Add assert.positiveNumber to marionette,

Comment 2

[James Graham [:jgraham]]

Attachment: Bug 1604506 - Add print command to marionette,

Add a WebDriver:Print command to marionette, following the proposed
WebDriver spec at https://github.com/w3c/webdriver/pull/1468
The implementation is largely the same as that added to the
remote agent in Bug 1599994.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29c893010729
Add assert.positiveNumber to marionette, r=marionette-reviewers,whimboo,ato
https://hg.mozilla.org/integration/autoland/rev/531ee079b538
Add print command to marionette, r=marionette-reviewers,whimboo,ato

Comment 4

[Raul Gurzau (:RaulG)]

Backed out 4 changesets (bug 1604506, bug 1602547, bug 1602544) for failing at /printcmd.py and /user_prompts.py

Backout link: https://hg.mozilla.org/integration/autoland/rev/70c7a9fa71304413d9acaa4eabe0df85d87f7522

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=cad798d12930ed716fd860af93deeeddad297019&selectedJob=281838694

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=281838694&repo=autoland&lineNumber=111493

Log snippet:
[task 2019-12-19T02:33:32.556Z] 02:33:32 INFO - PID 3138 | 1576722812520 webdriver::server DEBUG <- 200 OK {"value":null}
[task 2019-12-19T02:33:32.556Z] 02:33:32 INFO - PID 3138 | 1576722812522 webdriver::server DEBUG -> POST /session/aaeb77b3-abc3-d84d-8598-56859e8f39d4/moz/print {}
[task 2019-12-19T02:33:32.559Z] 02:33:32 INFO - PID 3138 | 1576722812531 Marionette DEBUG 0 -> [0,31,"WebDriver:Print",{"background":false,"margin":{"bottom":1,"left":1,"right":1,"top":1},"orientation":"portrait","page":{"height":27.94,"width":21.59},"pageRanges":[],"scale":1,"shrinkToFit":true}]
[task 2019-12-19T02:33:59.226Z] 02:33:59 INFO - TEST-UNEXPECTED-TIMEOUT | /_mozilla/webdriver/print/printcmd.py | expected OK
[task 2019-12-19T02:33:59.226Z] 02:33:59 INFO - TEST-INFO took 30118ms
[task 2019-12-19T02:33:59.374Z] 02:33:59 INFO - STDOUT: FAILED
[task 2019-12-19T02:33:59.374Z] 02:33:59 INFO - Closing logging queue
[task 2019-12-19T02:33:59.374Z] 02:33:59 INFO - queue closed
[task 2019-12-19T02:33:59.374Z] 02:33:59 INFO - Starting runner
[task 2019-12-19T02:33:59.407Z] 02:33:59 INFO - PID 3146 | 1576722839391 geckodriver DEBUG Listening on 127.0.0.1:58574
[task 2019-12-19T02:33:59.886Z] 02:33:59 INFO - WebDriver HTTP server listening at http://127.0.0.1:58574

Comment 5

[James Graham [:jgraham]]

Turns out that kPaperSizeMillimeters resulted in a too-large surface on Mac, so I have a suspicion that it's using inches for the paper size there regardless. Updated the patch to just use Inches always, verified locally we get sensible output and try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b216c2acab8a0fff61635996362c3e183f15409

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to James Graham [:jgraham] from comment #5)

Turns out that kPaperSizeMillimeters resulted in a too-large surface on Mac, so I have a suspicion that it's using inches for the paper size there regardless. Updated the patch to just use Inches always, verified locally we get sensible output and try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b216c2acab8a0fff61635996362c3e183f15409

Jonathan, do you have any idea why this is happening on Mac? Maybe it's a bug which we could cover with your rewrite of the printing code?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3ea83c0bad8
Add assert.positiveNumber to marionette, r=marionette-reviewers,whimboo,ato
https://hg.mozilla.org/integration/autoland/rev/d684a6d51c8a
Add print command to marionette, r=marionette-reviewers,whimboo,ato

Comment 8

[Jonathan Watt [:jwatt]]

(In reply to James Graham [:jgraham] from comment #5)

Turns out that kPaperSizeMillimeters resulted in a too-large surface on Mac, so I have a suspicion that it's using inches for the paper size there regardless.

As in you were seeing it be ~25x too large? Or a more general too large?

Comment 9

[James Graham [:jgraham]]

I don't actually know how large it was but the error was about the surface exceeding the allocation limit [1] and changing the nsIPrintSettings object to use kPaperSizeInches and converting the input as required worked well.

[1] https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=281840228&repo=autoland&lineNumber=319643-319649

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/a3ea83c0bad8
https://hg.mozilla.org/mozilla-central/rev/d684a6d51c8a

Comment 11

[Kayla Firestack [:kaylafire]]

Hello all!
Thanks for your work on this, I very much appreciate it.
I'm attempting to utilize the WebDriver:Print function implemented here.
When printing with the webdriver parameter "background" : true, Marionette returns a pdf which does not have background colors or images.

I believe this is due to the parameter being named printBackground instead of background, which makes sense as the chromedevtools protocol uses this convention, and these changes are based off of that implementation(https://bugzilla.mozilla.org/show_bug.cgi?id=1599994), but this is not complaint with the webdriver spec, and therefore the webdriver crate (which is used as an intermediary representation in Geckodriver, preventing the ability to manually send the parameter) as well.

(I've removed extra json arguments to point out the issue)

webdriver::server DEBUG -> POST /session/<SESSION_ID>/print {"background":true, <removed>, "printBackground":true, <removed>}
Marionette DEBUG 0 -> [0,5,"WebDriver:Print",{"background":true, <removed> }]
RemoteAgent DEBUG PDF output written to /build/Temp-<UUID>/marionette.pdf

the webdriver spec defines the argument which enables printing background items to have the name background, as shown here
https://w3c.github.io/webdriver/#print-page
https://github.com/w3c/webdriver/pull/1468/files#diff-0eb547304658805aad788d320f10bf1f292797b5e6d745a3bf617584da017051R9178-R9183

Should I open this as a separate bug linked to this since this is closed?
I'd also be happy to implement these changes, but I've not seen any documentation on how to get started on that (yet).

Comment 12

[James Graham [:jgraham]]

:kaylafire - Thanks for the issue report, and detailed diagnosis. It looks like you're correct and we should be using background in https://searchfox.org/mozilla-central/source/remote/marionette/driver.js#3047.

As you suggested, a new bug for this problem would be ideal. If you want to work on it see https://firefox-source-docs.mozilla.org/contributing/index.html for a generic getting started guide and please feel free to ask any questions in https://chat.mozilla.org/#/room/#webdriver:mozilla.org