feat: add webFrameMain.printToPDF() - #52439
Conversation
MarshallOfSound
added
target/41-x-y
jkleinsc
left a comment
jkleinsc
left a comment
There was a problem hiding this comment.
Conceptually API LGTM, but we should use a structure for the print options since it is reused.
| }) | ||
| .catch(() => {}); | ||
| return next; | ||
| return printToPDF(this, options); |
There was a problem hiding this comment.
Could we simplify PrintToPDFTarget by always directing to the frame-level API?
| return printToPDF(this, options); | |
| return this.mainFrame.printToPDF(options); |
There was a problem hiding this comment.
It's not quite equivalent. WebContents::PrintToPDF doesn't print the primary main frame, it prints GetRenderFrameHostToUse(web_contents()) (printing_utils.cc:111) which redirects to the PDF plugin child frame when the contents is showing the PDF viewer (OOPIF or full-page plugin) and to the focused frame if it has a selection. Routing through this.mainFrame would drop that, so printToPDF() on a loaded .pdf would print the extension embedder page instead of the document. And I don't think we want that redirect inside WebFrameMain::PrintToPDF since a frame-scoped API should print the frame you asked for.
That's why both entry points funnel into printToPDF(target, options) on the JS side and PrintFrameToPDF(rfh, settings) in C++. PrintToPDFTarget is just the shim so the option translation + queueing isn't duplicated.
There was a problem hiding this comment.
Makes sense and I do like that both are able to reuse the JS printToPDF method.
If my understanding is correct, an <iframe> embedding a PDF would not return the actual PDF's byte data in the result of frame.printToPDF. I'm a little confused about what the behavior might be which may imply it'd be worth adding a test to confirm expectations. An explicit call out in the docs might also be worth clarifying.
Unlike
webContents.printToPDF, this method prints only the contents of the frame it is called on.
If this already implies the expected behavior to what I mentioned above, then maybe that's fine too.
There was a problem hiding this comment.
printToPDF on an iframe with a PDF would print the PDF but in a scroll container. You'd need to do frameWithPDF.frames[0] basically to get the actual PDF iframe
|
I think the macos-arm64 CI failure here is unrelated to this PR, I think it's the same bug being addressed by #52448. If we land that first and then rebase here, it should go green. |
|
rebasing to |
Allows printing an individual frame (e.g. an <iframe>) to PDF from the main process, rather than only the full page via webContents.printToPDF. The C++ PDF path already operates on a RenderFrameHost, so the core is extracted to shell/browser/printing/print_to_pdf.cc and shared by both entry points. The JS option translation and job queueing move to a shared lib/browser/print-to-pdf.ts; jobs are serialized per frame tree (keyed by the top frame's frameTreeNodeId, which is stable across navigations) because concurrent print jobs within one frame tree conflict in the renderer, while frames in different webContents still print concurrently. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The options table was duplicated across webContents, <webview>, and now webFrameMain. Pull it into a PrintToPDFOptions structure and give its margins their own PrintToPDFMargins structure so the generated typings no longer merge them with webContents.print()'s pixel-based Margins. No-Verification-Needed: docs-only
68e8c8b to
1379418
Compare
jkleinsc
added
the
api-review/skip-delay ⏰
🤔 |
4 participants
Description of Change
Adds
frame.printToPDF(options)toWebFrameMain, allowing the main process to print individual frames (e.g. an<iframe>) to PDF. This reuses the existingwebContents.printToPDFimplementation, which already operates on a single frame internally — the C++ core and JS option handling are extracted into shared helpers used by both. Print jobs within a frame tree are serialized (the same guaranteewebContents.printToPDFalready had, as concurrent jobs in one tree conflict in the renderer), while frames in different webContents print concurrently.resolves #33625
resolves #22796
Checklist
npm testpassesRelease Notes
Notes: Added
webFrameMain.printToPDF()to allow printing individual frames to PDF from the main process.