Bug 1691286: Fix nsPrintSettingsWin::CopyFromNative to reliably encode the pages' orientation in the mOrientation field (fixing mismatched landscape/portrait rendering in some cases). r=jfkthame, a=pascalc

dholbert · dholbert · commit b9943ffb5fb5 · 2021-02-08T23:01:37.000Z
This was just a silly typo; I got the order of the ternary expression's options backwards. The mistake wasn't obvious from testing, because this function isn't always invoked. In particular, it's not invoked if I print "https://example.org" in a fresh profile, but it *is* invoked if I print "about:support" or other browser-internal pages. Differential Revision: https://phabricator.services.mozilla.com/D104354
diff --git a/layout/base/tests/chrome/chrome.ini b/layout/base/tests/chrome/chrome.ini
@@ -103,6 +103,7 @@ skip-if = (verify && (os == 'win'))
 skip-if = webrender
 [test_get_printer_basic_attributes.html]
 [test_get_printer_paper_sizes.html]
+[test_get_printer_orientation.html]
 [test_getClientRectsAndTexts.html]
 [test_css_visibility_propagation.xhtml]
 support-files =
diff --git a/layout/base/tests/chrome/test_get_printer_orientation.html b/layout/base/tests/chrome/test_get_printer_orientation.html
@@ -0,0 +1,50 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+</head>
+<body onload="run()">
+<script>
+
+SimpleTest.waitForExplicitFinish();
+
+async function run() {
+  try {
+    let printerList = Cc["@mozilla.org/gfx/printerlist;1"].getService(
+      Ci.nsIPrinterList
+    );
+    var settingsSvc = Cc["@mozilla.org/gfx/printsettings-service;1"].getService(
+      Ci.nsIPrintSettingsService
+    );
+
+    let printers = await printerList.printers;
+    for (let printer of printers) {
+      printer.QueryInterface(Ci.nsIPrinter);
+      let printerInfo = await printer.printerInfo;
+
+      // Look up the printer's defaultSettings:
+      let defaultSettings = printerInfo.defaultSettings;
+
+      // Let the printer impose its defaults onto a fresh settings object:
+      let freshSettings = settingsSvc.newPrintSettings;
+      printerList.initPrintSettingsFromPrinter(printer.name, freshSettings);
+
+      // Make sure they agree on the default orientation:
+      is(freshSettings.orientation, defaultSettings.orientation,
+         "initPrintSettingsFromPrinter should produce the same orientation " +
+         "as the printer's defaultSettings");
+    }
+
+    // This ok() just lets us avoid failure-due-to-no-tests-being-run, on
+    // devices that have no printers available & hence skip the loop above:
+    ok(true, "Finished traversing printers.");
+  } catch (e) {
+    ok(false, `Error thrown while retrieving printer info: ${e}.`);
+    Cu.reportError(e);
+  }
+  SimpleTest.finish();
+}
+
+</script>
+</body>
+</html>
diff --git a/widget/windows/nsPrintSettingsWin.cpp b/widget/windows/nsPrintSettingsWin.cpp
@@ -334,8 +334,9 @@ void nsPrintSettingsWin::CopyFromNative(HDC aHdc, DEVMODEW* aDevMode) {
     const bool arePagesPortraitMode =
         (areSheetsOfPaperPortraitMode != HasOrthogonalSheetsAndPages());
 
-    mOrientation = int32_t(arePagesPortraitMode ? kLandscapeOrientation
-                                                : kPortraitOrientation);
+    // Record the orientation of the pages (determined above) in mOrientation:
+    mOrientation = int32_t(arePagesPortraitMode ? kPortraitOrientation
+                                                : kLandscapeOrientation);
   }
 
   if (aDevMode->dmFields & DM_COPIES) {
