2057444 - Printing of PDF attachments sometimes fails (preview ok, actually prints blank)

Status: RESOLVED FIXED

(Core :: Printing: Output, defect)

Description

[Frank Winkler]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:152.0) Gecko/20100101 Firefox/152.0

Steps to reproduce:

Another long-term issue ...

In some rare cases, I can open a PDF attachment inside TB and it's shown completely properly. When printing the file, I end up in an empty sheet. Saving the PDF to a file, opening it with "Preview" or "Acrobat Reader" and printing it from there works fine. I couldn't find a pattern yet.

Actual results:

see above

Expected results:

The PDF file should be printed correctly.

Comment 1

[Wayne Mery (:wsmwk)]

Bug 2047850 - Printing via system print dialog no longer works on Mac - fixed an issue for v152. But it's indicated as not affecting v140. :(

Comment 2

[Frank Winkler]

I don't know why v140 was added here. I've been seeing this issue - very sporadically - across many macOS and TB versions during the last years. And it applies to my HP LaserJet @home as well as to a Kyocera business printer in the office. And to my impressions, it doesn't depend on just the TB version as an other PDF prints well with the same TB. I assume there must be some race condition in the PDF processing.

Comment 3

[Wayne Mery (:wsmwk)]

(In reply to Frank Winkler from comment #2)

I don't know why v140 was added here. I've been seeing this issue - very sporadically - across many macOS and TB versions during the last years.

That does call into question whether bug 2047850 will fix it, but still possible.

Comment 4

[Frank Winkler]

BTW: I totally forgot to mention that printing the PDF just with lp also worked fine so there were many ways of printing which worked fine - just TB failed ;) ...

Comment 5

[Magnus Melin [:mkmelin]]

FWIW, I've had this recently... printing through another application worked.

Comment 7

[Magnus Melin [:mkmelin]]

Bug 2055412 may be part of the story.

Comment 8

[Magnus Melin [:mkmelin]]

After further investigation, I found at least the current problem is a regression from bug 2034406.
At least this one seems easy to reproduce - but a physical printer is needed.

Comment 9

[Magnus Melin [:mkmelin]]

Attachment: Bug 2057444 - PDFs in Thunderbird print blank. r=emilio

Bug 2034406 changed nsPageSequenceFrame::PrePrintNextSheet to render every mozPrintCallback canvas into a recording DrawTarget instead of a real one:

RefPtr recorder = MakeAndAddRef<gfx::DrawEventRecorderMemory>(nullptr);
RefPtr<DrawTarget> canvasTarget = gfx::Factory::CreateRecordingDrawTarget(recorder, referenceDt, ...);

and made the paint side replay it (nsHTMLCanvasFrame.cpp):

if (presContext->Type() != nsPresContext::eContext_Print ||
!canvas-GetMozPrintCallback() ||
!dt.TryToReplaySurface(surface, destRect, srcRect)) {
dt.DrawSurface(surface, destRect, srcRect, ...);
}

TryToReplaySurface is only implemented by DrawTargetRecording (gfx/2d/DrawTargetRecording.cpp:555); the base returns false (gfx/2d/2D.h:1539). When it returns false the fallback draws the snapshot — but that snapshot is a SourceSurfaceRecording, which has no pixels. Result: nothing is painted.

The print destination is a recording DrawTarget only when the job is recorded in a content process for replay in the parent. So:

• Firefox: PDFs always render in a content process → recording destination → replay works.
• Thunderbird: a PDF attachment opens as a content tab on a mailbox:/imap: URL, which is parent-process only (browser.isRemoteBrowser == false). The print destination is a real DrawTarget → replay fails → every pdf.js page canvas paints nothing → blank sheets, while the viewer and the print preview both look correct (preview goes through HandlePrintCallback/on-screen painting, not this path.)

Assisted by Claude.

Comment 10

[BugBot [:suhaib / :marco]]

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

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2057444 - Make TryToReplaySurface work on arbitrary DrawTargets. r=lsalzman

Comment 12

[Magnus Melin [:mkmelin]]

D318194 seems alright in my testing

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3a18748d9a8c
https://hg.mozilla.org/integration/autoland/rev/63587df02f81
Make TryToReplaySurface work on arbitrary DrawTargets. r=lsalzman

Comment 14

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/63587df02f81

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Does this need a Release uplift request?

Comment 16

[Emilio Cobos Álvarez [:emilio]]

This doesn't affect Firefox so no unless Thunderbird needs it.

Comment 17

[Magnus Melin [:mkmelin]]

As 153 is esr, it would be good to uplift there, so Thunderbird ESR users don't run into this for the next year.

Comment 18

[Emilio Cobos Álvarez [:emilio]]

Requested!

Comment 19

[Phabricator Automation]

firefox-esr153 Uplift Approval Request

• User impact if declined/Reason for urgency: Affects Thunderbird.
• Code covered by automated testing?: no
• Fix verified in Nightly?: yes
• Needs manual QE testing?: no
• Steps to reproduce for manual QE testing:
• Risk associated with taking this patch: low
• Explanation of risk level: Relatively minimal change.
• String changes made/needed?: none
• Is Android affected?: yes

Comment 20

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 2057444 - Make TryToReplaySurface work on arbitrary DrawTargets.

The implementation living in DrawTargetRecording.cpp is slightly
annoying but makes things easier, wdyt?

Alternative would be to move this to a
SourceSurfaceRecording::TryToReplayInto(DrawTarget*) or so.

Original Revision: https://phabricator.services.mozilla.com/D318194

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr153/rev/2fa3e8c3e473

Comment 24

[Frank Winkler]

Just out of curiosity: what was the actual problem and what triggered it? I'm not deep enough in the source code to figure out what the above snippets (should) do or don't.
And the reason for "won't fix in FF 154" is that it was just about to be replaced by 155?