1401899 - [e10s] Print doesn't use the corresponded print preference on macOS.

Status: RESOLVED DUPLICATE of bug 117233

(Toolkit :: Printing, enhancement, P3)

Description

[Mantaroh Yoshinaga[:mantaroh]]

Currently, Gecko will not store printer settings into corresponded preference on macOS.

In macOS, gecko will load print settings from global printer settings(i.e. print.print_**. '**' is preference name like print.print_footerleft) since nsPrintOptionsX doesn't implement the nsIPrinterEnumerator. So gecko can't get the printer name.[1]

However gecko will store the print setting into printer preference when we push the 'OK' button of native print dialog. In this case, gecko will use printer name which gets from native print dialog. So gecko store this print option per printer. (i.e. print.<printer_name>.print_**) [2]

As result, Gecko doesn't carry the print settings from previous job.

I think that some bugs related to this bug.

[1] https://dxr.mozilla.org/mozilla-central/rev/319a34bea9e4f3459886b5b9e835bd338320f1fd/widget/nsPrintOptionsImpl.cpp#988
[2] https://dxr.mozilla.org/mozilla-central/rev/319a34bea9e4f3459886b5b9e835bd338320f1fd/widget/cocoa/nsPrintOptionsX.mm#75-78

Step To Reproduce:
 1) Open Print dialog
 2) Change print settings. (e.g. footer center to 'Title')
 3) Print or Save as PDF
 4) Re-Open Print dialog

Actually Result:
 Print settings is default, and generated print settings per printer(print.<printer_name>.print).

Comment 1

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: Bug 1401899 - Don't use the printer name which gets from NSPrintInfo for storing the printer settings on macOS.

The print widget of cocoa doesn't have nsIPrinterEnumerator implementation,
so gecko doesn't get the printer name correctly and gecko use global printer
settings which are named print.print_** preference.

If gecko use printer name which gets from NSPrintInfo, the gecko will store
current printer settings to the preference which is related to the printer name.
(i.e. print.<printer_name>.print_**)
So the gecko doesn't apply previous printer settings to next print job.

This patch will remove serialize/deserialize printer name in order to use global
printer settings.

Review commit: https://reviewboard.mozilla.org/r/182514/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182514/

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hi mantaroh, sorry it took me so long to get to this!

I'm not sure I fully understand, so maybe you can clear some things up for me:

1) Based on my read of the code, and your comment 0, it sounds like we're storing printing preferences keyed on printer names. Is that correct?

2) It also sounds like when we open up the print dialog, we're either not restoring those original settings from prefs, or we have the wrong printer selected. Is that correct?

Is my conception of the problem more or less accurate?

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8911033 [details]
Bug 1401899 - Don't use the printer name which gets from NSPrintInfo for storing the printer settings on macOS.

https://reviewboard.mozilla.org/r/182514/#review191062

Clearing review until I get some questions cleared up.

Comment 4

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #2)
> Hi mantaroh, sorry it took me so long to get to this!
> 

Mike, 
Thank you for confirming it.


> I'm not sure I fully understand, so maybe you can clear some things up for
> me:
> 
> 1) Based on my read of the code, and your comment 0, it sounds like we're
> storing printing preferences keyed on printer names. Is that correct?
> 

Yes, current code store printing preferences per printer. This printer name is propagated via NSPrintInfo when serializing printer settings after closing print dialog, and gecko use this printer name for storing preferences. 

This preference which related to printer name is introduced by bug 128142. And we should implemet nsIPrinterEnumerator on macOS, however we doesn't implement it yet (Bug 117233). So I think that we should use global print setting on macOS.

> 2) It also sounds like when we open up the print dialog, we're either not
> restoring those original settings from prefs, or we have the wrong printer
> selected. Is that correct?
> 

Yes, when we open the print dialog, nsPrintOptionImpl restoring global print preferences(i.e. print.printer_** key), not related to printer name(i.e. print.<printer_name>.printer_** key). In this dialog, selected printer is depended on macOS's printers&scanner preference. (e.g. If user set 'Default Printer' to 'Last printer used', selected printer is last used printer.)

Comment 5

[Mike Conley (:mconley) (:⚙️)]

I see. It seems then that maybe the better solution is to try to (finally!) fix bug 117233 instead of wallpapering over it. Would you be willing to try that?

Comment 6

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #5)
> I see. It seems then that maybe the better solution is to try to (finally!)
> fix bug 117233 instead of wallpapering over it. Would you be willing to try
> that?

I investigated what we need for implementing this feature:
 * Implementation of nsIPrinterEnumerator
   * we can use PMServerCreatePrinterList or PMSessionCreatePrinterList for getting printer lists. [1][2]
     I'm prefer PMSessionCreatePrinterList since we can get current printer(i.e. default printer).
   * run this service on main process only, maybe we can't use this code on content process.
 * Implementation of reading/storing print preference per printer.
   * nsIPrintSettingsService use printer name for reading/storing print preferences if we can get the default printer name via nsIPrinterEnumerator[3][4][5]. So I guess that we don't need to change around this mechanism.

[1] https://developer.apple.com/documentation/applicationservices/1463247-pmcreatesession?language=objc
[2] https://developer.apple.com/documentation/applicationservices/1460119-pmsessioncreateprinterlist?language=objc
[3] http://searchfox.org/mozilla-central/rev/7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1153,1195
[4] http://searchfox.org/mozilla-central/rev/7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1108
[5] http://searchfox.org/mozilla-central/rev/7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1018-1052

