Closed Bug 1636678 Opened 4 years ago Closed 4 years ago

The SavePrintSettingsToPrefs call in nsPrintDialogServiceGTK::ShowPageSetup shouldn't save all settings

Categories

(Core :: Printing: Setup, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: jwatt, Assigned: hiro)

References

Details

(Whiteboard: [print2020_v78])

Attachments

(2 files)

This SavePrintSettingsToPrefs call was added when ventnor took over the work and added his first patch in bug 193001 comment 42. There's no explanation there as to why he added the call or specifically why the GTK code is special.

I'd note that the mac code essentially does an extra save in the same place (ShowPageSetup - see bug 1636668), but with kInitSaveNativeData rather than InitSaveAll.

(It might be helpful to note that kInitSaveNativeData isn't handled explicitly. That constant is essentially mac only because the mac code is the only platform to override nsPrintSettingsService::WritePrefs.)

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

I'd note that the mac code essentially does an extra save in the same place (ShowPageSetup - see bug 1636668), but with kInitSaveNativeData rather than InitSaveAll.

Turns out the Mac code is broken and does need to save more of the settings, but that was for the most part being hidden. (See the bug given above for details.)

So this code does appear to be necessary, but we should only really be saving the settings that the Page Setup dialog actually shows.

Hiro, do you want to take this?

Flags: needinfo?(hikezoe.birchill)
Summary: Investigate removing the SavePrintSettingsToPrefs call in nsPrintDialogServiceGTK::ShowPageSetup → The SavePrintSettingsToPrefs call in nsPrintDialogServiceGTK::ShowPageSetup shouldn't save all settings
Blocks: 1638143

Will look this tomorrow.

Flags: needinfo?(hikezoe.birchill)
Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED

That's all properties GtkPageSetup has [1].

[1] https://developer.gnome.org/gtk3/stable/GtkPageSetup.html

Depends on D75898

Pushed by hikezoe.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ef5347cdad10
Drop unnecessary `if (kInitSaveOddEvenPages)` blocks. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/8fdc6cd9df8b
Save the page size, unwritable margins and orientation in GTK. r=jwatt
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: