From 6fe6dffe93e9c398db14d41c2b69dbfb5020d80c Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 24 Sep 2020 08:53:09 -0700 Subject: [PATCH] fix: honor pageRanges when printing (#25597) --- docs/api/web-contents.md | 10 +- docs/api/webview-tag.md | 10 +- patches/chromium/.patches | 1 + ..._properly_honor_printing_page_ranges.patch | 127 ++++++++++++++++++ .../browser/api/electron_api_web_contents.cc | 5 +- 5 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 patches/chromium/fix_properly_honor_printing_page_ranges.patch diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 20a5e9e569404..ecb1e7e296605 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -1296,9 +1296,9 @@ Returns [`PrinterInfo[]`](structures/printer-info.md) * `pagesPerSheet` Number (optional) - The number of pages to print per page sheet. * `collate` Boolean (optional) - Whether the web page should be collated. * `copies` Number (optional) - The number of copies of the web page to print. - * `pageRanges` Record (optional) - The page range to print. - * `from` Number - the start page. - * `to` Number - the end page. + * `pageRanges` Object[] (optional) - The page range to print. On macOS, only one range is honored. + * `from` Number - Index of the first page to print (0-based). + * `to` Number - Index of the last page to print (inclusive) (0-based). * `duplexMode` String (optional) - Set the duplex mode of the printed web page. Can be `simplex`, `shortEdge`, or `longEdge`. * `dpi` Record (optional) * `horizontal` Number (optional) - The horizontal dpi. @@ -1338,8 +1338,8 @@ win.webContents.print(options, (success, errorType) => { default margin, 1 for no margin, and 2 for minimum margin. * `scaleFactor` Number (optional) - The scale factor of the web page. Can range from 0 to 100. * `pageRanges` Record (optional) - The page range to print. - * `from` Number - the first page to print. - * `to` Number - the last page to print (inclusive). + * `from` Number - Index of the first page to print (0-based). + * `to` Number - Index of the last page to print (inclusive) (0-based). * `pageSize` String | Size (optional) - Specify page size of the generated PDF. Can be `A3`, `A4`, `A5`, `Legal`, `Letter`, `Tabloid` or an Object containing `height` and `width` in microns. * `printBackground` Boolean (optional) - Whether to print CSS backgrounds. diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index f60a8276d2781..a4d670e792dd2 100644 --- a/docs/api/webview-tag.md +++ b/docs/api/webview-tag.md @@ -560,9 +560,9 @@ Stops any `findInPage` request for the `webview` with the provided `action`. * `pagesPerSheet` Number (optional) - The number of pages to print per page sheet. * `collate` Boolean (optional) - Whether the web page should be collated. * `copies` Number (optional) - The number of copies of the web page to print. - * `pageRanges` Record (optional) - The page range to print. - * `from` Number - the start page. - * `to` Number - the end page. + * `pageRanges` Object[] (optional) - The page range to print. On macOS, only the first range is honored. + * `from` Number - Index of the first page to print (0-based). + * `to` Number - Index of the last page to print (inclusive) (0-based). * `duplexMode` String (optional) - Set the duplex mode of the printed web page. Can be `simplex`, `shortEdge`, or `longEdge`. * `dpi` Record (optional) * `horizontal` Number (optional) - The horizontal dpi. @@ -588,8 +588,8 @@ Prints `webview`'s web page. Same as `webContents.print([options])`. and `width` in microns. * `scaleFactor` Number (optional) - The scale factor of the web page. Can range from 0 to 100. * `pageRanges` Record (optional) - The page range to print. - * `from` Number - the first page to print. - * `to` Number - the last page to print (inclusive). + * `from` Number - Index of the first page to print (0-based). + * `to` Number - Index of the last page to print (inclusive) (0-based). * `pageSize` String | Size (optional) - Specify page size of the generated PDF. Can be `A3`, `A4`, `A5`, `Legal`, `Letter`, `Tabloid` or an Object containing `height` * `printBackground` Boolean (optional) - Whether to print CSS backgrounds. diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 9b6293a9342ee..35abff306b9dd 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -101,3 +101,4 @@ worker_feat_add_hook_to_notify_script_ready.patch provide_axtextchangevaluestartmarker_for_macos_a11y_value_change.patch allow_focus_to_move_into_an_editable_combobox_s_listbox.patch reconnect_p2p_socket_dispatcher_if_network_service_dies.patch +fix_properly_honor_printing_page_ranges.patch diff --git a/patches/chromium/fix_properly_honor_printing_page_ranges.patch b/patches/chromium/fix_properly_honor_printing_page_ranges.patch new file mode 100644 index 0000000000000..354397b278df8 --- /dev/null +++ b/patches/chromium/fix_properly_honor_printing_page_ranges.patch @@ -0,0 +1,127 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Shelley Vohr +Date: Wed, 23 Sep 2020 10:27:57 -0700 +Subject: fix: properly honor printing page ranges + +The print ranges in Chromium's print job settings were not being properly +plumbed through to PMPrintSettings on mcOS. This fixes that by setting +them should they exist. + +diff --git a/printing/printing_context_mac.h b/printing/printing_context_mac.h +index 06cdc0e1a4a50e29a148c97c964c30a939e800da..5db5cdb61be0beee4506313dcde46c499e011383 100644 +--- a/printing/printing_context_mac.h ++++ b/printing/printing_context_mac.h +@@ -81,6 +81,10 @@ class PRINTING_EXPORT PrintingContextMac : public PrintingContext { + // Returns true if the orientation was set. + bool SetOrientationIsLandscape(bool landscape); + ++ // Set the page range in native print info object. ++ // Returns true if the range was set. ++ bool SetPrintRangeInPrintSettings(const PageRanges& ranges); ++ + // Sets duplex mode in PMPrintSettings. + // Returns true if duplex mode is set. + bool SetDuplexModeInPrintSettings(mojom::DuplexMode mode); +diff --git a/printing/printing_context_mac.mm b/printing/printing_context_mac.mm +index 93438609a3a0eaeec0dbc76efe5c46e03fb40e52..dc64546f7d88db3b48f379b34c7dcc49b692a048 100644 +--- a/printing/printing_context_mac.mm ++++ b/printing/printing_context_mac.mm +@@ -186,7 +186,8 @@ PrintingContext::Result PrintingContextMac::UpdatePrinterSettings( + !SetCopiesInPrintSettings(settings_->copies()) || + !SetCollateInPrintSettings(settings_->collate()) || + !SetDuplexModeInPrintSettings(settings_->duplex_mode()) || +- !SetOutputColor(settings_->color())) { ++ !SetOutputColor(settings_->color()) || ++ !SetPrintRangeInPrintSettings(settings_->ranges())) { + return OnError(); + } + } +@@ -339,6 +340,22 @@ bool PrintingContextMac::SetCopiesInPrintSettings(int copies) { + return PMSetCopies(print_settings, copies, false) == noErr; + } + ++bool PrintingContextMac::SetPrintRangeInPrintSettings(const PageRanges& ranges) { ++ // Default is already NSPrintAllPages - we can safely bail. ++ if (ranges.empty()) ++ return true; ++ ++ auto* print_settings = ++ static_cast([print_info_.get() PMPrintSettings]); ++ ++ // macOS does not allow multiple ranges, so pluck the first. ++ auto range = ranges.front(); ++ bool set_first_page = PMSetFirstPage(print_settings, range.from + 1, false) == noErr; ++ bool set_last_page = PMSetLastPage(print_settings, range.to + 1, false) == noErr; ++ ++ return set_first_page && set_last_page; ++} ++ + bool PrintingContextMac::SetCollateInPrintSettings(bool collate) { + PMPrintSettings print_settings = + static_cast([print_info_.get() PMPrintSettings]); +diff --git a/printing/printing_context_system_dialog_win.cc b/printing/printing_context_system_dialog_win.cc +index a53e4045bdc8a4ea68720c7a4792599097792814..1cb0c34dce72f99d8689e37f2a5d587767d224bf 100644 +--- a/printing/printing_context_system_dialog_win.cc ++++ b/printing/printing_context_system_dialog_win.cc +@@ -52,14 +52,28 @@ void PrintingContextSystemDialogWin::AskUserForSettings( + PRINTPAGERANGE ranges[32]; + dialog_options.nStartPage = START_PAGE_GENERAL; + if (max_pages) { +- // Default initialize to print all the pages. + memset(ranges, 0, sizeof(ranges)); +- ranges[0].nFromPage = 1; +- ranges[0].nToPage = max_pages; +- dialog_options.nPageRanges = 1; +- dialog_options.nMaxPageRanges = base::size(ranges); ++ ++ auto page_ranges = settings_->ranges(); ++ if (!page_ranges.empty()) { ++ for (size_t i = 0; i < page_ranges.size(); i++) { ++ auto range = page_ranges[i]; ++ ranges[i].nFromPage = range.from + 1; ++ ranges[i].nToPage = range.to + 1; ++ } ++ dialog_options.nPageRanges = page_ranges.size(); ++ ++ // Ensure the Pages radio button is selected. ++ dialog_options.Flags |= PD_PAGENUMS; ++ } else { ++ ranges[0].nFromPage = 1; ++ ranges[0].nToPage = max_pages; ++ dialog_options.nPageRanges = 1; ++ } ++ + dialog_options.nMinPage = 1; + dialog_options.nMaxPage = max_pages; ++ dialog_options.nMaxPageRanges = base::size(ranges); + dialog_options.lpPageRanges = ranges; + } else { + // No need to bother, we don't know how many pages are available. +diff --git a/ui/gtk/printing/print_dialog_gtk.cc b/ui/gtk/printing/print_dialog_gtk.cc +index 57900d2854d7c3cb427bc3bd8b1742335ab820b8..166e7eeb9c46f04664fe8986767114b9f4809b71 100644 +--- a/ui/gtk/printing/print_dialog_gtk.cc ++++ b/ui/gtk/printing/print_dialog_gtk.cc +@@ -240,6 +240,23 @@ void PrintDialogGtk::UpdateSettings( + gtk_print_settings_set_n_copies(gtk_settings_, settings->copies()); + gtk_print_settings_set_collate(gtk_settings_, settings->collate()); + ++ auto print_ranges = settings->ranges(); ++ if (!print_ranges.empty()) { ++ // Tell the system that we only intend to print a subset of pages. ++ gtk_print_settings_set_print_pages(gtk_settings_, GTK_PRINT_PAGES_RANGES); ++ ++ GtkPageRange* ranges; ++ ranges = g_new(GtkPageRange, print_ranges.size()); ++ for (size_t i = 0; i < print_ranges.size(); i++) { ++ auto range = print_ranges[i]; ++ ranges[i].start = range.from; ++ ranges[i].end = range.to; ++ } ++ ++ gtk_print_settings_set_page_ranges(gtk_settings_, ranges, 1); ++ g_free(ranges); ++ } ++ + #if defined(USE_CUPS) + // Set advanced settings first so they can be overridden by user applied + // settings. diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc index b6bfd04d0cbb7..d765529bbb4f5 100644 --- a/shell/browser/api/electron_api_web_contents.cc +++ b/shell/browser/api/electron_api_web_contents.cc @@ -2102,8 +2102,9 @@ void WebContents::Print(gin_helper::Arguments* args) { int from, to; if (range.Get("from", &from) && range.Get("to", &to)) { base::Value range(base::Value::Type::DICTIONARY); - range.SetIntKey(printing::kSettingPageRangeFrom, from); - range.SetIntKey(printing::kSettingPageRangeTo, to); + // Chromium uses 1-based page ranges, so increment each by 1. + range.SetIntKey(printing::kSettingPageRangeFrom, from + 1); + range.SetIntKey(printing::kSettingPageRangeTo, to + 1); page_range_list.Append(std::move(range)); } else { continue;