From d5b46d07f8bbe497808aacd5ad1791580fa1749e Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Sat, 14 Sep 2019 00:11:03 +0000 Subject: [PATCH] Print Preview: Do not show N-up setting when it cannot be honored. The internal printing code can only handle N-up for content from Blink or the PDF plugin. For any other plugin, which is effectively just Flash, the N-up setting does not work. So do not show the setting in the Print Preview UI in this case. To do this, take advantage of an earlier CL that exposes whether the content being printed is PDF or not, and plumb that setting into the Print Preview UI and show the pages-per-sheet control based on that. Change-Id: Ic57f50a451aba9ba6918b59be572a371a1b4beca Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790256 Commit-Queue: Lei Zhang Reviewed-by: Rebekah Potter Cr-Commit-Position: refs/heads/master@{#696598} --- chrome/browser/printing/print_test_utils.cc | 1 + .../print_preview/data/document_info.js | 7 +++++- .../resources/print_preview/data/model.js | 7 ++++-- .../resources/print_preview/native_layer.js | 1 + .../browser/resources/print_preview/ui/app.js | 4 ++-- .../print_preview/print_preview_handler.cc | 2 ++ .../print_preview_handler_unittest.cc | 1 + .../model_settings_availability_test.js | 22 +++++++++++++++++++ .../print_render_frame_helper_browsertest.cc | 1 + printing/print_job_constants.cc | 5 +++++ printing/print_job_constants.h | 1 + 11 files changed, 47 insertions(+), 5 deletions(-) diff --git a/chrome/browser/printing/print_test_utils.cc b/chrome/browser/printing/print_test_utils.cc index bd6d793de345e3..0047d3efe01664 100644 --- a/chrome/browser/printing/print_test_utils.cc +++ b/chrome/browser/printing/print_test_utils.cc @@ -40,6 +40,7 @@ base::Value GetPrintTicket(PrinterType type, bool cloud) { ticket.SetKey(kSettingShouldPrintBackgrounds, base::Value(false)); ticket.SetKey(kSettingShouldPrintSelectionOnly, base::Value(false)); ticket.SetKey(kSettingPreviewModifiable, base::Value(true)); + ticket.SetKey(kSettingPreviewIsPDF, base::Value(false)); ticket.SetKey(kSettingPrintToPDF, base::Value(!cloud && type == kPdfPrinter)); ticket.SetKey(kSettingCloudPrintDialog, base::Value(cloud)); ticket.SetKey(kSettingPrintWithPrivet, base::Value(is_privet_printer)); diff --git a/chrome/browser/resources/print_preview/data/document_info.js b/chrome/browser/resources/print_preview/data/document_info.js index 67ce1c952af04b..8663ca2be820ee 100644 --- a/chrome/browser/resources/print_preview/data/document_info.js +++ b/chrome/browser/resources/print_preview/data/document_info.js @@ -9,6 +9,7 @@ cr.exportPath('print_preview'); * hasCssMediaStyles: boolean, * hasSelection: boolean, * isModifiable: boolean, + * isPdf: boolean, * isScalingDisabled: boolean, * fitToPageScaling: number, * pageCount: number, @@ -48,6 +49,7 @@ Polymer({ hasCssMediaStyles: false, hasSelection: false, isModifiable: true, + isPdf: false, isScalingDisabled: false, fitToPageScaling: 100, pageCount: 0, @@ -113,13 +115,16 @@ Polymer({ /** * Initializes the state of the data model. * @param {boolean} isModifiable Whether the document is modifiable. + * @param {boolean} isPdf Whether the document is PDF. * @param {string} title Title of the document. * @param {boolean} hasSelection Whether the document has user-selected * content. */ - init: function(isModifiable, title, hasSelection) { + init: function(isModifiable, isPdf, title, hasSelection) { this.isInitialized_ = true; this.set('documentSettings.isModifiable', isModifiable); + // TODO(crbug.com/702995): Remove once Flash is deprecated. + this.set('documentSettings.isPdf', isPdf); this.set('documentSettings.title', title); this.set('documentSettings.hasSelection', hasSelection); }, diff --git a/chrome/browser/resources/print_preview/data/model.js b/chrome/browser/resources/print_preview/data/model.js index 1673d8ecb14dad..114cc16fc078ed 100644 --- a/chrome/browser/resources/print_preview/data/model.js +++ b/chrome/browser/resources/print_preview/data/model.js @@ -484,8 +484,8 @@ Polymer({ observers: [ 'updateSettingsFromDestination_(destination.capabilities)', 'updateSettingsAvailabilityFromDocumentSettings_(' + - 'documentSettings.isModifiable, documentSettings.hasCssMediaStyles,' + - 'documentSettings.hasSelection)', + 'documentSettings.isModifiable, documentSettings.isPdf,' + + 'documentSettings.hasCssMediaStyles, documentSettings.hasSelection)', 'updateHeaderFooterAvailable_(' + 'margins, settings.margins.value, ' + 'settings.customMargins.value, settings.mediaSize.value)', @@ -726,6 +726,9 @@ Polymer({ return; } + this.setSettingPath_( + 'pagesPerSheet.available', + this.documentSettings.isModifiable || this.documentSettings.isPdf); this.setSettingPath_( 'margins.available', this.documentSettings.isModifiable); this.setSettingPath_( diff --git a/chrome/browser/resources/print_preview/native_layer.js b/chrome/browser/resources/print_preview/native_layer.js index ddf522abeb6bc9..0aae1c1853491c 100644 --- a/chrome/browser/resources/print_preview/native_layer.js +++ b/chrome/browser/resources/print_preview/native_layer.js @@ -37,6 +37,7 @@ cr.define('print_preview', function() { * decimalDelimiter: string, * unitType: !print_preview.MeasurementSystemUnitType, * previewModifiable: boolean, + * previewIsPdf: boolean, * documentTitle: string, * documentHasSelection: boolean, * shouldPrintSelectionOnly: boolean, diff --git a/chrome/browser/resources/print_preview/ui/app.js b/chrome/browser/resources/print_preview/ui/app.js index 8f30d4e635811a..ccd7ca5e5e7957 100644 --- a/chrome/browser/resources/print_preview/ui/app.js +++ b/chrome/browser/resources/print_preview/ui/app.js @@ -266,8 +266,8 @@ Polymer({ settings.uiLocale); } this.$.documentInfo.init( - settings.previewModifiable, settings.documentTitle, - settings.documentHasSelection); + settings.previewModifiable, settings.previewIsPdf, + settings.documentTitle, settings.documentHasSelection); this.$.model.setStickySettings(settings.serializedAppStateStr); this.$.model.setPolicySettings( settings.headerFooter, settings.isHeaderFooterManaged); diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc index ac0b3d7e283699..b734fe4d7a90ad 100644 --- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc +++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc @@ -982,6 +982,8 @@ void PrintPreviewHandler::SendInitialSettings( print_preview_ui()->initiator_title()); initial_settings.SetBoolKey(kSettingPreviewModifiable, print_preview_ui()->source_is_modifiable()); + initial_settings.SetBoolKey(kSettingPreviewIsPDF, + print_preview_ui()->source_is_pdf()); initial_settings.SetStringKey(kSettingPrinterName, default_printer); initial_settings.SetBoolKey(kDocumentHasSelection, print_preview_ui()->source_has_selection()); diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc index 67b354e803677d..808eaeb632491e 100644 --- a/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc +++ b/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc @@ -98,6 +98,7 @@ base::Value GetPrintPreviewTicket() { print_ticket.SetKey(kIsFirstRequest, base::Value(true)); print_ticket.SetKey(kPreviewRequestID, base::Value(0)); print_ticket.SetKey(kSettingPreviewModifiable, base::Value(false)); + print_ticket.SetKey(kSettingPreviewIsPDF, base::Value(true)); print_ticket.RemoveKey(kSettingPageWidth); print_ticket.RemoveKey(kSettingPageHeight); print_ticket.RemoveKey(kSettingShowSystemDialog); diff --git a/chrome/test/data/webui/print_preview/model_settings_availability_test.js b/chrome/test/data/webui/print_preview/model_settings_availability_test.js index eec907a23e636f..3a935bc4b724f8 100644 --- a/chrome/test/data/webui/print_preview/model_settings_availability_test.js +++ b/chrome/test/data/webui/print_preview/model_settings_availability_test.js @@ -16,6 +16,7 @@ cr.define('model_settings_availability_test', function() { hasCssMediaStyles: false, hasSelection: false, isModifiable: true, + isPdf: false, isScalingDisabled: false, fitToPageScaling: 100, pageCount: 3, @@ -509,6 +510,27 @@ cr.define('model_settings_availability_test', function() { assertFalse(model.settings.selectionOnly.setFromUi); }); + test('pages per sheet', function() { + // Pages per sheet is available everywhere except for Flash content. + // With the default settings for Blink content, it is available. + model.set('documentSettings.isModifiable', true); + model.set('documentSettings.isPdf', false); + assertTrue(model.settings.pagesPerSheet.available); + + // This state should never occur, but if it does, |isModifiable| takes + // precedence and this is still interpreted as Blink content. + model.set('documentSettings.isPdf', true); + assertTrue(model.settings.pagesPerSheet.available); + + // Still available for PDF content. + model.set('documentSettings.isModifiable', false); + assertTrue(model.settings.pagesPerSheet.available); + + // Not available for Flash content. + model.set('documentSettings.isPdf', false); + assertFalse(model.settings.pagesPerSheet.available); + }); + if (cr.isChromeOS) { test('pin', function() { // Make device unmanaged. diff --git a/components/printing/test/print_render_frame_helper_browsertest.cc b/components/printing/test/print_render_frame_helper_browsertest.cc index 278fa9ca383101..14fccb31b0db0c 100644 --- a/components/printing/test/print_render_frame_helper_browsertest.cc +++ b/components/printing/test/print_render_frame_helper_browsertest.cc @@ -127,6 +127,7 @@ void CreatePrintSettingsDictionary(base::DictionaryValue* dict) { dict->SetBoolean(kIsFirstRequest, true); dict->SetInteger(kSettingMarginsType, DEFAULT_MARGINS); dict->SetBoolean(kSettingPreviewModifiable, true); + dict->SetBoolean(kSettingPreviewIsPDF, false); dict->SetBoolean(kSettingHeaderFooterEnabled, false); dict->SetBoolean(kSettingShouldPrintBackgrounds, false); dict->SetBoolean(kSettingShouldPrintSelectionOnly, false); diff --git a/printing/print_job_constants.cc b/printing/print_job_constants.cc index 5a94092f7f2ed8..1c15dfabca8ccc 100644 --- a/printing/print_job_constants.cc +++ b/printing/print_job_constants.cc @@ -145,6 +145,11 @@ const char kSettingPinValue[] = "pinValue"; // Policies affecting printing destination. const char kSettingPolicies[] = "policies"; +// Whether the source page content is PDF or not. +const char kSettingPreviewIsPDF[] = "previewIsPDF"; + +// Whether the source page content is modifiable. True for web content. +// i.e. Anything from Blink. False for everything else. e.g. PDF/Flash. const char kSettingPreviewModifiable[] = "previewModifiable"; // Keys that specifies the printable area details. diff --git a/printing/print_job_constants.h b/printing/print_job_constants.h index 58048c9edad59a..c894de81149145 100644 --- a/printing/print_job_constants.h +++ b/printing/print_job_constants.h @@ -55,6 +55,7 @@ PRINTING_EXPORT extern const char kSettingPageHeight[]; PRINTING_EXPORT extern const char kSettingPagesPerSheet[]; PRINTING_EXPORT extern const char kSettingPinValue[]; PRINTING_EXPORT extern const char kSettingPolicies[]; +PRINTING_EXPORT extern const char kSettingPreviewIsPDF[]; PRINTING_EXPORT extern const char kSettingPreviewModifiable[]; PRINTING_EXPORT extern const char kSettingPrintToGoogleDrive[]; PRINTING_EXPORT extern const char kSettingPrintToPDF[];