From de2b36d2d8f4906b0d2784bd24f1ca6343eca574 Mon Sep 17 00:00:00 2001 From: Sam Foster Date: Wed, 14 Oct 2020 19:51:35 +0000 Subject: [PATCH] Bug 1666523 - Update paperSizeUnit whenever we update paper size settings. r=mstriemer * Only use paperWidth/paperHeight values from the settings when searching for a paper size match * Calculate unit-appropriate values for paperWidth, paperHeight and write them to settings whenever a paper size is selected * Always write paperSizeUnit and the width/height values to settings to ensure any bad data from prefs is overwritten Differential Revision: https://phabricator.services.mozilla.com/D91021 --- toolkit/components/printing/content/print.js | 150 ++++++++++-------- toolkit/components/printing/tests/browser.ini | 1 + .../tests/browser_print_paper_sizes.js | 143 +++++++++++++++++ toolkit/components/printing/tests/head.js | 7 +- 4 files changed, 230 insertions(+), 71 deletions(-) create mode 100644 toolkit/components/printing/tests/browser_print_paper_sizes.js diff --git a/toolkit/components/printing/content/print.js b/toolkit/components/printing/content/print.js index 5cbcbda9f1fb..bb3c06fe5de5 100644 --- a/toolkit/components/printing/content/print.js +++ b/toolkit/components/printing/content/print.js @@ -419,21 +419,7 @@ var PrintEventHandler = { } // See if the paperId needs to change. - let paperId, paperWidth, paperHeight, paperSizeUnit; - if (settingsToUpdate.paperId) { - // The user changed paperId in this instance and session, - // We should have details on the paper size from the previous printer - paperId = settingsToUpdate.paperId; - let cachedPaperSize = this.allPaperSizes[paperId]; - paperWidth = cachedPaperSize.width; - paperHeight = cachedPaperSize.height; - paperSizeUnit = cachedPaperSize.paperSizeUnit; - } else { - paperId = this.viewSettings.paperId; - paperWidth = this.viewSettings.paperWidth; - paperHeight = this.viewSettings.paperHeight; - paperSizeUnit = this.viewSettings.paperSizeUnit; - } + let paperId = settingsToUpdate.paperId || this.viewSettings.paperId; logger.debug("Using paperId: ", paperId); logger.debug( @@ -441,13 +427,30 @@ var PrintEventHandler = { PrintSettingsViewProxy.availablePaperSizes ); let matchedPaper = - paperId && - PrintSettingsViewProxy.getBestPaperMatch( - paperId, + paperId && PrintSettingsViewProxy.availablePaperSizes[paperId]; + if (!matchedPaper) { + let paperWidth, paperHeight, paperSizeUnit; + if (settingsToUpdate.paperId) { + // The user changed paperId in this instance and session, + // We should have details on the paper size from the previous printer + paperId = settingsToUpdate.paperId; + let cachedPaperWrapper = this.allPaperSizes[paperId]; + // for the purposes of finding a best-size match, we'll use mm + paperWidth = cachedPaperWrapper.paper.width * MM_PER_POINT; + paperHeight = cachedPaperWrapper.paper.height * MM_PER_POINT; + paperSizeUnit = PrintEventHandler.settings.kPaperSizeMillimeters; + } else { + paperId = this.viewSettings.paperId; + paperWidth = this.viewSettings.paperWidth; + paperHeight = this.viewSettings.paperHeight; + paperSizeUnit = this.viewSettings.sizeUnit; + } + matchedPaper = PrintSettingsViewProxy.getBestPaperMatch( paperWidth, paperHeight, paperSizeUnit ); + } if (!matchedPaper) { // We didn't find a good match. Take the first paper size matchedPaper = Object.values( @@ -461,14 +464,17 @@ var PrintEventHandler = { `Requested paperId: "${paperId}" missing on this printer, using: ${matchedPaper.id} instead` ); delete this._userChangedSettings.paperId; - settingsToUpdate.paperId = matchedPaper.id; } + // Always write paper details back to settings + settingsToUpdate.paperId = matchedPaper.id; // See if we need to change the custom margin values + let paperHeightInInches = matchedPaper.paper.width * INCHES_PER_POINT; + let paperWidthInInches = matchedPaper.paper.width * INCHES_PER_POINT; if ( parseFloat(this.viewSettings.customMargins.marginTop) + parseFloat(this.viewSettings.customMargins.marginBottom) > - paperHeight || + paperHeightInInches || this.viewSettings.customMargins.marginTop < 0 || this.viewSettings.customMargins.marginBottom < 0 ) { @@ -480,7 +486,7 @@ var PrintEventHandler = { if ( parseFloat(this.viewSettings.customMargins.marginRight) + parseFloat(this.viewSettings.customMargins.marginLeft) > - paperWidth || + paperWidthInInches || this.viewSettings.customMargins.marginLeft < 0 || this.viewSettings.customMargins.marginRight < 0 ) { @@ -546,7 +552,11 @@ var PrintEventHandler = { } for (let [setting, value] of Object.entries(changedSettings)) { - if (this.viewSettings[setting] != value) { + // Always write paper changes back to settings as pref-derived values could be bad + if ( + this.viewSettings[setting] != value || + (printerChanged && setting == "paperId") + ) { this.viewSettings[setting] = value; if ( @@ -868,11 +878,7 @@ var PrintSettingsViewProxy = { "Microsoft XPS Document Writer", ]), - getBestPaperMatch(paperId, paperWidth, paperHeight, paperSizeUnit) { - let matchedPaper = paperId && this.availablePaperSizes[paperId]; - if (matchedPaper) { - return matchedPaper; - } + getBestPaperMatch(paperWidth, paperHeight, paperSizeUnit) { let paperSizes = Object.values(this.availablePaperSizes); if (!(paperWidth && paperHeight)) { return null; @@ -890,16 +896,16 @@ var PrintSettingsViewProxy = { // equality to 1pt. const equal = (a, b) => Math.abs(a - b) < 1; const findMatch = (widthPts, heightPts) => - paperSizes.find(paperInfo => { + paperSizes.find(paperWrapper => { // the dimensions on the nsIPaper object are in points let result = - equal(widthPts, paperInfo.paper.width) && - equal(heightPts, paperInfo.paper.height); + equal(widthPts, paperWrapper.paper.width) && + equal(heightPts, paperWrapper.paper.height); return result; }); // Look for a paper with matching dimensions, using the current printer's // paper size unit, then the alternate unit - matchedPaper = + let matchedPaper = findMatch(paperWidth / unitsPerPoint, paperHeight / unitsPerPoint) || findMatch(paperWidth / altUnitsPerPoint, paperHeight / altUnitsPerPoint); @@ -911,24 +917,24 @@ var PrintSettingsViewProxy = { async fetchPaperMargins(paperId) { // resolve any async and computed properties we need on the paper - let paperInfo = this.availablePaperSizes[paperId]; - if (!paperInfo) { + let paperWrapper = this.availablePaperSizes[paperId]; + if (!paperWrapper) { throw new Error("Can't fetchPaperMargins: " + paperId); } - if (paperInfo._resolved) { + if (paperWrapper._resolved) { // We've already resolved and calculated these values return; } - let margins = await paperInfo.paper.unwriteableMargin; + let margins = await paperWrapper.paper.unwriteableMargin; margins.QueryInterface(Ci.nsIPaperMargin); - // margin dimenions are given on the paper in points, setting values need to be in inches - paperInfo.unwriteableMarginTop = margins.top * INCHES_PER_POINT; - paperInfo.unwriteableMarginRight = margins.right * INCHES_PER_POINT; - paperInfo.unwriteableMarginBottom = margins.bottom * INCHES_PER_POINT; - paperInfo.unwriteableMarginLeft = margins.left * INCHES_PER_POINT; + // margin dimensions are given on the paper in points, setting values need to be in inches + paperWrapper.unwriteableMarginTop = margins.top * INCHES_PER_POINT; + paperWrapper.unwriteableMarginRight = margins.right * INCHES_PER_POINT; + paperWrapper.unwriteableMarginBottom = margins.bottom * INCHES_PER_POINT; + paperWrapper.unwriteableMarginLeft = margins.left * INCHES_PER_POINT; // No need to re-resolve static properties - paperInfo._resolved = true; + paperWrapper._resolved = true; }, async resolvePropertiesForPrinter(printerName) { @@ -989,12 +995,12 @@ var PrintSettingsViewProxy = { ); printerInfo.paperList = this.fallbackPaperList; } - let paperSizeUnit = printerInfo.settings.paperSizeUnit; - let unitsPerPoint = - paperSizeUnit == printerInfo.settings.kPaperSizeMillimeters - ? MM_PER_POINT - : INCHES_PER_POINT; - + // don't trust the settings to provide valid paperSizeUnit values + let sizeUnit = + printerInfo.settings.paperSizeUnit == + printerInfo.settings.kPaperSizeMillimeters + ? printerInfo.settings.kPaperSizeMillimeters + : printerInfo.settings.kPaperSizeInches; let papersById = (printerInfo.availablePaperSizes = {}); // Store a convenience reference this.availablePaperSizes = papersById; @@ -1008,13 +1014,8 @@ var PrintSettingsViewProxy = { paper, id: paper.id, name: paper.name, - // Prepare dimension values in the correct unit for the settings. Paper dimensions - // are given in points, so we multiply with the units-per-pt to get dimensions - // in the correct unit for the current printer - width: paper.width * unitsPerPoint, - height: paper.height * unitsPerPoint, - unitsPerPoint, - paperSizeUnit, + // XXXsfoster: Eventually we want to get the unit from the nsIPaper object + sizeUnit, }; } } @@ -1034,12 +1035,12 @@ var PrintSettingsViewProxy = { } case "marginPresets": - let paperSize = this.get(target, "currentPaper"); + let paperWrapper = this.get(target, "currentPaper"); return { - none: PrintEventHandler.getMarginPresets("none", paperSize), - minimum: PrintEventHandler.getMarginPresets("minimum", paperSize), - default: PrintEventHandler.getMarginPresets("default", paperSize), - custom: PrintEventHandler.getMarginPresets("custom", paperSize), + none: PrintEventHandler.getMarginPresets("none", paperWrapper), + minimum: PrintEventHandler.getMarginPresets("minimum", paperWrapper), + default: PrintEventHandler.getMarginPresets("default", paperWrapper), + custom: PrintEventHandler.getMarginPresets("custom", paperWrapper), }; case "marginOptions": { @@ -1156,10 +1157,10 @@ var PrintSettingsViewProxy = { logger.warn("Unexpected margin preset name: ", value); value = "default"; } - let paperSize = this.get(target, "currentPaper"); + let paperWrapper = this.get(target, "currentPaper"); let marginPresets = PrintEventHandler.getMarginPresets( value, - paperSize + paperWrapper ); for (let [settingName, presetValue] of Object.entries(marginPresets)) { target[settingName] = presetValue; @@ -1168,14 +1169,25 @@ var PrintSettingsViewProxy = { case "paperId": { let paperId = value; - let paperSize = this.availablePaperSizes[paperId]; - target.paperWidth = paperSize.width; - target.paperHeight = paperSize.height; - target.unwriteableMarginTop = paperSize.unwriteableMarginTop; - target.unwriteableMarginRight = paperSize.unwriteableMarginRight; - target.unwriteableMarginBottom = paperSize.unwriteableMarginBottom; - target.unwriteableMarginLeft = paperSize.unwriteableMarginLeft; - target.paperId = paperSize.id; + let paperWrapper = this.availablePaperSizes[paperId]; + // Dimensions on the paper object are in pts. + // We convert to the printer's specified unit when updating settings + let unitsPerPoint = + paperWrapper.sizeUnit == target.kPaperSizeMillimeters + ? MM_PER_POINT + : INCHES_PER_POINT; + // paperWidth and paperHeight are calculated values that we always treat as suspect and + // re-calculate whenever the paperId changes + target.paperSizeUnit = paperWrapper.sizeUnit; + target.paperWidth = paperWrapper.paper.width * unitsPerPoint; + target.paperHeight = paperWrapper.paper.height * unitsPerPoint; + // Unwriteable margins were pre-calculated from their async values when the paper size + // was selected. They are always in inches + target.unwriteableMarginTop = paperWrapper.unwriteableMarginTop; + target.unwriteableMarginRight = paperWrapper.unwriteableMarginRight; + target.unwriteableMarginBottom = paperWrapper.unwriteableMarginBottom; + target.unwriteableMarginLeft = paperWrapper.unwriteableMarginLeft; + target.paperId = paperWrapper.paper.id; // pull new margin values for the new paper size this.set(target, "margins", this.get(target, "margins")); break; diff --git a/toolkit/components/printing/tests/browser.ini b/toolkit/components/printing/tests/browser.ini index 36f30d098b39..14446b752ff6 100644 --- a/toolkit/components/printing/tests/browser.ini +++ b/toolkit/components/printing/tests/browser.ini @@ -15,6 +15,7 @@ support-files = file_page_change_print_original_2.html skip-if = os == "mac" +[browser_print_paper_sizes.js] [browser_pdf_printer_settings.js] [browser_print_bcg_id_overflow.js] [browser_print_margins.js] diff --git a/toolkit/components/printing/tests/browser_print_paper_sizes.js b/toolkit/components/printing/tests/browser_print_paper_sizes.js new file mode 100644 index 000000000000..a85492c57511 --- /dev/null +++ b/toolkit/components/printing/tests/browser_print_paper_sizes.js @@ -0,0 +1,143 @@ +"use strict"; + +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +async function selectPaperOptionWithValue(helper, value) { + let paperSelect = helper.get("paper-size-picker"); + info( + "selectPaperOptionWithValue: Current value: " + + paperSelect.value + + ", new value: " + + value + ); + + let targetOptionIndex = Array.from(paperSelect.options).findIndex( + o => o.value == value + ); + ok(targetOptionIndex > -1, "Paper size option exists for value:" + value); + + await helper.openMoreSettings({ scroll: true }); + paperSelect.focus(); + EventUtils.synthesizeKey("VK_SPACE", {}, helper.win); + await helper.awaitAnimationFrame(); + + while (paperSelect.selectedIndex !== targetOptionIndex) { + let directionKey = + targetOptionIndex > paperSelect.selectedIndex + ? "KEY_ArrowDown" + : "KEY_ArrowUp"; + EventUtils.synthesizeKey(directionKey, {}, helper.win); + } + EventUtils.synthesizeKey("KEY_Enter", {}, helper.win); + is(paperSelect.value, value, "New option was selected"); + await helper.awaitAnimationFrame(); +} + +add_task(async function testBadPaperSizeUnitCorrection() { + await PrintHelper.withTestPage(async helper => { + // Set prefs to select a non-default paper size + await SpecialPowers.pushPrefEnv({ + set: [ + ["print.printer_Mozilla_Save_to_PDF.print_paper_id", "na_letter"], + // paperSizeUnit is a bogus value, but the dimensions are correct for inches + ["print.printer_Mozilla_Save_to_PDF.print_paper_size_unit", 99], + ["print.printer_Mozilla_Save_to_PDF.print_paper_height", "11.0"], + ["print.printer_Mozilla_Save_to_PDF.print_paper_width", "8.50"], + ], + }); + await helper.startPrint(); + + let paperSelect = helper.get("paper-size-picker"); + is(paperSelect.value, "na_letter", "The expected paper size is selected"); + is( + helper.viewSettings.paperId, + "na_letter", + "The settings have the expected paperId" + ); + is( + helper.viewSettings.paperSizeUnit, + helper.settings.kPaperSizeInches, + "Check paperSizeUnit" + ); + is(helper.viewSettings.paperWidth.toFixed(1), "8.5", "Check paperWidth"); + is(helper.viewSettings.paperHeight.toFixed(1), "11.0", "Check paperHeight"); + + await selectPaperOptionWithValue(helper, "iso_a3"); + is(paperSelect.value, "iso_a3", "The expected paper size is selected"); + is( + helper.viewSettings.paperId, + "iso_a3", + "The settings have the expected paperId" + ); + is( + helper.viewSettings.paperSizeUnit, + helper.settings.kPaperSizeInches, + "Check paperSizeUnit" + ); + is(helper.viewSettings.paperWidth.toFixed(1), "11.7", "Check paperWidth"); + is(helper.viewSettings.paperHeight.toFixed(1), "16.5", "Check paperHeight"); + + await SpecialPowers.popPrefEnv(); + await helper.closeDialog(); + }); +}); + +add_task(async function testMismatchedPaperSizeUnitCorrection() { + await PrintHelper.withTestPage(async helper => { + // Set prefs to select a non-default paper size + await SpecialPowers.pushPrefEnv({ + set: [ + ["print.printer_Mozilla_Save_to_PDF.print_paper_id", "na_ledger"], + // paperSizeUnit is millimeters, but the dimensions are correct for inches + ["print.printer_Mozilla_Save_to_PDF.print_paper_size_unit", 1], + ["print.printer_Mozilla_Save_to_PDF.print_paper_width", "11.0"], + ["print.printer_Mozilla_Save_to_PDF.print_paper_height", "17.0"], + ], + }); + await helper.startPrint(); + + let paperSelect = helper.get("paper-size-picker"); + is(paperSelect.value, "na_ledger", "The expected paper size is selected"); + + // We expect to honor the paperSizeUnit, and convert paperWidth/Height to that unit + is( + helper.viewSettings.paperId, + "na_ledger", + "The settings have the expected paperId" + ); + is( + helper.viewSettings.paperSizeUnit, + helper.settings.kPaperSizeMillimeters, + "Check paperSizeUnit" + ); + is(helper.viewSettings.paperWidth.toFixed(1), "279.4", "Check paperWidth"); + is( + helper.viewSettings.paperHeight.toFixed(1), + "431.8", + "Check paperHeight" + ); + + await selectPaperOptionWithValue(helper, "iso_a3"); + is(paperSelect.value, "iso_a3", "The expected paper size is selected"); + is( + helper.viewSettings.paperId, + "iso_a3", + "The settings have the expected paperId" + ); + is( + helper.viewSettings.paperSizeUnit, + helper.settings.kPaperSizeMillimeters, + "Check paperSizeUnit" + ); + is(helper.viewSettings.paperWidth.toFixed(1), "297.0", "Check paperWidth"); + is( + helper.viewSettings.paperHeight.toFixed(1), + "420.0", + "Check paperHeight" + ); + + await SpecialPowers.popPrefEnv(); + await helper.closeDialog(); + }); +}); diff --git a/toolkit/components/printing/tests/head.js b/toolkit/components/printing/tests/head.js index f0d77faadc21..b63138349c23 100644 --- a/toolkit/components/printing/tests/head.js +++ b/toolkit/components/printing/tests/head.js @@ -257,8 +257,11 @@ class PrintHelper { EventUtils.sendString(text, this.win); } - async openMoreSettings() { - this.click(this.get("more-settings").firstElementChild); + async openMoreSettings(options) { + let details = this.get("more-settings"); + if (!details.open) { + this.click(details.firstElementChild, options); + } await this.awaitAnimationFrame(); }