Closed Bug 1661823 Opened 3 months ago Closed 3 months ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox81 --- fixed
firefox82 --- fixed

People

(Reporter: nordzilla, Assigned: nordzilla)

Details

(Whiteboard: [print2020_v81])

Attachments

(1 file)

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.

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.

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.

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.

Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f5d4bad92465
Fix Default Print Settings Unwriteable Margins Units r=AlaskanEmily
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Reminder to request Beta uplift.

Flags: needinfo?(enordin)

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:
Attachment #9172795 - Flags: approval-mozilla-beta?
Flags: needinfo?(enordin)

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

Approved for 81.0b5.

Attachment #9172795 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.