1669187 - Print preview settings are ignored on first print to a printer when `print.tab_modal.enabled` is set to `false`

Status: VERIFIED FIXED

(Toolkit :: Printing, defect, P1)

Description

[Ciprian Muresan [:cmuresan], Ecosystem QA, Out of the office until the 27th, ping mcoman, romartin, or poiegas for anything]

Attachment: cant switch orientation to landscape without printing first.gif

[Affected versions]:

• Firefox Nightly 83.0a1, Build ID 20201004212809
• Firefox Beta 82.0b7, BuildID 20201002191150

[Affected Platforms]:

• Windows 10
• Ubuntu 20.04

[Prerequisites]:

• Have the print.tab_modal.enabled set to false.

[Steps to reproduce]:

1. Open a new Firefox Beta profile and navigate to a printable page.
2. Click the Firefox Menu button and select the Print... option.
3. Click the Landscape option and observe the behavior.

[Expected results]:

• The page is refreshed and the orientation is changed to landscape.

[Actual results]:

• The page is refreshed and the orientation remains in portrait.

[Notes]:

• The issue is no longer reproducible after printing a page.
• Attached a screen recording of the issue.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Regression for the old ui. Tracking this for 82.

Comment 2

[Bob Owen (:bobowen)]

I can reproduce, I'll take a look.

Comment 3

[Bob Owen (:bobowen)]

Looks like this is another symptom of print_printer not being set until first print.

Comment 4

[Ciprian Muresan [:cmuresan], Ecosystem QA, Out of the office until the 27th, ping mcoman, romartin, or poiegas for anything]

I've searched for a regression window and have reached to following pushlog. I've confirmed this regression window on two different machines (Linux MX 4.19 and Windows 10).

Based on it, it looks like bug 1667953 has caused the issue and not bug 1668516 like I initially thought. I think we should reopen this issue and treat it separately from bug 1668621 based on this information and the fact that this behavior is not reproducible on Firefox Release 81 while bug 1668621 is.

@jwatt, could you please take a look at this issue? Is there anything in bug 1667953 that could have caused this?

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to Ciprian Muresan [:cmuresan], Ecosystem QA from comment #4)

@jwatt, could you please take a look at this issue? Is there anything in bug 1667953 that could have caused this?

Thanks for doing the bisect.

Yes, given what we know now, it makes perfect sense that bug 1667953 would cause this. The issue is that when print_printer hasn't been set (the user hasn't printed before) we implicitly save the print settings unprefixed when saving them in order to pass settings to the content process. Since https://phabricator.services.mozilla.com/D91819 made us ignore most unprefixed prefs in nsPrintSettingsService::InitPrintSettingsFromPrefs, we now fail to pick up those "passed settings" in the content process.

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: Bug 1669187. Make sure we have a last used printer saved in the common case. r=bobowen

Comment 10

[Jonathan Watt [:jwatt]]

The landed version of this patch uses the system default printer, if one is set. It does not further fall back to trying to get an available printer list using the nsIPrinterList as the original version of the patch did. That's because it seemed way too risky to uplift that to Beta, given that it requires making a bunch of code async, and the APIs to get the printer list and default settings from a printer have very variable delays according to our telemetry, for ~1% of users taking over (potentially way over) 10 seconds.

This patch fixed the issue for me on Window 10 and macOS 10.15.7. It did not fix things on Ubuntu 20.04. Again, the differences are whether a system default printer is set or not. It's possible the patch will not fix the issue for Windows 7 users (I don't know, yet), and it's possible that it will not fix the issue for macOS users who have never printed before using any app on their computer. However, the issue is only present for users using the old UI, who have never printed using Firefox before, and who are using Print Preview or the Page Settings dialog. (Our telemetry says that only around 7% of prints are made via print preview, and presumably a similarly small number of prints involve the Print Settings dialog.) That should be a relatively small subset of users affected (albeit mostly new users, the ones we're trying to retain), and the patch on this bug should significantly further reduce that subset of users given most of them are on Windows. As for the rest, I think we may have to live with the regression until we switch to the new print UI.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9270d4334587
Make sure we have a last used printer saved in the common case. r=bobowen,remote-protocol-reviewers

