Bug 78715 - [GTK] Allow printing multiple pages per sheet in WebKit2 for printers that don't support it
Summary: [GTK] Allow printing multiple pages per sheet in WebKit2 for printers that do...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 75544 78793
  Show dependency treegraph
 
Reported: 2012-02-15 07:13 PST by Carlos Garcia Campos
Modified: 2012-02-17 08:43 PST (History)
1 user (show)

See Also:


Attachments
Patch (14.99 KB, patch)
2012-02-15 07:25 PST, Carlos Garcia Campos
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2012-02-15 07:13:16 PST
Multiple pages per sheet is not supported when printing to a file, for example. We need to provide custom code to implement it.
Comment 1 Carlos Garcia Campos 2012-02-15 07:25:05 PST
Created attachment 127184 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-02-16 16:24:16 PST
Comment on attachment 127184 [details]
Patch

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.
Comment 3 Carlos Garcia Campos 2012-02-16 23:18:55 PST
(In reply to comment #2)
> (From update of attachment 127184 [details])
> 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?

No, because the printer resets the settings of the features it supports. For example, when printing with cups in a printer that supports multiple pages per sheet, the cups gtk print backend sets n_up to 1 in the print job. 

> > 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.

Yes, because rotation is always enabled in the print dialog. Manual capabilities correspond to GtkPrintCapabilities, you need to set them explicitly to enable them in the print dialog. A following patch will enable the supported ones in WebKitPrintOperation. So in this case the 'manual' prefix means the feature correspond to a GtkPrintCapabilities enabled in the print dialog with gtk_print_unix_dialog_set_manual_capabilities(). I don't mind to remove it 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.

You are right, and this is causing a compile warning indeed.
Comment 4 Gustavo Noronha (kov) 2012-02-17 06:35:02 PST
(In reply to comment #3)
> No, because the printer resets the settings of the features it supports. For example, when printing with cups in a printer that supports multiple pages per sheet, the cups gtk print backend sets n_up to 1 in the print job. 

OK, that makes sense.
 
> > 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.
> 
> Yes, because rotation is always enabled in the print dialog. Manual capabilities correspond to GtkPrintCapabilities, you need to set them explicitly to enable them in the print dialog. A following patch will enable the supported ones in WebKitPrintOperation. So in this case the 'manual' prefix means the feature correspond to a GtkPrintCapabilities enabled in the print dialog with gtk_print_unix_dialog_set_manual_capabilities(). I don't mind to remove it anyway.

That means this will be dead code meanwhile?
Comment 5 Carlos Garcia Campos 2012-02-17 06:41:17 PST
(In reply to comment #4)
> (In reply to comment #3)
> > No, because the printer resets the settings of the features it supports. For example, when printing with cups in a printer that supports multiple pages per sheet, the cups gtk print backend sets n_up to 1 in the print job. 
> 
> OK, that makes sense.
> 
> > > 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.
> > 
> > Yes, because rotation is always enabled in the print dialog. Manual capabilities correspond to GtkPrintCapabilities, you need to set them explicitly to enable them in the print dialog. A following patch will enable the supported ones in WebKitPrintOperation. So in this case the 'manual' prefix means the feature correspond to a GtkPrintCapabilities enabled in the print dialog with gtk_print_unix_dialog_set_manual_capabilities(). I don't mind to remove it anyway.
> 
> That means this will be dead code meanwhile?

I can just add the new capability on every patch while landing. Patches don't include it, because API hadn't landed when I submitted the patches
Comment 6 Carlos Garcia Campos 2012-02-17 08:43:17 PST
Committed r108080: <http://trac.webkit.org/changeset/108080>