Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: honor pageRanges when printing #25064

Merged
merged 8 commits into from Aug 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions docs/api/web-contents.md
Expand Up @@ -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<string, number> (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<string, number> (optional)
* `horizontal` Number (optional) - The horizontal dpi.
Expand All @@ -1324,10 +1324,10 @@ Example usage:
const options = {
silent: true,
deviceName: 'My-Printer',
pageRanges: {
pageRanges: [{
from: 0,
to: 1
}
}]
}
win.webContents.print(options, (success, errorType) => {
if (!success) console.log(errorType)
Expand All @@ -1345,8 +1345,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<string, number> (optional) - The page range to print.
* `from` Number - zero-based index of the first page to print.
* `to` Number - zero-based index of 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.
Expand Down
12 changes: 6 additions & 6 deletions docs/api/webview-tag.md
Expand Up @@ -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<string, number> (optional) - The page range to print.
* `from` Number - zero-based index of the first page to print.
* `to` Number - zero-based index of the last page to print (inclusive).
* `pageRanges` Object[] (optional) - The page range to print.
* `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<string, number> (optional)
* `horizontal` Number (optional) - The horizontal dpi.
Expand All @@ -587,9 +587,9 @@ Prints `webview`'s web page. Same as `webContents.print([options])`.
default margin, 1 for no margin, and 2 for minimum margin.
and `width` in microns.
* `scaleFactor` Number (optional) - The scale factor of the web page. Can range from 0 to 100.
* `pageRanges` Record<string, number> (optional) - The page range to print.
* `from` Number - the first page to print.
* `to` Number - the last page to print (inclusive).
* `pageRanges` Record<string, number> (optional) - The page range to print. On macOS, only the first range is honored.
codebytere marked this conversation as resolved.
Show resolved Hide resolved
* `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.
Expand Down
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -100,3 +100,4 @@ remove_some_deps_that_do_not_work_on_arm64.patch
fix_check_issecureeventinputenabled_in_constructor_before_setting.patch
skip_atk_toolchain_check.patch
worker_feat_add_hook_to_notify_script_ready.patch
fix_properly_honor_printing_page_ranges.patch
129 changes: 129 additions & 0 deletions patches/chromium/fix_properly_honor_printing_page_ranges.patch
@@ -0,0 +1,129 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Thu, 20 Aug 2020 10:55:48 -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.

This will be upstreamed.

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 56bcfbe625537c7b1bc2a999c261836bb15896ba..f916e78de99b87e0dae4f5f87329886b658ccd93 100644
--- a/printing/printing_context_mac.mm
+++ b/printing/printing_context_mac.mm
@@ -188,7 +188,8 @@ PMPaper MatchPaper(CFArrayRef paper_list,
!SetCopiesInPrintSettings(settings_->copies()) ||
!SetCollateInPrintSettings(settings_->collate()) ||
!SetDuplexModeInPrintSettings(settings_->duplex_mode()) ||
- !SetOutputColor(settings_->color())) {
+ !SetOutputColor(settings_->color()) ||
+ !SetPrintRangeInPrintSettings(settings_->ranges()) ) {
return OnError();
}
}
@@ -341,6 +342,22 @@ PMPaper MatchPaper(CFArrayRef paper_list,
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<PMPrintSettings>([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<PMPrintSettings>([print_info_.get() PMPrintSettings]);
diff --git a/printing/printing_context_system_dialog_win.cc b/printing/printing_context_system_dialog_win.cc
index d3c8677f30d72efc49b28f293260c74c7b8d8b4e..f6e66aaa58ab1881d64dcbb320ae8b5ac7631b28 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 8a78a609295a1d90208bdc7e73d7a850c319f361..2f5bc01bedf84126d8fb1597a0b813a2b7230279 100644
--- a/ui/gtk/printing/print_dialog_gtk.cc
+++ b/ui/gtk/printing/print_dialog_gtk.cc
@@ -239,6 +239,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
5 changes: 3 additions & 2 deletions shell/browser/api/electron_api_web_contents.cc
Expand Up @@ -2068,8 +2068,9 @@ void WebContents::Print(gin::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;
Expand Down