Comment 12

[Jonathan Watt [:jwatt]]

Comment on attachment 9180348 [details]
Bug 1669187. Make sure we have a last used printer saved in the common case. r=bobowen

Beta/Release Uplift Approval Request

• User impact if declined: Users printing for the first time who are using Print Preview or using the Page Setup dialog will have settings changes they make ignored, which is a very poor first user experience.
• 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): Bob and I have talked over the risk and we think it should be fairly low since for the most part it just reuses some existing tried and tested code, just in a different way in the particular case that the user has never printed before.
• String changes made/needed:

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9180348 [details]
Bug 1669187. Make sure we have a last used printer saved in the common case. r=bobowen

fix issues with settings on first print; approved for 82.0b9

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/88f215d632fe

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9270d4334587

Comment 16

[Ciprian Muresan [:cmuresan], Ecosystem QA, Out of the office until the 27th, ping mcoman, romartin, or poiegas for anything]

I have verified that changing the orientation to landscape and back, changing the scaling options, modifying the margins, and checking the image backgrounds and pictures options work as expected before printing a page on the latest Beta 82.0b9 (BuildID 20201008183927) using Windows 10 and Windows 7.
The issues are still reproducible on LinuxMX 4.19 and Ubuntu 20.04, however based on comment 10, this seems to be expected in this case. Based on this and the verification, I'll mark this issue as Verified.

Comment 17

[Bob Owen (:bobowen)]

(In reply to Ciprian Muresan [:cmuresan], Ecosystem QA from comment #16)

I have verified that changing the orientation to landscape and back, changing the scaling options, modifying the margins, and checking the image backgrounds and pictures options work as expected before printing a page on the latest Beta 82.0b9 (BuildID 20201008183927) using Windows 10 and Windows 7.
The issues are still reproducible on LinuxMX 4.19 and Ubuntu 20.04, however based on comment 10, this seems to be expected in this case. Based on this and the verification, I'll mark this issue as Verified.

Thanks, if you set a default printer on those linux machines, does that then work?

Comment 18

[Ciprian Muresan [:cmuresan], Ecosystem QA, Out of the office until the 27th, ping mcoman, romartin, or poiegas for anything]

If I'd need an actual printer I can't do that because I don't have one available, but I've reached out to Emil Ghitta and he might be able to check this.

Comment 19

[Anca Soncutean [:Anca], Desktop Release QA]

Thanks, if you set a default printer on those linux machines, does that then work?

By strictly opening a new profile and executing the steps from bug’s description with a printer connected, the issue is still reproducible. Changes made from Print or Print Setup still doesn't make any difference unless they are followed by an actual printing action.
The issue is fixed without executing any printing operation first, only if the print_printer pref is manually populated with the name of the used printer.

Comment 20

[Bob Owen (:bobowen)]

(In reply to Anca Soncutean [:Anca], Desktop Release QA from comment #19)

Thanks, if you set a default printer on those linux machines, does that then work?

By strictly opening a new profile and executing the steps from bug’s description with a printer connected, the issue is still reproducible. Changes made from Print or Print Setup still doesn't make any difference unless they are followed by an actual printing action.
The issue is fixed without executing any printing operation first, only if the print_printer pref is manually populated with the name of the used printer.

Did you try setting the printer as default in the operating system printer configuration tool?
I think on at least some (possibly most/all) linux systems a default printer isn't set automatically.

Comment 21

[Anca Soncutean [:Anca], Desktop Release QA]

(In reply to Bob Owen (:bobowen) from comment #20)

Did you try setting the printer as default in the operating system printer configuration tool?
I think on at least some (possibly most/all) linux systems a default printer isn't set automatically.

Yes, the printer wasn't set as default at the OS level, once I did that, indeed the issue is no longer reproducible (with latest Nightly/ latest Beta on Ubuntu 18.04)

Comment 22

[Maria Berlinger [:mberlinger]]

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