Skip to content

Commit f0d8729

Browse files
codebyterecodebytere
authored
chore: default to sys printer and prevent bad default crash (electron#22012)
1 parent 40a212e commit f0d8729

File tree

5 files changed

+96
-40
lines changed

5 files changed

+96
-40
lines changed

‎docs/api/web-contents.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1244,7 +1244,7 @@ Returns [`PrinterInfo[]`](structures/printer-info.md)
12441244
* `silent` Boolean (optional) - Don't ask user for print settings. Default is `false`.
12451245
* `printBackground` Boolean (optional) - Prints the background color and image of
12461246
the web page. Default is `false`.
1247-
* `deviceName` String (optional) - Set the printer device name to use. Default is `''`.
1247+
* `deviceName` String (optional) - Set the printer device name to use. Must be the system-defined name and not the 'friendly' name, e.g 'Brother_QL_820NWB' and not 'Brother QL-820NWB'.
12481248
* `color` Boolean (optional) - Set whether the printed web page will be in color or grayscale. Default is `true`.
12491249
* `margins` Object (optional)
12501250
* `marginType` String (optional) - Can be `default`, `none`, `printableArea`, or `custom`. If `custom` is chosen, you will also need to specify `top`, `bottom`, `left`, and `right`.

‎patches/chromium/printing.patch

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -600,24 +600,6 @@ index 71c0c15217b62cd7a6087c6d9ae50481f9041d5f..18d853d7f808aaf816de86e8c5b82317
600600

601601
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
602602
// Set options for print preset from source PDF document.
603-
diff --git a/printing/print_settings_conversion.cc b/printing/print_settings_conversion.cc
604-
index 17c363ff9aa2e2262cacd0c9baea3820334bf67b..5b02461c2e9afe254405ddacd904e4bdbddd0b8b 100644
605-
--- a/printing/print_settings_conversion.cc
606-
+++ b/printing/print_settings_conversion.cc
607-
@@ -184,11 +184,12 @@ bool PrintSettingsFromJobSettings(const base::Value& job_settings,
608-
609-
settings->set_dpi_xy(dpi_horizontal.value(), dpi_vertical.value());
610-
#endif
611-
+ if (!device_name->empty())
612-
+ settings->set_device_name(base::UTF8ToUTF16(*device_name));
613-
614-
settings->set_collate(collate.value());
615-
settings->set_copies(copies.value());
616-
settings->SetOrientation(landscape.value());
617-
- settings->set_device_name(base::UTF8ToUTF16(*device_name));
618-
settings->set_duplex_mode(static_cast<DuplexMode>(duplex_mode.value()));
619-
settings->set_color(static_cast<ColorModel>(color.value()));
620-
settings->set_scale_factor(static_cast<double>(scale_factor.value()) / 100.0);
621603
diff --git a/printing/printing_context.cc b/printing/printing_context.cc
622604
index cd5c27c87df175676504a06b4e1904f6b836dc90..c4f6acf66bc69f1e7db633aa5b3b03a913ffb666 100644
623605
--- a/printing/printing_context.cc

‎shell/browser/api/atom_api_web_contents.cc

Lines changed: 67 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,6 @@
113113
#include "ui/gfx/font_render_params.h"
114114
#endif
115115

116-
#if BUILDFLAG(ENABLE_PRINTING)
117-
#include "chrome/browser/printing/print_view_manager_basic.h"
118-
#include "components/printing/common/print_messages.h"
119-
#endif
120-
121116
#if BUILDFLAG(ENABLE_ELECTRON_EXTENSIONS)
122117
#include "shell/browser/extensions/atom_extension_web_contents_observer.h"
123118
#endif
@@ -313,6 +308,35 @@ void OnCapturePageDone(util::Promise promise, const SkBitmap& bitmap) {
313308
promise.Resolve(gfx::Image::CreateFrom1xBitmap(bitmap));
314309
}
315310

311+
#if BUILDFLAG(ENABLE_PRINTING)
312+
// This will return false if no printer with the provided device_name can be
313+
// found on the network. We need to check this because Chromium does not do
314+
// sanity checking of device_name validity and so will crash on invalid names.
315+
bool IsDeviceNameValid(const base::string16& device_name) {
316+
#if defined(OS_MACOSX)
317+
base::ScopedCFTypeRef<CFStringRef> new_printer_id(
318+
base::SysUTF16ToCFStringRef(device_name));
319+
PMPrinter new_printer = PMPrinterCreateFromPrinterID(new_printer_id.get());
320+
bool printer_exists = new_printer != nullptr;
321+
PMRelease(new_printer);
322+
return printer_exists;
323+
#elif defined(OS_WIN)
324+
printing::ScopedPrinterHandle printer;
325+
return printer.OpenPrinterWithName(device_name.c_str());
326+
#endif
327+
return true;
328+
}
329+
330+
base::string16 GetDefaultPrinterAsync() {
331+
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
332+
base::BlockingType::MAY_BLOCK);
333+
334+
scoped_refptr<printing::PrintBackend> backend =
335+
printing::PrintBackend::CreateInstance(nullptr);
336+
std::string printer_name = backend->GetDefaultPrinterName();
337+
return base::UTF8ToUTF16(printer_name);
338+
}
339+
#endif
316340
} // namespace
317341

