Closed Bug 1662426 Opened 1 year ago Closed 1 year ago

PDFs sometimes contain blank pages when printing

Categories

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

defect

Tracking

()

VERIFIED FIXED
82 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 --- verified
firefox82 --- verified

People

(Reporter: mbalfanz, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression, Whiteboard: [print2020_v81])

Attachments

(4 files, 1 obsolete file)

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.

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

Component: Printing → Printing: Output
Product: Toolkit → Core

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

See Also: → 1662375
Flags: needinfo?(mbalfanz)
Regressed by: 1636728

Fun stuff, thank you.

I could repro this on Mac, thank you!

Flags: needinfo?(emilio)
Blocks: 1631440
Severity: -- → S1
Priority: -- → P1

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)

See Also: → 1661222

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?

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.

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

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

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.

See Also: 1662375
Assignee: nobody → emilio
Status: NEW → ASSIGNED

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

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

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

Attached file PDF.js PR

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.

Flags: needinfo?(mbalfanz)
Flags: needinfo?(emilio)
Flags: needinfo?(aryx.bugmail)
Keywords: leave-open

I can confirm that this build works for me 👍

Flags: needinfo?(mbalfanz)
Pushed by ealvarez@mozilla.com:
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

Issue seems to be resolved with the mentioned build.

Flags: needinfo?(aryx.bugmail)
Duplicate of this bug: 1662375
Attachment #9173579 - Attachment is obsolete: true
Depends on: 1662914

The PDFJS change landed in bug 1662914.

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

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

Flags: needinfo?(emilio)

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:
Attachment #9173577 - Flags: approval-mozilla-beta?
Attachment #9173578 - Flags: approval-mozilla-beta?

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

Target Milestone: --- → 82 Branch

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

Approved for 81.0b6.

Attachment #9173577 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9173578 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9173583 - Flags: approval-mozilla-beta+
Regressions: 1663053
Flags: needinfo?(emilio)
Flags: qe-verify+
Regressions: 1663131
QA Whiteboard: [qa-triaged]

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

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.