Closed Bug 1604506 Opened 4 years ago Closed 4 years ago

Implement print command in marionette

Categories

(Remote Protocol :: Marionette, task)

Version 3
task
Not set
normal

Tracking

(firefox73 fixed)

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: jgraham, Assigned: jgraham)

References

Details

Attachments

(2 files)

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

Assignee: nobody → james
Status: NEW → ASSIGNED

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.

Attachment #9116414 - Attachment description: Bug 1604506 - Add print endpoint to marionette, → Bug 1604506 - Add print command to marionette,
Pushed by james@hoppipolla.co.uk:
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

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

Flags: needinfo?(james)

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

Flags: needinfo?(james)

(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?

Flags: needinfo?(jwatt)
Pushed by james@hoppipolla.co.uk:
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

(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?

Flags: needinfo?(jwatt) → needinfo?(james)
Flags: needinfo?(jwatt)

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

Flags: needinfo?(james)
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Flags: needinfo?(jwatt)

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).

Flags: needinfo?(james)

: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

Flags: needinfo?(james)
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.