Comment 7

[Mantaroh Yoshinaga[:mantaroh]]

Note to self.
 * nsIPrinterEnumerator::GetDefaultPrinterName should use PMPSessionCreatePrinterList() in order to distinguish the selected printer(i.e. default printer).
 * We might create PMPrintSession during this GetDefaultPrinterName function. I.e. We don't need have the PMPrintSession as class member.
 * nsIPrinterEnumerator::GetPrinterNameList should use PMServerCreatePrinterList() since we don't need get the default printer.
 * nsIPrinterEnumerator::InitPrintSettingsFromPrinter should set PMPrintSettings and PMPageFormat.

Comment 8

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #6)
> (In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from
> comment #5)
> > I see. It seems then that maybe the better solution is to try to (finally!)
> > fix bug 117233 instead of wallpapering over it. Would you be willing to try
> > that?
> 
> I investigated what we need for implementing this feature:
>  * Implementation of nsIPrinterEnumerator
>    * we can use PMServerCreatePrinterList or PMSessionCreatePrinterList for
> getting printer lists. [1][2]
>      I'm prefer PMSessionCreatePrinterList since we can get current
> printer(i.e. default printer).
>    * run this service on main process only, maybe we can't use this code on
> content process.
>  * Implementation of reading/storing print preference per printer.
>    * nsIPrintSettingsService use printer name for reading/storing print
> preferences if we can get the default printer name via
> nsIPrinterEnumerator[3][4][5]. So I guess that we don't need to change
> around this mechanism.
> 
> [1]
> https://developer.apple.com/documentation/applicationservices/1463247-
> pmcreatesession?language=objc
> [2]
> https://developer.apple.com/documentation/applicationservices/1460119-
> pmsessioncreateprinterlist?language=objc
> [3]
> http://searchfox.org/mozilla-central/rev/
> 7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1153,
> 1195
> [4]
> http://searchfox.org/mozilla-central/rev/
> 7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1108
> [5]
> http://searchfox.org/mozilla-central/rev/
> 7ba03cebf1dcf3b4c40c85a4efad42a5022cf1ec/widget/nsPrintOptionsImpl.cpp#1018-
> 1052

Furthermore..

The page setup preference (e.g. scaling / paper size / orientation) save to preferences as binary data[1].
So these preferences of each printer doesn't affect print (see 1346649), maybe we should store these settings to each printer's preferences.

Comment 9

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews from comment #5)
> I see. It seems then that maybe the better solution is to try to (finally!)
> fix bug 117233 instead of wallpapering over it. Would you be willing to try
> that?

OK. I've implemented the prototype for this, and clarify the problems(comment 6 / comment 7 / comment 8).

I think I need a little time for implementing it, So I want to choose the workaround until landed this 'each printer preferences' implementation. What do you think?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> I think I need a little time for implementing it, So I want to choose the
> workaround until landed this 'each printer preferences' implementation. What
> do you think?

Hey mantaroh,

How much time do you need? If the scale of time is "days", then we should probably just take that instead of landing the workaround. If the scale of time is "months", then we should probably land this workaround.

So do you have a sense of how long it'll take?

Comment 11

[Mantaroh Yoshinaga[:mantaroh]]

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #10)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> > I think I need a little time for implementing it, So I want to choose the
> > workaround until landed this 'each printer preferences' implementation. What
> > do you think?
> 
> Hey mantaroh,
> 
> How much time do you need? If the scale of time is "days", then we should
> probably just take that instead of landing the workaround. If the scale of
> time is "months", then we should probably land this workaround.
> 
> So do you have a sense of how long it'll take?

OK. Maybe it will take few days, I think.
So I'll skip this workaround, and I try to implement it.
Thanks.

Comment 12

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: WIP: Part1. Add empty printer enumerator service

Comment 13

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: WIP: Part2. Implement printerEnumeratorService for macOS

Comment 14

[Mantaroh Yoshinaga[:mantaroh]]

Current (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #13)
> Created attachment 8920939 [details] [diff] [review]
> WIP: Part2. Implement printerEnumeratorService for macOS

This patch will not apply creating printer settings from real printer information(i.e. not implemented InitPrintSettingsFromPrinter) yet.

Note to self:
Before implement this bug, We need to stop overwrite the printer name from NSPrintInfo on content process.[1]
Now, we have 2 nsIPrintSettings (i.e. parent / content processes). As Result of it, we don't sent corresponded printer name from content process to parent process even if we implement printer enumerator.

[1] http://searchfox.org/mozilla-central/rev/8a6a6bef7c54425970aa4fb039cc6463a19c0b7f/widget/cocoa/nsPrintOptionsX.mm#75-78

Comment 15

[Mantaroh Yoshinaga[:mantaroh]]

Attachment: WIP: Part3. Skip to copy the printer name from NSPrintInfo when serializing print data on content process.

Comment 16

[Jonathan Watt [:jwatt]]

Erik, can you check if your patches for bug 117233 fixed this bug also, or if we have further work to do?

Comment 17

[Erik Nordin [:nordzilla]]

The functionality described in this bug appears to work correctly both in the current nightly 78.0a1 (2020-05-28) and in my patch.

Step To Reproduce

1. Open Print dialog
2. Change print settings. (e.g. footer center to 'Title')
3. Print or Save as PDF
4. Re-Open Print dialog

Expected Results
The print dialog retains the setting from the previous print.

Actual Results
Same as expected