553154 - [10.6] Print settings are not remembered

Status: RESOLVED FIXED

(Camino Graveyard :: Printing, defect)

Description

[philippe (part-time)]

STR, in Print dialog
1. configure print settings, such as 'Print background colors / images' or 'Headers' / 'Footers'
2. print or preview or save as PDF
3. open a new page, try to print

A.R:  the settings have reverted to what was set before the dialog was previously opened
E.R: the settings are remembered

Toggling those settings while having about:config open, filtered for 'print', shows that the settings are _not_ toggled in about:config.

As far as I have been able to test, Minefield and Fx 3.6 nighties work correctly.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I don't see this on 10.5, and I'm really confused because all of the printing stuff should be entirely in Gecko, as a black box to us.

You don't see anything in the console, do you?

Comment 2

[philippe (part-time)]

(In reply to comment #1)
 
> You don't see anything in the console, do you?
Unfortunately, no.
Thanks for testing on 10.5, btw.

Comment 3

[Lisa Thompson]

I can confirm what Philippe is seeing, on 10.6.2. It seems the print settings stick with a tab. Continuing from the STR:

4. go back to the tab you printed from in step 2
5. open the print dialog and the two boxes in step 1 are checked again

Moreover, I'm seeing this on all versions of Camino: 2.1a1pre/1.9.2, 2.1a1pre/1.9.0.x, 2.0.2 and 1.6.10, tested with fresh profiles. But not on Firefox 3.5 or 3.6.

I'm also seeing what I guess is a separate Core issue in Camino 2.1a1pre/1.9.2 only, as well as Firefox 3.6 -- printing presets are not remembered or respected. I mention this only to note that I deleted all my presets before testing the issue in this bug to try to prevent any, uh, cross-contamination in the results.

Comment 4

[Markus Stange [:mstange]]

Saving the settings happens in printService.savePrintSettingsToPrefs. Does Camino call that?
I'm also very confused that this bug is platform-dependent.
I didn't have time to set up a Camino 1.9.2 build yet, so I can't debug it.

Comment 5

[Lisa Thompson]

Not sure what it means, but I just found the same issue in TextWrangler: using a single window with multiple documents open, changed settings will stick to a document, but do not carry across documents.

I looked at Apple apps to make sure I understood "normal" behavior. In TextEdit and Font Book, which add their own modules to the print dialog, settings in the app-specific modules will stick as Standard settings across docs (or fonts), but settings in the OS-wide and printer-specific modules (eg. Layout and Cover Page) do not stick, even when you immediately print the same document; that's also true in apps that do not add their own printing modules, eg. Pages. I don't have a 10.5 boot drive atm, but it seems from memory that's how printing worked there as well.

So Camino (and TextWrangler) under 10.6 differs from Apple app printing in two ways:
a. settings in the app-specific module do not persist across documents (tabs)
b. settings in the OS-wide modules do persist when printing the same tab

Regarding the other issue I mentioned, not being able to save presets, I now see that this applies only to saving settings in the app-specific module, and TextEdit and Font Book seem to have the same "bug", if that's what it is.

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #4)
> Saving the settings happens in printService.savePrintSettingsToPrefs. Does
> Camino call that?

http://mxr.mozilla.org/seamonkey/source/camino/src/embedding/CHBrowserView.mm#908 looks like where we call that (no mxr for the 1.9.2-test repo, but this code hasn't changed).

Comment 7

[philippe (part-time)]

(In reply to comment #5)
> Not sure what it means, but I just found the same issue in TextWrangler: using
> a single window with multiple documents open, changed settings will stick to a
> document, but do not carry across documents.

I've now tested this with SubEthaEdit, which has an app-specific module (& window with multiple tabs/documents) and it shows the same issue: app-specific settings are not remembered across documents & presets don't save anything in the app-specific module.

Safari on the other hand always remembers app-specific settings; they are however not saved in presets.

Smultron (the 3.7 'fork') also remembers settings in the app module across documents.

On 10.5, SubEthaEdit does remember those app-specific settings across documents (as tabs or in separate windows).

Comment 8

[Markus Stange [:mstange]]

Attachment: use kInitSaveAll flag

I don't know why this bug wouldn't exist on 10.5, or even on 1.9.0, but this fixes it for me. nsPrintOptions::WritePrefs didn't save the prefs because CHBrowserView didn't pass the right save flags. So this patch changes kInitSaveNativeData to kInitSaveAll and re-reads the prefs whenever ensurePrintSettings is called so that the options are synced after switching tabs.

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Thanks, Markus!

(In reply to comment #8)
> I don't know why this bug wouldn't exist on 10.5, or even on 1.9.0

I wonder if we're just getting lucky in those cases (and should take this on 1.9.0)?

Actually, I can reproduce the per-tab nature of the settings on 10.5 (1.9.0 and 1.9.2), though most of the time on 1.9.0 I end up losing the PDE, even if there's no plug-in content on the page :-(

Comment 10

[Stuart Morgan]

I'm not familiar with this codeis it kosher to call InitPrintSettingsFromPrefs repeatedly on the same object?

Comment 11

[Markus Stange [:mstange]]

Comment on attachment 435621 [details] [diff] [review]
use kInitSaveAll flag

It's not - calling it a second time is a no-op. I wonder why I thought it worked.

Comment 12

[Markus Stange [:mstange]]

Comment on attachment 435621 [details] [diff] [review]
use kInitSaveAll flag

I thought it worked because, in fact, it *does* work. This code is a little confusing...

InitPrintSettingsFromPrefs does this:
 if isInitializedFromPrefs return;
 read general prefs;
 if !aUsePNP (which we set to false) return;
 read printer-specific prefs;
 isInitializedFromPrefs = true

So in our case it's only calling ReadPrefs for the general case. We don't hit "isInitializedFromPrefs = true" because we're passing false for aUsePNP.
I've found nothing in ReadPrefs that wouldn't survive being done twice.

Comment 13

[Stuart Morgan]

It sounds like this is sort of only working by accident then. Would it make more sense to just recreate mPrintSettings each time? It looks like we save after each operation, so presumably we wouldn't lose any settings.

Comment 14

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Related, I think: I see this in my 2.0.3pre debug build when going to print:

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file /Users/smokey/Camino/dev/trunk/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1112

^G###!!! ASSERTION: nsIPrintSettingsX::ReadPageFormatFromPrefs() failed: 'NS_SUCCEEDED(rv)', file /Users/smokey/Camino/dev/trunk/mozilla/widget/src/cocoa/nsPrintOptionsX.mm, line 113

WARNING: Caller should supply a printer name.: file /Users/smokey/Camino/dev/trunk/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1068

and worse (more failures) with 2.1a1pre-M1.9.2:

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1107

^G###!!! ASSERTION: nsPrintSettingsX::ReadPageFormatFromPrefs() failed: 'NS_SUCCEEDED(rv)', file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/cocoa/nsPrintOptionsX.mm, line 64

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x8000FFFF: file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1055

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004005: file /Users/smokey/Camino/dev/192branch/mozilla/netwerk/base/src/nsIOService.cpp, line 583

WARNING: Couldn't get AppShellService in order to get hidden window ref: file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/cocoa/nsCocoaUtils.mm, line 249

WARNING: NS_ENSURE_TRUE(aStr) failed: file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1118

WARNING: Caller should supply a printer name.: file /Users/smokey/Camino/dev/192branch/mozilla/widget/src/xpwidgets/nsPrintOptionsImpl.cpp, line 1063

Comment 15

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #14)
> and worse (more failures) with 2.1a1pre-M1.9.2:

(That build has attachment 435621 [details] [diff] [review] applied, fwiw.)

Comment 16

[Daniel Holbert [:dholbert]]

(In reply to comment #14)
> ^G###!!! ASSERTION: nsIPrintSettingsX::ReadPageFormatFromPrefs() failed:

NOTE: On tinderbox, our mac reftest runs fail this same assertion many times per run, on both 10.5 & 10.6.  I filed Bug 564071 on that.

Comment 17

[Markus Stange [:mstange]]

Attachment: recreate mPrintSettings each time

Comment 18

[Stuart Morgan]

Comment on attachment 446316 [details] [diff] [review]
recreate mPrintSettings each time

sr=smorgan. Thanks!

Comment 19

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Markus, feel free to land this in Hg and on CVS trunk, or let me know if I should land it instead.  And thanks again for figuring this out!

Comment 20

[Markus Stange [:mstange]]

Landed on Hg: http://hg.mozilla.org/camino/rev/46318e60bc5c
Can you do the CVS checkin for me, please?

Comment 21

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #20)
> Can you do the CVS checkin for me, please?

Landed on CVS trunk.

Comment 22

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since we were landing a bunch of other stuff on the branch tonight, I also landed this on CAMINO_2_0_BRANCH in advance of 2.0.4.