1661823 - nsIPrinter.CreateDefaultSettings() converts to the wrong units for unwriteableMargins

Status: RESOLVED FIXED

(Core :: Printing: Setup, defect, P1)

Description

[Erik Nordin [:nordzilla]]

When I implemented bug 1658299 I saw this comment which suggests that the unwriteable margins for print settings are stored in TWIPS.

While this is true, I didn't look high enough to the IDL, which specifies that the setters expect inches, and will convert internally to TWIPS as appropriate.

As a result, when creating the default settings, the code prematurely converts to TWIPS to pass the argument to the setter, which will result in incorrectly running a conversion from inches to TWIPS on units that are already in TWIPS.

As such, I need to change this conversion to indiscriminately convert to inches before passing into the argument.

Comment 1

[Erik Nordin [:nordzilla]]

At the time of creating this bug:

• The shared base class implementation assumes the argument is in inches and converts it into TWIPS to store internally.

• The GTK print settings implementation calls the base class setter (which converts inches to TWIPS), and converts the TWIPS back to inches for the internal settings:

• The macOS print settings implementation calls the base class setter, which converts inches to TWIPS.

• The Windows print settings implementation implementation does not override the base class default, so it also expects inches and converts to TWIPS.

Comment 2

[Erik Nordin [:nordzilla]]

Attachment: Bug 1661823 - Fix Default Print Settings Unwriteable Margins Units r=alaskanemily

Print settings stores margin units in TWIPS, but the API expects inches.
This was causing an extra conversion from inches to TWIPS on values that
were already in TWIPS.

Comment 3

[Erik Nordin [:nordzilla]]

Thankfully, the front-end code is not using nsIPrinter.createDefaultSettings() yet, so the code path with the defect is not being exercised.

This change should be totally safe to uplift into 81 beta.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5d4bad92465
Fix Default Print Settings Unwriteable Margins Units r=AlaskanEmily

Comment 5

[Cosmin Sabou [:CosminS]]

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

Comment 6

[Jonathan Watt [:jwatt]]

Reminder to request Beta uplift.

Comment 7

[Jonathan Watt [:jwatt]]

Comment on attachment 9172795 [details]
Bug 1661823 - Fix Default Print Settings Unwriteable Margins Units r=alaskanemily

Beta/Release Uplift Approval Request

• User impact if declined: Default print settings retrieved from printer via new createDefaultSettings() will have incorrect margins, potentially resulting in actual print output being cropped.
• 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): This function is not used yet in print.js but it will prevent a potentially serious bug once it is.
• String changes made/needed:

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9172795 [details]
Bug 1661823 - Fix Default Print Settings Unwriteable Margins Units r=alaskanemily

Approved for 81.0b5.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0ec3a7607e34