1659470 - [Windows][parent process about pages][print-to-pdf] Print preview infinite loading state and wrong output

Status: VERIFIED FIXED

(Core :: Print Preview, defect, P2)

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: loading.gif

Affected versions

• 81.0a1 (BuildId:20200817093723)

Affected platforms

• Windows 10 64bit

Unaffected platforms

• macOS 10.14
• Ubuntu 18.04 64bit.

Preconditions

• Ensure that the print.tab_modal.enabled pref is set to true

Steps to reproduce

1. Launch Firefox.
2. Access the about:support page.
3. Open print preview.
4. Switch between available destinations (ex: From printer x to Save to PDF)

Expected result

• The print preview gets successfully updated & the print to file or print to paper is successfully performed.

Actual result

• The print preview remains in an infinite loading state and clicking print returns in (see attachment for print to file) a blank page (for print to paper).

Regression Window

• This seems to be a regression:
• Potential Regressor: Bug 1658043

Additional Information

• The following errors are being outputted inside the browser console while triggering this:
  [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIPrintSettingsService.initPrintSettingsFromPrinter]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: resource://gre/actors/PrintingChild.jsm :: getPrintSettings :: line 126" data: no] PrintingChild.jsm:126:13
can't access property "showPrintProgress", printSettings is null PrintingChild.jsm:322
• For further information regarding this issue please observe the attached screencast.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Olli!

It seems that mozregression pointed out Bug 1658454 - nsISHEntry.cacheKey setter for session-history-in-parent, r=peterv for causing this regression.

Can you please take a look?

Thank you!

Comment 2

[Emil Ghitta, QA [:emilghitta]]

Attachment: printToFileAboutSupport.pdf

Comment 3

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

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

Comment 4

[Mark Striemer [:mstriemer]]

This only happens for me for the Save to PDF printer. I can switch between my other printers on Windows, but once I select Save to PDF the preview stops working.

Comment 5

[Olli Pettay [:smaug]]

Super weird. I accidentally noticed this bug, but I never got any bugmail about this.
However, bug 1658454 most probably didn't regress this.

https://hg.mozilla.org/mozilla-central/rev/9d3ade989dabfcdf2a63e1c9b71edcf27ea48c50 which happened a bit before bug 1658454 smells way more likely.

Comment 6

[Olli Pettay [:smaug]]

And I can't reproduce this anyhow.

Emil, if you can reproduce this, could you check if https://hg.mozilla.org/mozilla-central/rev/9d3ade989dabfcdf2a63e1c9b71edcf27ea48c50 regressed this?

Comment 7

[Emil Ghitta, QA [:emilghitta]]

Attachment: repro.gif

I can still reproduce this behavior. Indeed Bug 1658043 seems to be the better candidate for causing this regression (tried with the last release without & first release with from https://hg.mozilla.org/mozilla-central/rev/9d3ade989dabfcdf2a63e1c9b71edcf27ea48c50)

I'm going to update this Bug to better reflect this.

Thank you Olli ! And sorry for pointing out the wrong patch!

Comment 8

[Jens Stutte [:jstutte]]

Hi Mike, is there someone who could look at this any time soon? Thank you!

Comment 9

[:Gijs (he/him)]

I can reproduce on my Windows machine, but not on macOS.

There are exceptions in the browser console - initPrintSettingsFromPrinter is throwing NS_ERROR_FAILURE. :jwatt, is that expected for the PDF printer, or something?

Comment 10

[Jonathan Watt [:jwatt]]

(In reply to :Gijs (he/him) from comment #9)

There are exceptions in the browser console - initPrintSettingsFromPrinter is throwing NS_ERROR_FAILURE. :jwatt, is that expected for the PDF printer, or something?

For our own "Save to PDF" pseudo "printer" it would be. But we have code to guard against that, don't we?

Comment 11

[:Gijs (he/him)]

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

(In reply to :Gijs (he/him) from comment #9)

There are exceptions in the browser console - initPrintSettingsFromPrinter is throwing NS_ERROR_FAILURE. :jwatt, is that expected for the PDF printer, or something?

For our own "Save to PDF" pseudo "printer" it would be. But we have code to guard against that, don't we?

Not in PrintingChild.jsm we don't...

Comment 12

[:Gijs (he/him)]

Also not in setPrinterDefaultsForSelectedPrinter, in pringPageSetup.js .

Comment 13

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #11)

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

(In reply to :Gijs (he/him) from comment #9)

There are exceptions in the browser console - initPrintSettingsFromPrinter is throwing NS_ERROR_FAILURE. :jwatt, is that expected for the PDF printer, or something?

For our own "Save to PDF" pseudo "printer" it would be. But we have code to guard against that, don't we?

Not in PrintingChild.jsm we don't...

Guarding this access alone doesn't appear to be sufficient to fix this issue - we get the same behaviour but an error dialog now also shows up, because

          docShell
            .initOrReusePrintPreviewViewer()
            .printPreview(printSettings, contentWindow, listener);

at https://searchfox.org/mozilla-central/rev/d54210d490ef335b13fc1fcac817525120c8c46b/toolkit/actors/PrintingChild.jsm#370-372 throws (in the printPreview call).

Happy to keep debugging but not sure where to go from here.

Edit:

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIWebBrowserPrint.printPreview]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/actors/PrintingChild.jsm :: printPreviewInitialize :: line 376"  data: no] PrintingChild.jsm:376:14
    printPreviewInitialize resource://gre/actors/PrintingChild.jsm:376

is the precise error if that helps.

Comment 14

[Jonathan Watt [:jwatt]]

(In reply to :Gijs (he/him) from comment #11)

Not in PrintingChild.jsm we don't...

For some reason I had in mind that being loaded in the content process == errors go to the Web Console (not Browser Console). :/

(In reply to :Gijs (he/him) from comment #12)

Also not in setPrinterDefaultsForSelectedPrinter, in pringPageSetup.js .

I'm pretty sure that file isn't used by the new UI.

Comment 15

[Jonathan Watt [:jwatt]]

This really needs someone to walk through what's happening under nsPrintJob::DoCommonPrint in a debugger. Can you post your new guard code to save that person writing the same thing?

Comment 16

[:Gijs (he/him)]

As noted on Matrix:

• despite this code being in PrintingChild.jsm, we're running in the parent (because it's a parent-process about: page)
• PrintingChild.jsm needs to have the same "is this the PDF printer" check, or a try...catch (as it appears to work on non-Windows?)
• further failure is because when previewing, the settings for the PDF printer do not have a filename set. If we print child process pages, we create a nsDeviceContextSpecProxy which doesn't care. But if we're in the parent process, we try to create an nsDeviceContextSpecWin instance, which in the PDF case creates an nsIFile instance and tries to initWithPath it with the specified path (ie ""). That fails, which means we fail to create the print target, which causes the printPreview() call to fail.

:jwatt suggested we may not want to create a print target at all in the print preview case? Unsure if that is the most expedient solution here.

Anyway, I think we should maybe downgrade this issue's severity as it only affects parent process pages, which are relatively rare, and it only affects printing to PDF. Unless we think it's going to be very common for people to be attempting to print about:support and similar to PDF... :jstutte, thoughts?

Comment 17

[Jens Stutte [:jstutte]]

I agree, this sounds much less scary then.

Comment 18

[Hiroyuki Ikezoe (:hiro)]

Now it turned out it's scary since it causes a crash (bug 1663426).

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1659470 - Handle printing with an empty file name in nsDeviceContextSpecWin. r=jwatt,bobowen

Ideally print preview and co. shouldn't need a PrintTarget at all, I'd think...
Though that's a bigger refactoring.

Comment 20

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

(It sounds like this is a core bug…)

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b6be24539b1d
Handle printing with an empty file name in nsDeviceContextSpecWin. r=jwatt

Comment 22

[Narcis Beleuzu [:NarcisB]]

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

Comment 23

[Bogdan Maris [:bogdan_maris], Release Desktop QA]

Reproduced the initial issue using an old Nightly build (2020-08-17), verified that using Firefox 82.0b3 the print preview is opened but still suffers from a delay (bug 1665272). Using Nightly 83.0a1 which also contains the fix from bug 1665272 the print preview opens almost instantly so I will call this as verified fixed since once the patch from 1665272 will land in Beta 82 it will have similar performance as well.

Comment 24

[Maria Berlinger [:mberlinger]]

https://testrail.stage.mozaws.net/index.php?/cases/view/1084722