1662426 - PDFs sometimes contain blank pages when printing

Status: VERIFIED FIXED

(Core :: Printing: Output, defect, P1)

Description

[Martin Balfanz [:mbalfanz]]

Attachment: Kapture 2020-09-01 at 15.03.22.mp4

Unfortunately, the steps to reproduce are not 100% reliable. As you can see in the attached video, it's a bit of a trial and error.

STR:

1. visit https://www.uscis.gov/sites/default/files/document/forms/i-9-paper-version.pdf (the same can be reproduced with other PDF documents. This one was just the first result on google when searching for "pdf document")
2. open the print dialog (I alternate between cmd+p and clicking the UI button)
3. scroll the preview

ER: All pages should have content
AR: sometimes, the second page is blank

The produced PDF and printouts reflect the preview. If the preview is complete I get a complete result. If the preview is showing a blank page, I'll get a blank page in the printed PDF.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

(Not sure whether this is Print Preview or Printing: Output…)

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Martin, I assume that's on Mac? Can you try to confirm a regression range? Bug 1662375 looks similar but I haven't been able to reproduce it yet...

Comment 3

[Martin Balfanz [:mbalfanz]]

Yes I'm on Mac.

Took several attempts, but here we go: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2969f3de028f013d30ea20bfccc13670ef8e195f&tochange=0c1847dee730f55b184459f5578c67ab45b611c1

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Fun stuff, thank you.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I could repro this on Mac, thank you!

Comment 6

[Hiroyuki Ikezoe (:hiro)]

I believe this is caused by the same root cause of bug 1661222, and bug 1636728 made it more noticeable see bug 1661222 comment 5.

(edited: fixed one of the bug number)

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Yeah, I know why this is now, but still trying to figure out what the best way to fix it is.

The issue is that we hit this code path in some cases from PDF.js.

The reason is that we destroy the previous print job when starting a new one, but we don't create a pres shell for the new print job until later:

https://searchfox.org/mozilla-central/rev/84922363f4014eae684aabc4f1d06380066494c5/layout/printing/nsPrintJob.cpp#787,791

ShowPrintProgress spins the event loop which causes the print callbacks to run while the document is not yet rendered.

A potential fix (other than "don't spin the event loop") would be something like this: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f78c3fe724b1aa4655573afd2dc2519554160ca

Which ensures that the last print request always completes. I need to confirm that works though.

Other than that:

• I could try to fix the canvas code to avoid needing a pres shell (though it's a bit hard because it needs to compute styles).
• I could try to remove the nested event loop (in fact bug 1653340 may do that for most cases we care about).
• I could try to hack around it in pdf.js (not great).

Thoughts?

Comment 8

[Hiroyuki Ikezoe (:hiro)]

FWIW, I still see the issue with Emilio's fix in comment 7 (and with bug 1653319), in my case the event loop comes from this SetupSilentPrinting call, and the call runs the event loop only on Linux. So maybe it's not a big deal.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Adding currentRenderTask.cancel() call in the error case stops the blank page on my Linux box, FWIW.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

I am supposing it's been a long standing issue in PDF printing sutff, bug 1020878 or bug 1084327.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

One more note I noticed is, when the print preview is triggered by Ctrl+P from the browser, mozPrintCallback is called for each page just once respectively, whereas when the preview is triggered by the print icon in the PDF viewer (i.e. window.print() I assume), mozPrintCallback is called twice. I don't know where the difference comes from (I presume that destroying the previous print job in Emilio's comment in comment 7 implies it?), but if we could avoid it, it seems to be nice to me.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1662426 - Add a couple warnings in the canvas2d code. r=jwatt

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1662426 - When destroying an HTML <canvas> frame, reset its print callback data. r=jwatt

This ensures that we call the callback again if the canvas becomes
displayed later. This is important because a callback may run after
destroying the frame tree, and in that case some canvas operations may
fail. Without this there's no guarantee that the callback runs ever
again.

Depends on D89111

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1662426 - Deal better with print callback failures in pdf.js. r=bdahl

With the current code, once the callback failed once, it gets stuck in
the canvasInRendering set, and thus we can't run it again. This was very
unlikely but could happen already before my recent print changes, but
with the new setup for window.print() it can happen more often.

Depends on D89112

Comment 15

[Emilio Cobos Álvarez (:emilio)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #11)

One more note I noticed is, when the print preview is triggered by Ctrl+P from the browser, mozPrintCallback is called for each page just once respectively, whereas when the preview is triggered by the print icon in the PDF viewer (i.e. window.print() I assume), mozPrintCallback is called twice. I don't know where the difference comes from (I presume that destroying the previous print job in Emilio's comment in comment 7 implies it?), but if we could avoid it, it seems to be nice to me.

Yeah, that's expected, and a bit unfortunate, but the settings from the print preview are different from the ones from window.print()... We could try to avoid doing the first print job though...

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Attachment: PDF.js PR

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Martin, Sebastian, mind confirming that with a build from here you don't see the issue anymore?

I can confirm that I don't see it anymore on Mac or on Linux with chaos mode, which are the two setups where I could reproduce this in the first place, but if you can confirm too it'd be great.

Comment 18

[Martin Balfanz [:mbalfanz]]

I can confirm that this build works for me 👍

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e55282f6711
Add a couple warnings in the canvas2d code. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/dfddd9655dec
When destroying an HTML <canvas> frame, reset its print callback data. r=jwatt

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Issue seems to be resolved with the mentioned build.

Comment 22

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

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

Comment 23

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7e55282f6711
https://hg.mozilla.org/mozilla-central/rev/dfddd9655dec

Comment 24

[Emilio Cobos Álvarez (:emilio)]

The PDFJS change landed in bug 1662914.

Comment 25

[Blake Winton (:bwinton) (:☕️)]

Should we request uplift on that bug, Emilio, so that this once can get uplifted?

Comment 26

[Jonathan Watt [:jwatt]]

Comment on attachment 9173577 [details]
Bug 1662426 - Add a couple warnings in the canvas2d code. r=jwatt

Beta/Release Uplift Approval Request

• User impact if declined: Broken PDF printing
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 27

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9173577 [details]
Bug 1662426 - Add a couple warnings in the canvas2d code. r=jwatt

Approved for 81.0b6.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a16c07ef3476
https://hg.mozilla.org/releases/mozilla-beta/rev/ec0ac471ab42
https://hg.mozilla.org/releases/mozilla-beta/rev/bb2adc6352af

Comment 30

[Vlad Lucaci, Desktop QA (:vlucaci)]

Confirming this issue as verified fixed on macOS 10.15, Ubuntu 18.04 and Windows 10x64. Verified using 82.0a1 (2020-09-09) and 81.0b8