printToPDF isn't working

[bbondy]

Its callback is never called, I think the renderer needs to handle the message.

[willy-b]

are you replacing the use of printToPDF in #4769 or are you fixing printToPDF itself?

I'm asking because normally the webview.printToPDF method invokes the callback with two args (error, data)..
if the callback is never called, how do you get the PDF data to "handle the message" in the renderer?

[bsclifton]

@bbondy It may be due to this:
brave/muon@46407b6#diff-a2a29fbd6a5f9747649d3c2c444dae3f

Let's try adding back in the two files that were removed

edit: this would only affect Windows- nevermind

[bbondy]

I'm going to try to get webcontents.printToPDF to work

[willy-b]

woah, 63 chromium level files changed in a single patch file for this! (https://github.com/brave/muon/blob/9ea426b623d79450272736789d4e651669ad5230/patches/master_patch.patch)

is fixing this Electron-level API regression so complicated because Brave's Electron fork (Muon) is using a more recent version of Chromium (54) than upstream Electron?

[bbondy]

It isn't as impressive as it sounds because this is just a change of an existing patch file. A diff of a patch looks pretty deceiving. There were a bunch of changes but definitely no 63 files worth :P

[bbondy]

confirmed my patch works on macOS and linux but not Windows, will get back to this soonish.

[willy-b]

sorry to pester with a question, but when does this come into brave/browser-laptop?

trying to figure out if I should revert the PDF statement before next release or not

[bbondy]

It will be in Preview7 or anything after muon v2.0.8

[willy-b]

hey I can confirm this works on Linux in Preview7, but @srirambv shared a screencap of it still being broken on Windows (#6385 (comment))

[willy-b]

@bsclifton so this is only broken on Windows now.

Do you think we need to add those two files back in?

[bsclifton]

@willy-b no- I don't think it will help. @bbondy is investigating it though 😄

[willy-b]

sweet! one last question -- do you know what tag/release for brave/browser-laptop will have brave/muon@465539e in it?

[bbondy]

Preview8 and 0.13 release will have it.

[luixxiul]

@willy-b @bbondy would you please specify QA steps or the commit which has them in its commit message? If QA is not required, please label so.

Thanks!

[bbondy]

It passes if this passes:
#4769

So I'll mark this as no qa needed.

[willy-b]

Happy to help test this on Linux and Windows -- Do you know when Preview8 is coming?

[willy-b]

got browser-laptop-bootstrap integrated build with Muon working and can confirm #4769 is working on Linux with @bbondy's latest changes.