[webkit-reviews] review granted: [Bug 78715] [GTK] Allow printing multiple pages per sheet in WebKit2 for printers that don't support it : [Attachment 127184] Patch

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 78715: [GTK] Allow printing multiple pages per sheet in WebKit2 for
printers that don't support it
https://bugs.webkit.org/show_bug.cgi?id=78715

Attachment 127184: Patch
https://bugs.webkit.org/attachment.cgi?id=127184&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=127184&action=review


> Source/WebKit2/ChangeLog:3
> +	   [GTK] Allow printing multiple pages per sheet in WebKit2 for
printers that don't support it

Correct me if I'm wrong, but as far as I understood this code will run
regardless of what printer is selected, isn't that right? Where you say
printers that don't support it, do you mean this is for the case where we set
the number of pages to print per sheet in the dialog, instead of through some
printer-specific thing, or am I misssing something?

> Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp:93
> +	   printOperation->m_manualNumberUp =
gtk_print_job_get_n_up(printOperation->m_printJob.get());
> +	   printOperation->m_manualNumberUpLayout =
gtk_print_job_get_n_up_layout(printOperation->m_printJob.get());
> +

Can we do away with the 'manual' here and in the other patches? It feels out of
place to me; I understand what you mean with it, but I think by having our own
PrintOperation we already convey the idea that we are doing a lot of the
processing ourselves, anyway; and we are not using the manual name in rotation,
for instance, in which gtk unix print operation does; just a nit anyway.

> Source/WebKit2/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp:273
> +    int sheetNumber;
> +    int numberOfSheets;

I am wondering about the usage of size_t and int interchangeably. Any reason
why we shouldn't standardize on size_t? It feels more correct to me.