318342
WebContents::WebContents(v8::Isolate* isolate,
@@ -1647,6 +1671,30 @@ bool WebContents::IsCurrentlyAudible() {
16471671
}
16481672

16491673
#if BUILDFLAG(ENABLE_PRINTING)
1674+
void WebContents::OnGetDefaultPrinter(
1675+
base::DictionaryValue print_settings,
1676+
printing::CompletionCallback print_callback,
1677+
base::string16 device_name,
1678+
bool silent,
1679+
base::string16 default_printer) {
1680+
base::string16 printer_name =
1681+
device_name.empty() ? default_printer : device_name;
1682+
print_settings.SetStringKey(printing::kSettingDeviceName, printer_name);
1683+
1684+
auto* print_view_manager =
1685+
printing::PrintViewManagerBasic::FromWebContents(web_contents());
1686+
auto* focused_frame = web_contents()->GetFocusedFrame();
1687+
auto* rfh = focused_frame && focused_frame->HasSelection()
1688+
? focused_frame
1689+
: web_contents()->GetMainFrame();
1690+
1691+
print_view_manager->PrintNow(
1692+
rfh,
1693+
std::make_unique<PrintMsg_PrintPages>(rfh->GetRoutingID(), silent,
1694+
std::move(print_settings)),
1695+
std::move(print_callback));
1696+
}
1697+
16501698
void WebContents::Print(mate::Arguments* args) {
16511699
mate::Dictionary options = mate::Dictionary::CreateEmpty(args->isolate());
16521700
base::DictionaryValue settings;
@@ -1710,11 +1758,15 @@ void WebContents::Print(mate::Arguments* args) {
17101758
options.Get("landscape", &landscape);
17111759
settings.SetBoolean(printing::kSettingLandscape, landscape);
17121760

1713-
// We set the default to empty string here and only update
1714-
// if at the Chromium level if it's non-empty
1761+
// We set the default to the system's default printer and only update
1762+
// if at the Chromium level if the user overrides.
1763+
// Printer device name as opened by the OS.
17151764
base::string16 device_name;
17161765
options.Get("deviceName", &device_name);
1717-
settings.SetString(printing::kSettingDeviceName, device_name);
1766+
if (!device_name.empty() && !IsDeviceNameValid(device_name)) {
1767+
args->ThrowError("webContents.print(): Invalid deviceName provided.");
1768+
return;
1769+
}
17181770

17191771
int scale_factor = 100;
17201772
options.Get("scaleFactor", &scale_factor);
@@ -1781,16 +1833,13 @@ void WebContents::Print(mate::Arguments* args) {
17811833
settings.SetInteger(printing::kSettingDpiVertical, dpi);
17821834
}
17831835

1784-
auto* print_view_manager =
1785-
printing::PrintViewManagerBasic::FromWebContents(web_contents());
1786-
auto* focused_frame = web_contents()->GetFocusedFrame();
1787-
auto* rfh = focused_frame && focused_frame->HasSelection()
1788-
? focused_frame
1789-
: web_contents()->GetMainFrame();
1790-
print_view_manager->PrintNow(rfh,
1791-
std::make_unique<PrintMsg_PrintPages>(
1792-
rfh->GetRoutingID(), silent, settings),
1793-
std::move(callback));
1836+
base::PostTaskAndReplyWithResult(
1837+
FROM_HERE,
1838+
{base::ThreadPool(), base::MayBlock(), base::TaskPriority::USER_BLOCKING},
1839+
base::BindOnce(&GetDefaultPrinterAsync),
1840+
base::BindOnce(&WebContents::OnGetDefaultPrinter,
1841+
weak_factory_.GetWeakPtr(), std::move(settings),
1842+
std::move(callback), device_name, silent));
17941843
}
17951844

17961845
std::vector<printing::PrinterBasicInfo> WebContents::GetPrinterList() {

‎shell/browser/api/atom_api_web_contents.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,14 @@
3131
#include "ui/gfx/image/image.h"
3232

3333
#if BUILDFLAG(ENABLE_PRINTING)
34+
#include "chrome/browser/printing/print_view_manager_basic.h"
35+
#include "components/printing/common/print_messages.h"
3436
#include "printing/backend/print_backend.h"
3537
#include "shell/browser/printing/print_preview_message_handler.h"
38+
39+
#if defined(OS_WIN)
40+
#include "printing/backend/win_helper.h"
41+
#endif
3642
#endif
3743

3844
namespace blink {
@@ -181,6 +187,11 @@ class WebContents : public mate::TrackableObject<WebContents>,
181187
v8::Local<v8::Value> GetNativeView() const;
182188

183189
#if BUILDFLAG(ENABLE_PRINTING)
190+
void OnGetDefaultPrinter(base::DictionaryValue print_settings,
191+
printing::CompletionCallback print_callback,
192+
base::string16 device_name,
193+
bool silent,
194+
base::string16 default_printer);
184195
void Print(mate::Arguments* args);
185196
std::vector<printing::PrinterBasicInfo> GetPrinterList();
186197
// Print current page as PDF.

‎spec-main/api-web-contents-spec.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as http from 'http'
66
import { BrowserWindow, ipcMain, webContents, session } from 'electron'
77
import { emittedOnce } from './events-helpers';
88
import { closeAllWindows } from './window-helpers';
9-
import { ifdescribe } from './spec-helpers';
9+
import { ifdescribe, ifit } from './spec-helpers';
1010

1111
const { expect } = chai
1212

@@ -102,21 +102,35 @@ describe('webContents module', () => {
102102
})
103103

104104
ifdescribe(features.isPrintingEnabled())('webContents.print()', () => {
105+
let w: BrowserWindow
106+
107+
beforeEach(() => {
108+
w = new BrowserWindow({ show: false })
109+
})
110+
111+
afterEach(closeAllWindows)
112+
105113
it('throws when invalid settings are passed', () => {
106-
const w = new BrowserWindow({ show: false })
107114
expect(() => {
108115
// @ts-ignore this line is intentionally incorrect
109116
w.webContents.print(true)
110117
}).to.throw('webContents.print(): Invalid print settings specified.')
118+
})
111119

120+
it('throws when an invalid callback is passed', () => {
112121
expect(() => {
113122
// @ts-ignore this line is intentionally incorrect
114123
w.webContents.print({}, true)
115124
}).to.throw('webContents.print(): Invalid optional callback provided.')
116125
})
117126

127+
ifit(process.platform !== 'linux')('throws when an invalid deviceName is passed', () => {
128+
expect(() => {
129+
w.webContents.print({ deviceName: 'i-am-a-nonexistent-printer' }, () => {})
130+
}).to.throw('webContents.print(): Invalid deviceName provided.')
131+
})
132+
118133
it('does not crash', () => {
119-
const w = new BrowserWindow({ show: false })
120134
expect(() => {
121135
w.webContents.print({ silent: true })
122136
}).to.not.throw()

0 commit comments

Comments
 (0)