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

Add BrowserWindow.printToPDF API Implementation #1835

Merged
merged 28 commits into from
Jun 17, 2015
Merged

Conversation

hokein
Copy link
Contributor

@hokein hokein commented Jun 1, 2015

This PR implements a API allowing user to save the browser window page as PDF in local disk. The API provides Chromium Preview feature in Electron. Fixes #1157, #805.

Currently, the API definition:

WebContents.printToPDF(options, callback)

  • options Object
    • marginsType Integer - Specify the type of margins to use
      • 0 - default
      • 1 - none
      • 2 - minimum
    • printBackground Boolean - Whether to print CSS background.
    • printSelectionOnly Boolean - Whether to print selection only.
    • landscape Boolean - true for landscape, false for portrait.
  • callback Function - function(error, buffer) {}
    • error Error
    • buffer node:Buffer - Contains the pdf data

This PR also exposes print API to WebContents and WebView.

TODO

  • Implement on Windows
  • Implement on OS X
  • Implement on Linux

@hokein
Copy link
Contributor Author

hokein commented Jun 1, 2015

@zcbenz This PR is ready for review now.

@YurySolovyov
Copy link
Contributor

I think it is better to use node-style callback convention like:

function(err, result) {
    // code
}

Where err is error object with error info or null

P.s. in this case there might be nothing reasonable to pass as result variable, but my point was about node's error-first callbacks convention.

@zcbenz
Copy link
Contributor

zcbenz commented Jun 8, 2015

We can make print and printToPDF methods of WebContents so it would be possible to print webview too.

atom::NativeWindow* window = atom::NativeWindow::FromWebContents(
web_contents());
base::FilePath save_path;
if (!file_dialog::ShowSaveDialog(window, "Save As",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filename should be passed as a parameter of printToPdf, users might want to do the printing silently.

@zcbenz
Copy link
Contributor

zcbenz commented Jun 8, 2015

It seems that the only difference between print and printToPdf is: printToPdf writes the PDF file to disk while print creates a new print job? So I think we can simply implement the printToPdf by hacking the PrintViewManagerBase::OnDidPrintPage function?

@hokein hokein force-pushed the pdf-api branch 2 times, most recently from 3e27d90 to 961a5c3 Compare June 10, 2015 07:37
@hokein
Copy link
Contributor Author

hokein commented Jun 10, 2015

I have refined the API definition. Please take a look again.

It seems that the only difference between print and printToPdf is: printToPdf writes the PDF file to disk while print creates a new print job? So I think we can simply implement the printToPdf by hacking the PrintViewManagerBase::OnDidPrintPage function?

It's not easy to hack the PrintViewManagerBase::OnDidPrintPage function to achieve it. Another difference is the printing setting and its processing procedure. But I have simplifed the code.

@YurySolovyov
Copy link
Contributor

@hokein still not using node-style callbacks?

@hokein
Copy link
Contributor Author

hokein commented Jun 10, 2015

@YuriSolovyov Done, thanks for the hint.

@YurySolovyov
Copy link
Contributor

@hokein great! but docs seem to be out-of-sync though...

@@ -28,6 +28,8 @@
#include "brightray/browser/inspectable_web_contents.h"
#include "brightray/browser/inspectable_web_contents_view.h"
#include "chrome/browser/printing/print_view_manager_basic.h"
#include "chrome/browser/printing/print_preview_message_handler.h"
#include "chrome/browser/ui/browser_dialogs.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These headers are not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Done

@anaisbetts
Copy link
Contributor

This is great work @hokein!


unsigned char* data_begin = static_cast<unsigned char*>(shared_buf->memory());
std::vector<unsigned char> data(data_begin, data_begin + data_size);
return base::RefCountedBytes::TakeVector(&data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The base::RefCountedBytes* has to be put in an scoped_refptr, otherwise the class itself will never be deleted. For this case I think we can return a scoped_ptr<char[]>, since the ownership of data is moved not shared.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the data(PDF content) in RefCountedBytes is manipulated in node::Bufer using through node::Buffer::Use here.

Now I have simplified this code and logic byusing node::Buffer::New, which avoid generating the vector data by ourselves. see commit: hokein@149c00b

@frankhale
Copy link
Contributor

Thank you @hokein for all your effort!!!

@robhoefakker
Copy link

Wow this API works really well! Thanks @hokein :)

v8::Locker locker(isolate);
v8::HandleScope handle_scope(isolate);
if (data) {
v8::Local<v8::Value> buffer = node::Buffer::New(isolate,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling node::Buffer::New will copy all the data and cause hangs, I think that's why we switched to use node::Buffer::Use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pdf file content is returned through base::SharedMemory by renderer process. We cannot pass the memory-map raw data to node::Buffer directly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can copy the data in IO thread and then pass the copied data back to UI thread and call node::Buffer::Use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Done.

@zcbenz
Copy link
Contributor

zcbenz commented Jun 17, 2015

👍

zcbenz added a commit that referenced this pull request Jun 17, 2015
Add `BrowserWindow.printToPDF` API Implementation
@zcbenz zcbenz merged commit a751f4c into electron:master Jun 17, 2015
@Godisemo
Copy link

Would it be possible to specify the size of the resulting pdf? I am making an application for creating charts, and want a way to export the window's content to a pdf. Say if the window has dimensions 200x600 px would it be possible to create a pdf with dimensions 20x60 cm?

@hokein
Copy link
Contributor Author

hokein commented Jul 26, 2015

@Godisemo

Would it be possible to specify the size of the resulting pdf?

No. Current the API only generates PDF with A4 paper size. It's resonable to support more paper size(A3, Letter, Legal, Tabloid), which is also supported in Chromium Print Preview.

But I don't think allowing the API to generate PDF with arbitray paper size is a good idea since PDF is a print-oriented format with a fixed layout, and the printer page size is adopted to the ISO paper size standards, also Chromium Print Preview doesn't support neither.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

print zero margin .. and headers
7 participants