flatpak / xdg-desktop-portal-gtk

Gtk implementation of xdg-desktop-portal
GNU Lesser General Public License v2.1
130 stars 102 forks source link

print: Use Poppler to render pages to print #366

Open ThierryHFR opened 2 years ago

ThierryHFR commented 2 years ago

When you print (event "handle_print") with the new mozilla printing interface (thunderbird and firefox), you systematically open the portal printing box. The printing is supposed to start immediately. The eventemnent "handle-prepare-print" is the only one to open the portal print box.

TingPing commented 2 years ago

Your description of this change isn't very helpful. Could you go into more detail why its necessary to depend on Poppler?

TingPing commented 2 years ago

If I understand this change correctly you are bypassing GTK's print dialog. This is counter to how portals work as they are supposed to have user interactions so users have the final say, in a trusted environment, over an action happening or not.

ThierryHFR commented 2 years ago

Firefox sends a pdf file that contains all the information, paper size, margins... You have to read the pdf to be able to print it page by page.

ThierryHFR commented 2 years ago

I don't agree: the event "handle-prepare-print" opens the dialog box the event "handle-print" starts the printing

This is the behavior of xdg-desktop-porta-kdel

TingPing commented 2 years ago

One thing I am certain of: Poppler is not a safe library to use as you use it here.

The data that applications send is considered untrusted and the xdg-desktop-portal-gtk process is on the host.

If poppler really is something we should be using it needs to be executed in a sandboxed subprocess.

ThierryHFR commented 2 years ago

I have no idea how to sandboxed the use of poppler, I am taking advice

TingPing commented 2 years ago

We use the bubblewrap tool for sandboxing.

Broadly speaking you use the GSubprocess APIs to launch something similiar to bwrap --die-with-parent --unshare-all --ro-bind / / --dev /dev --proc /proc --tmpfs /tmp --file $your_fd /tmp/your_file your-subprocess

This will execute a process that has no network access, can't write to disk, etc. You could transfer return data over stdout or another fd.

I'm not sure the exact format you would return necessarily, its just I don't believe we should be doing any PDF processing in the main process.

ThierryHFR commented 2 years ago

Thanks, I'll take a look.

TingPing commented 2 years ago

the event "handle-prepare-print" opens the dialog box the event "handle-print" starts the printing

Also to answer my own question. You are right.

PreparePrint() returns a token and Print() directly prints if that token is present, otherwise presents a dialog. I'm not sure this PR respects that entirely but this might be handled at the xdg-desktop-portal layer.

ThierryHFR commented 2 years ago

I based myself on the implementation of kde, if there is a token we use it. otherwise we launch directly the printing, we never launch the dialog box. This is a good error handling mechanism. Should it be present? It is the choice of the GtK implementation.

TingPing commented 2 years ago

It sounds like KDE is not following the documentation: https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Print

ThierryHFR commented 2 years ago

It sounds like KDE is not following the documentation: https://flatpak.github.io/xdg-desktop-portal/#gdbus-org.freedesktop.portal.Print

Thanks for the documentation, and you are right. Indeed Kde does not respect the documentation, but Mozilla does the same. the new Mozilla print box builds a PDF according to the user's choices, size defined by the browser, color, pages. The Print action and the file descriptor of the generated PDF are transmitted. Without Portal the mechanism works perfectly, with Portal the absence of the token opens the portal print box.

TingPing commented 2 years ago

Without Portal the mechanism works perfectly, with Portal the absence of the token opens the portal print box.

Yes that is on purpose. Again the point of portals is for users to always be in control, not applications. If an application can print anything it is a bypass.

ThierryHFR commented 2 years ago

This can also be seen as an evolution. A PDF contains all the information necessary to be processed by a printer. Why transmit more information than necessary. For a long time the standard printer was postscript, the pdf is an evolution.

TingPing commented 2 years ago

This is a security boundary it has nothing to do with the format.

Applications in Flatpak cannot print without explicit user approval.

ThierryHFR commented 2 years ago

That I understand! It's the application that generates the token, and that transmits it to Flatpak, it's just an understood format, to discuss. When I am in Firefox or thunderbird (new printing interface) I have the feeling (as a user) that it is me who decides. The new mozilla print interface is the result of increased security! !

ThierryHFR commented 2 years ago

@TingPing I'm thinking of sandboxing the pdftocairo usage which will generate a JPEG file. Is this ok?

TingPing commented 2 years ago

The safest format might be a raw GdkPixbuf, since it doesn't involve parsing anything. Or a similar pixel format in cairo.

ThierryHFR commented 2 years ago

Can we add a binary to the project that does the work? No binary, extracts a PDF page, and returns raw data.

TingPing commented 2 years ago

Yes absolutely you can build the binary here, install it to $libexec/xdg-desktop-portal-gtk/

ThierryHFR commented 2 years ago

I'm almost done! This gives you an idea of things. I'm moving slowly, it's the holiday season. I haven't figured out how to get through this MR while working.

ThierryHFR commented 2 years ago

@TingPing, I just finished! On the "handle-print" event: 1 - If a token exists, use the token 2 - If the file-descriptor contains a pdf file, use the file-descriptor (as a token). I added the pdftoraw binary in the "/usr/libexec" folder. It allows to obtain the following information:

a) --test => is this a pdf? b) --pages => The number of pages c) --width => The width in pixels d) --height => The height in pixels e) -raw => The pixels of a GdkPixbuf

3 - Otherwise open the print dialog box

ThierryHFR commented 2 years ago

It would be nice to integrate a git script analyzing the style as it is done on the gitlab project of sane-backend : style-check.sh Make a check:

git ls-files | xargs ./tools/style-check.sh

Correct:

git ls-files | xargs ./tools/style-check.sh --fix
ThierryHFR commented 2 years ago

I broke something, I'm looking for ...

ThierryHFR commented 2 years ago

I think I'm done!

tinywrkb commented 2 years ago

Can this be behind a make configure option?
This adds a libpoppler (&-glib) requirement to a minimal base system.
CUPS is removing printing filters support, so no libpoppler there anymore.

ThierryHFR commented 2 years ago

Can this be behind a make configure option?

Fixed by ee2cc51

tinywrkb commented 2 years ago

Fixed by ee2cc51

Thanks!