1741698 - window.print() fails when about:config pref `print.always_print_silent` is enabled (which blocks fuzzing)

Status: VERIFIED FIXED

(Core :: Printing: Setup, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: log.txt

Last good build: 20211116-3890e2f0b025
First build with issue: 20211116-bb05a9c04907

Fuzzing started triggering a dialog that hangs the browser and blocks fuzzing. It seems that it is very easy for the fuzzer to trigger this.

Comment 1

[Tyson Smith [:tsmith]]

Attachment: prefs.js

The test case is just a call to window.print() while using a fuzzing build with our fuzzing prefs.js file

Comment 2

[Daniel Holbert [:dholbert]]

window.print() does indeed trigger a tab-modal dialog which requires user interaction to proceed. Note that it's not an "error dialog"; it's just the regular print dialog (unless you're talking about something different?)

The dialog shouldn't hang the whole browser, but I think it does pause JS execution in the current tab, until the user clicks through the dialog. (similar to alert() and prompt() I think). Are you seeing this behave differently from those APIs, regarding whether/when the fuzzer is able to proceed?

(I confirmed that I can e.g. successfully close the tab with the "x" button after doing window.print(). I would imagine the fuzzer is able to to that, though I'm not sure precisely how it works / what it needs to do next after having invoked this API in order to proceed.)

Whatever the core issue is here, do you know if it's a longtime issue vs. if it started recently?

(In any case, emilio probably would probably be the best person to comment here, since he e.g. implemented dom.window_print.fuzzing.block_while_printing (in bug 1698175) which I see in your prefs.js file and which might be related or in the neighborhood of whatever's needed/broken here. Ticking ni=emilio to be sure this is on his radar.)

Comment 3

[Daniel Holbert [:dholbert]]

Ah, I see print.always_print_silent in your attached prefs.js, too; and I can confirm that, with that set, window.print() does spawn a popup error dialog saying

Print Preview Error
An error occurred while printing.
[OK]

Presumably that's the error popup you're talking about here.

Comment 4

[Daniel Holbert [:dholbert]]

Yeah, ok -- I'm seeing a regression here. Testcase/regression range coming up; looks like fallout from Bug 1669149, probably.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I don't understand why https://phabricator.services.mozilla.com/D99808 is desirable? what am I missing?

Comment 6

[Daniel Holbert [:dholbert]]

STR:

1. Load attached testcase, in Firefox Nightly with print.always_print_silent set to true
2. Click the button

EXPECTED RESULTS:
A print-progress dialog appears and disappears. (maybe output should get written/printed somewhere; not sure, but it doesn't really matter)

ACTUAL RESULTS:
An error dialog appears, saying "Print Preview Error", as discussed in comment 3.

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=a7a2d1aa79e0fb84693eebf87764d30f553681a0&tochange=2f40a908044e148079d0db8f2dc86571b73d9149

--> regression from bug 1669149 which jwatt just rebased and shepherded in to central. Perhaps specifically a regression from part 5, which emilio linked above.

jwatt, mind taking a look? As noted in comment 0, this is blocking fuzzers from testing printing.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (note, be sure to set pref "print.always_print_silent" before testing)

Comment 8

[Daniel Holbert [:dholbert]]

ni=jwatt per comment 5 - 6.

Comment 9

[Tyson Smith [:tsmith]]

For clarity on the fuzzing impact, this blocks print code fuzzing but also has significant impact of the iteration rate of DOM fuzzing that calls window.print()

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1669149

Comment 11

[Calixte Denizet (:calixte)]

There are some failures in pdf.js' integration tests:
https://github.com/mozilla/pdf.js/issues/14293
because of this bug.

Comment 12

[Jonathan Watt [:jwatt]]

Attachment: Bug 1741698 - Fix window.print() for print.always_print_silent. r=emilio

Comment 13

[Emilio Cobos Álvarez (:emilio)]

[Tracking Requested - why for this release]: Breaking silent printing not only blocks fuzzers. In the past we've had enterprise uses and such complain when we've broken the feature. We should probably back out the regressing bug for 96 or land the fix here soon.

Comment 14

[Jonathan Watt [:jwatt]]

Attachment: Bug 1741698 - Revert 1fb0f7dc84a9 (part 3 of bug 1669149) since it broke silent printing. r=emilio

Comment 15

[Jonathan Watt [:jwatt]]

https://phabricator.services.mozilla.com/D132377 doesn't appear to actually fix the issue so I must have messed up my original testing.

The actual culprit here is https://hg.mozilla.org/integration/autoland/rev/1fb0f7dc84a9c5e7c68b56e6a66561177356a544 . That's logically separate from the other patches that landed in bug 1669149, so let's back that out as the safer course of action for uplift to beta.

Hence the new Phab rev: https://phabricator.services.mozilla.com/D134621

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe90fc6b12b5
Revert 1fb0f7dc84a9 (part of bug 1669149) since it broke silent printing. r=emilio

Comment 17

[Atila Butkovits]

Backed out for causing failures at delayed_window_print.html.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=d3gAUMlgQ7eX5KxjNMpYHA.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=fe90fc6b12b557475ce37dff6ff9c288d5de64ca

Failure log: https://treeherder.mozilla.org/logviewer?job_id=362262659&repo=autoland&lineNumber=20178
https://treeherder.mozilla.org/logviewer?job_id=362247745&repo=autoland&lineNumber=18083

Comment 18

[Jonathan Watt [:jwatt]]

The full treeherder results for the push:

https://treeherder.mozilla.org/jobs?repo=autoland&revision=fe90fc6b12b557475ce37dff6ff9c288d5de64ca

So this other, related test fails too:

https://treeherder.mozilla.org/logviewer?job_id=362259066&repo=autoland&lineNumber=18079

Neither of these two tests fail locally for me on Windows 10, so maybe something to do with the virtual machine Windows install the CI uses? This could be fun to figure out...

Comment 19

[Jonathan Watt [:jwatt]]

FWIW, what's causing silent printing to fail in the first place is that part 3 of bug 1669149 meant silent printing no longer enters the section of nsPrintJob::DoCommonPrint that does the event loop spinning to "open" a system print dialog in the parent process to get the print settings that should be used. That causes us to error out later in nsPrintJob::DoCommonPrint when we call nsDeviceContextSpecProxy::Init. That's because nsDeviceContextSpecProxy::Init fails if we don't have a RemotePrintJobChild, which is what the call to "open" the print dialog in the parent process got us.

Comment 20

[Jonathan Watt [:jwatt]]

Attachment: Bug 1741698 - Fix silent printing for both window.print() and print to file. r=emilio

Printing was failing for silent printing because we need a RemotePrintJobChild.
We should also round-trip to the parent to get a valid set of print settings.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8105d0c98b35
Fix silent printing for both window.print() and print to file. r=emilio

Comment 22

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing wpt failures on Devices-without-focus.https.html. CLOSED TREE

Backout link : https://hg.mozilla.org/integration/autoland/rev/9a5b0987d80a7a106ab76e45512294cf0a16ac54

Push with failures : https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=windows%2C10%2Cx64%2C2004%2Cwebrender%2Copt%2Cweb%2Cplatform%2Ctests%2Cwith%2Cfission%2Cenabled%2Ctest-windows10-64-2004-qr%2Fopt-web-platform-tests-fis-e10s%2Cwpt6&revision=8105d0c98b35764aa0a5743e85b3cbe1e725b492&selectedTaskRun=dpdBx1YtQoS7rEKO7G0dbQ.0

Link to failure log : https://treeherder.mozilla.org/logviewer?job_id=362773806&repo=autoland&lineNumber=9062

Failure message :
TEST-UNEXPECTED-TIMEOUT | /_mozilla/mediacapture-streams/enumerateDevices-without-focus.https.html | enumerateDevices without focus - Test timed out

Comment 23

[Jonathan Watt [:jwatt]]

This doesn't fail locally running ./mach test testing/web-platform/mozilla/tests.

The code my patch touches obviously doesn't directly affect the failing mediacapture-streams tests. However, the log from CI shows delayed_window_print.html running just before the failing enumerateDevices-without-focus.https.html test.

What appears to be happening is that in delayed_window_print.html the window.print() on CI is using the "Microsoft Print to PDF" printer (presumably the default on CI where no actual printers are connected). The "Microsoft Print to PDF" printer opens an app modal dialog that grabs focus (Microsoft's code obviously doesn't pay attention to our always_print_silent pref), and that modal dialog blocks the event loop of the main thread of the parent process.

Parent process:

::StartDocW  <-- blocks
gfx::PrintTargetWindows::BeginPrinting
nsDeviceContext::BeginDocument
layout::RemotePrintJobParent::InitializePrintDevice
layout::RemotePrintJobParent::RecvInitializePrint
layout::PRemotePrintJobParent::OnMessageReceived
dom::PContentParent::OnMessageReceived
ipc::MessageChannel::DispatchAsyncMessage
ipc::MessageChannel::DispatchMessage
ipc::MessageChannel::MessageTask::Run()
mozilla::RunnableTask::Run
mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal
mozilla::TaskController::ExecuteNextTaskOnlyMainThreadInternal
mozilla::TaskController::ProcessPendingMTTask
mozilla::TaskController::InitializeInternal::<lambda_1>::operator()
mozilla::detail::RunnableFunction<`lambda
nsThread::ProcessNextEvent
NS_ProcessNextEvent

Child process:

mozilla::SpinEventLoopUntil
layout::RemotePrintJobChild::InitializePrint
nsDeviceContextSpecProxy::BeginDocument
nsDeviceContext::BeginDocument
nsPrintJob::SetupToPrintContent
nsPrintJob::DocumentReadyForPrinting
nsPrintJob::MaybeResumePrintAfterResourcesLoaded
nsPrintJob::InitPrintDocConstruction
nsPrintJob::DoCommonPrint
nsPrintJob::CommonPrint
nsPrintJob::Print
nsDocumentViewer::Print
nsGlobalWindowOuter::Print
dom::BrowserChild::RecvPrint
dom::PBrowserChild::OnMessageReceived
ipc::MessageChannel::DispatchAsyncMessage
ipc::MessageChannel::DispatchMessage
ipc::MessageChannel::MessageTask::Run
mozilla::RunnableTask::Run
mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal

delayed_window_print.html reports itself as having completed/passed successfully because it calls t.done() in response to the beforeprint event (well, after returning to the event loop), which will be before the async code that gets us into the above situation is invoked.

It looks from the log like the browser is successfully restarted "for new test group" (mediacapture-streams) despite the "hang" under StartDocW, but something's clearly not right if enumerateDevices-without-focus.https.html goes on to time out.

I guess some questions that still should be answered are:

• Why did this not previously fail? Was the browser terminated for the new test group before the dialog could open?
• Why don't other window.print() tests fail?
• What can we do about this? How can we test window.print() without it opening an dialog that won't get a response?
• Should we disable this test for now on windows so we can get this code landed? It seems to be a test issue, not an issue with Firefox.

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Can we use the regular PDF printer for that test instead?

Comment 25

[Jonathan Watt [:jwatt]]

(In reply to Jonathan Watt [:jwatt] from comment #23)

• Why don't other window.print() tests fail?

The answer to this seems to be because delayed_window_print.html is the only test that uses both silent printing and window.print(). Other than that test, only print reftests (ours and wpt) use silent printing. Tests that don't use silent printing open the new UI, and that's non-blocking and closes automatically on tab navigation/tab close.

The tests that use window.print() are:

toolkit/components/printing/tests
These use PrintHelper.addMockPrinter so avoid the "Microsoft Print to PDF" printer anyway, but they also don't use silent printing.

dom/tests/mochitest/bugs/test_bug739038.html
This test only checks that window.print() throws for pref dom.enable_window_print=false

layout/generic/crashtests/1697262-1.html
layout/tables/crashtests/1118168.html
These don't use silent printing.

testing/web-platform/tests/html/webappapis/user-prompts/resources/print-during-event.sub.html
These don't use silent printing.

Comment 26

[Jonathan Watt [:jwatt]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #24)

Can we use the regular PDF printer for that test instead?

Thanks. A Try push shows switching to our "Mozilla Save to PDF" printer gets the test passing (presumably we actually write a PDF file to disk). But it's probably simpler to just stop using silent printing and allow the tab-modal print preview to open since that will close fine when Marionette closes/navigates the tab. I'm just waiting for a Try build to confirm that that works.

Comment 27

[Jonathan Watt [:jwatt]]

WPT doesn't like running without silent printing for some reason. I'm not sure exactly why but I've sunk so much time into this bug now I'm not up to trying to coax that information out of CI right now. Let's go with your suggestion.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d944538c5760
Fix silent printing for both window.print() and print to file. r=emilio

Comment 29

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/d944538c5760

Comment 30

[Jonathan Watt [:jwatt]]

It seems that we're reconsidering taking a fix for Firefox 96. In light of that, here's some information to help inform that decission.

TL;DR: I recommend taking https://phabricator.services.mozilla.com/D134621 for 96 only.

Being unable to reproduce locally, I never got to the bottom of why D134621 (Revert part 3 of bug 1669149) caused delayed_window_print.html (comment 17) and 1697262-1.html (comment 18) to fail on central. I did figure out various things over the last couple of weeks though, partly due to a painful number of speculative and and bisecting pushes to Try. The main thing is that those failures happen due to a bad interaction with the progress dialog removal in bug 1558588, which landed some time after the regression, and after 96 moved to beta. So these failures aren't an issue for 96. Or to be more precise, reverting part 3 of the regressing bug doesn't cause delayed_window_print.html to fail, but it does seem to return to a pre-regression state of intermittently failing on debug Windows:

Here are some relevant Try pushes:

Immediately before the patches from the regressing bug 1669149 landed, layout/generic/crashtests/1697262-1.html was intermittently failing on Windows debug builds:
https://treeherder.mozilla.org/jobs?repo=try&revision=790d37cbc4bf1a04596fd6e3a4b83eda22745f2d&group_state=expanded

Immediately after the patches from the regressing bug 1669149 landed, the layout/generic/crashtests/1697262-1.html intermittent failures seem to disappear:
https://treeherder.mozilla.org/jobs?repo=try&revision=450a1b7fb3eb249b169de3b65b7b269a9b1bf51a&group_state=expanded

Backing out part 3 of the regressing bug on beta HEAD as of Dec 23rd, the layout/generic/crashtests/1697262-1.html intermittent failures appear to come back:
https://treeherder.mozilla.org/jobs?repo=try&revision=ebc3077248f80ccf23f332e63b6e4b757cbba5a9&group_state=expanded

Backing out part 3 of the regressing bug on beta HEAD as of Dec 28th shows the same (unsurprising):
https://treeherder.mozilla.org/jobs?repo=try&revision=f02978caaefa8a8327974230e0b988c00d207998&group_state=expanded

The one query I'd have about the above Try pushes is why there isn't a suggested bug that comes up for layout/generic/crashtests/1697262-1.html since it looks like it fails enough in the pre-regression Try push linked above to have warranted one.

Comment 31

[Jonathan Watt [:jwatt]]

Comment on attachment 9256717 [details]
Bug 1741698 - Revert 1fb0f7dc84a9 (part 3 of bug 1669149) since it broke silent printing. r=emilio

Beta/Release Uplift Approval Request

• User impact if declined: Printing from websites that use window.print() (for example, from a button that creates a print version of the site and invokes window.print() - pretty common) will be broken for users that use silent printing. It's unclear how big a group that overlapping set is, but breaking silent printing has resulted in a significant number of complaints in the past. If I recall correctly (I may not) corporate users and people using SaaS services in Firefox to print receipts and labels have been the bulk of this group in the past.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce: If we want QE testing, see the testcase attached in the bug.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): I think the risk is low. Part 3 from the regressing bug (what we'd be reverting here) is independent of the other patches. The backout would revert us back to our previous behavior. Only "incomplete" nsIPrintSettings objects passed to nsPrintJob::DoCommonPrint will be affected, and we only get them from window.print(), when silent printing is enabled (due to this code meaning we do make the Print() call instead of skipping the PrintPreview() call, since aIsPreview == IsPreview::No for silent printing). So if somehow this was to break anything, it's limited to breaking the case that's already broken (it fixes it in my testing).
• String changes made/needed:

Comment 32

[Dianna Smith [:diannaS]]

Comment on attachment 9256717 [details]
Bug 1741698 - Revert 1fb0f7dc84a9 (part 3 of bug 1669149) since it broke silent printing. r=emilio

Approved for 96.0rc2

Comment 33

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/5301a841e204

Comment 34

[Catalin Sasca, Desktop QA [:csasca]]

I was able to reproduce the issue on Firefox 96.0a1 under macOS 12.0.1 by using the provided TC and steps in Comment 6.

The issue is fixed on Firefox RC2 96.0 and 97.0a1 (2022-01-06) under macOS 12.0.1 and Windows 11. However while testing on Ubuntu 20.04, Firefox 97.0a1 behaved as expected while 96.0 would do nothing when using the TC (the silent prompt will appear but no job is actually sent to the physical printer, and no errors are thrown in the console).

Jonathan, any idea why is this happening on Ubuntu ?

Comment 35

[Jonathan Watt [:jwatt]]

Thanks for testing, Catalin. Can you check the pref print_printer? If the value is set to "Mozilla Save as PDF" (or if it defaults to that because the value isn't set) then silent printing will save a PDF file instead of printing to your printer. Most likely the file will be saved to $HOME/mozilla.pdf. If this is the issue, then you can change the value of print_printer to the correct value for your physical printer by turning off silent printing and printing once to that printer (you can cancel the print job immediately if you like). Assuming the value of print_printer updates correctly, then once you turn silent printing back on the testcase should silent print to your physical printer. (At least it works for me on Fedora.)

Comment 36

[Catalin Sasca, Desktop QA [:csasca]]

Just looked for print_printer and it doesn't seem to have any value (maybe it will have a value after the first print, but it won't do anything unfortunately). I've only tested with clean profiles and silent printing pref on (everytime Firefox defaults to the printer and not to print to pdf). It is strange that 97 works as expected with the same steps, with a new profile.

Comment 37

[Jonathan Watt [:jwatt]]

Can you print to the printer with silent printing off?

Comment 38

[Catalin Sasca, Desktop QA [:csasca]]

After further discussions with Jonathan, a first print needs to be done so that Firefox will default to the physical printer and then it will work as expected with silent printing pref on. Thank you Jonathan for the informations!

Comment 39

[Calixte Denizet (:calixte)]

It's verified too with pdf.js' integration tests.

Comment 40

[Jonathan Watt [:jwatt]]

Thanks, Calixte.