1722188 - Print function fails on protonmail

Status: VERIFIED FIXED

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

Description

[efriend]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:89.0) Gecko/20100101 Firefox/89.0

Steps to reproduce:

Using website protonmail.com
Trying to print an email and tried to print the information directly from the source website.
Tried on two different computers with WIN10 (current build) with FF 90.0.2

Actual results:

Received pop-up with print failure notice. Document was printed, but was missing substantial amount of information (2 very sparse pages). I tried to use two different print destinations (PDF995 print driver, and Microsoft Print to PDF). Same results.

I downgraded to FF 89.0 and printing worked OK (as it had many times before).

Expected results:

4 pages of text, black and white, no graphics.

Comment 1

[efriend]

I noticed that the FF90.0.2 release notes mentioned a printing fix, but apparently, it did not work.

Comment 2

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

The Bugbug bot thinks this bug should belong to the 'Core::Printing: Output' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[Petruta Horea [:phorea], Desktop QA]

I created a protonmail account and was able to reproduce this issue on Firefox 90.0.2, 91b8 and latest Nightly 92.0a1 under Win 10 64-bit - using Microsoft print to PDF option. I used the email client option to print, and the pop-up error was present for emails longer than 4 pages. Only the first two pages are filled in.

According to mozregresson, this was regressed by bug 1540737, I'm not marking it as dupe of bug 1722173 because the output is different here.
Bug 1540737 - Don't use PushLayerWithBlend since it's not implemented by all backend

Comment 5

[Alice0775 White]

The following error is displayed in Browser Console when printing a proton mail:

LoginRecipes: getRecipes: falling back to a synchronous message for: https://mail.protonmail.com LoginRecipes.jsm:295:9
[Exception... "Failure"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/Prompter.jsm :: openPromptSync :: line 1116"  data: no] Prompter.jsm:1116:17
    openPromptSync resource://gre/modules/Prompter.jsm:1116
    alert resource://gre/modules/Prompter.jsm:1393
    alert resource://gre/modules/Prompter.jsm:80
    _displayPrintingError chrome://global/content/printUtils.js:559
    receiveMessage resource://gre/actors/PrintingParent.jsm:25
    (Async: JSActor query)
    handleEvent resource://gre/actors/PrintingChild.jsm:66
    i https://mail.protonmail.com/7.b3220512.chunk.js:1
    onSubmit https://mail.protonmail.com/vendors~index.b615def6.chunk.js:93
    s https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    d https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    b https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    b https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    se https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    oe https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    ue https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    pe https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    N https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    Z https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    $e https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    Je https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    bind_applyFunctionN self-hosted:1373
    Je self-hosted:1336
    unstable_runWithPriority https://mail.protonmail.com/vendors~index.b615def6.chunk.js:38
    Ki https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    R https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    Qe https://mail.protonmail.com/vendors~index.b615def6.chunk.js:119
    bind_applyFunctionN self-hosted:1373
    Qe self-hosted:1336
    r https://mail.protonmail.com/vendors~index.b615def6.chunk.js:30

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: Standalone test case

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Removing the anim-modal-in animation from .modal seems to make the problem go away.

Matthieu, perhaps you can test out temporarily removing the animation as a short term fix until we fix the problem in Firefox?

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Attachment: Somewhat reduced test case

Comment 9

[Matt Woodrow (:mattwoodrow)]

This crashes for me with 'Assertion failed: ! surface->unbounded, file c:/src/mozilla-central/gfx/cairo/cairo/src/cairo-recording-surface.c, line 606', same as bug 1722173.

Comment 10

[Matt Woodrow (:mattwoodrow)]

Attachment: Minimal test case

Comment 11

[Matt Woodrow (:mattwoodrow)]

Calling PushLayer with opaque contents (triggered by will-change:opacity, but no actual opacity value), results in the cairo group being unbounded (as the original recording surface was also unbounded). When we PopLayer and paint that group into the main surface, we capture a copy-on-write snapshot surface into the recording.

When we replay the recording into the win32 print surface, we try to handle the paint being a recording surface, but it's not, it's the snapshot wrapper. Instead we fallback to trying to get an image surface, which crashes since it's unbounded.

This code doesn't handle snapshot wrappers around recording surfaces: https://searchfox.org/mozilla-central/rev/9c91451cc2392d942a42493fc895f5aeeddde45d/gfx/cairo/cairo/src/win32/cairo-win32-printing-surface.c#1122-1123

Comment 12

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1722188 - Handle unwrapping snapshot surfaces in win32 printing surface. r?jrmuizel

Comment 13

[Matt Woodrow (:mattwoodrow)]

Attachment: Bug 1722188 - Don't call PushLayer for no-op nsDisplayOpacity items. r?jrmuizel

Comment 14

[Matt Woodrow (:mattwoodrow)]

Second patch works around this issue in our code, and doesn't involve touching cairo. Might be easier, though the underlying bug still seems like it could be possible to hit in the future.

Will try turn the reduced test case into a reftest later on this afternoon.

Comment 16

[matthieu.lux]

Thanks a lot for the suggestion Jeff, I just tried several things around that particular animation. Indeed, removing it seems to fix the problem. It's quite big for us, if I commit that, we lose all entering animations of all modals of all apps so I continued a bit the investigation.

Overriding it at last minute or specifically in the print media doesn't work. Also I verified that it's only the opacity property which is important. I succeed by overriding it especially for this particular modal which is a lot more acceptable for us. My guess would be that it fails when the modal had been animated on the opacity before printing.

Last question, you seems to have already fixed the pb. What will be the estinated date for it to hit production? It would help me a lot to organize in consequence.

Thanks again.

Comment 17

[Jeff Muizelaar [:jrmuizel]]

I expect this will be fixed in 91 which is released August 10

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16b06a2c00b0
Don't call PushLayer for no-op nsDisplayOpacity items. r=jrmuizel

Comment 19

[Jeff Muizelaar [:jrmuizel]]

I landed the second patch, so that it will be on track for getting into RC

Comment 20

[Cosmin Sabou [:CosminS]]

Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1633189#c52. Thank you.

Comment 21

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/16b06a2c00b0

Comment 22

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9233854 [details]
Bug 1722188 - Don't call PushLayer for no-op nsDisplayOpacity items. r?jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Failed printing, with will-change:opacity and no real opacity value.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Super simple patch to just skip over no-op opacity groups.
• String changes made/needed:

Comment 23

[Julien Cristau [:jcristau] (back April 22)]

Comment on attachment 9233854 [details]
Bug 1722188 - Don't call PushLayer for no-op nsDisplayOpacity items. r?jrmuizel

approved for 91 rc1

Comment 25

[Julien Cristau [:jcristau] (back April 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/8aee7562fe15

Comment 26

[Petruta Horea [:phorea], Desktop QA]

Microsoft print to PDF works correctly on latest Nightly 92.0a1 2021-08-02 for the testcase presented here with ProtonMail and the tc from duplicate bug 1722173. Marking as verified.

Comment 27

[Petruta Horea [:phorea], Desktop QA]

Verified fixed on Firefox 91 RC build on Win 10 64-bit.