Bug 165298 – [GTK] Minibrowser: Add printing support

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Tomas Popela]

It would be nice to have a possibility of showing a print dialog when pressing a Ctrl + P shortcut.

[Tomas Popela]

Created attachment 295936 [details]
Patch

[Michael Catanzaro]

Comment on attachment 295936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295936&action=review

Maybe we should add a toolbar button for it...?

> Tools/MiniBrowser/gtk/BrowserWindow.c:533
> +    if (webkit_print_operation_run_dialog(printOperation, GTK_WINDOW(window)) != WEBKIT_PRINT_OPERATION_RESPONSE_CANCEL)
> +        g_signal_connect(printOperation, "failed", G_CALLBACK(webKitPrintOperationFailedCallback), window);

Does this error path actually work? Did you test it? The "failed" signal is emitted after the dialog is closed, so you don't need to connect in advance?

[Michael Catanzaro]

I think if that actually works, then we need better documentation for WebKitPrintOperation, because that's not what I'd expect.

[Carlos Garcia Campos]

Comment on attachment 295936 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295936&action=review

I've always used javascript:print() in the url bar to test printing with MiniBrowser, but I'm ok with adding CTRL + P too.

> Tools/MiniBrowser/gtk/BrowserWindow.c:519
> +static void webKitPrintOperationFailedCallback(WebKitPrintOperation *printOperation, GError *error, BrowserWindow *window)

BrowserWindow is unused, you could just pas NULL as user_data

> Tools/MiniBrowser/gtk/BrowserWindow.c:524
> +        g_warning("Print failed without error set");

Is this really possible? I think this should be a bug, failed callback should always provide a valid GError.

[Michael Catanzaro]

Ah yeah, if there's no GError set then you should just crash by dereferencing it. :) As a rule, functions that can fail must always set the GError when returning FALSE; that convention should apply to our signal here too.

[Tomas Popela]

(In reply to comment #4)
> Is this really possible? I think this should be a bug, failed callback
> should always provide a valid GError.

I added it there "just to be sure". I will remove it.

[Tomas Popela]

(In reply to comment #3)
> I think if that actually works, then we need better documentation for
> WebKitPrintOperation, because that's not what I'd expect.

No, it does not work. I actually don't know how to make the print to fail.

[Michael Catanzaro]

(In reply to comment #7)
> (In reply to comment #3)
> > I think if that actually works, then we need better documentation for
> > WebKitPrintOperation, because that's not what I'd expect.
> 
> No, it does not work. I actually don't know how to make the print to fail.

To test it, modify GTK+ so that the operation does nothing but fail.

[Tomas Popela]

Created attachment 298004 [details]
Patch

[Michael Catanzaro]

Comment on attachment 298004 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=298004&action=review

> Tools/MiniBrowser/gtk/BrowserWindow.c:521
> +    g_warning("Print failed with: '%s'", error->message);

Remove the word "with"

[Tomas Popela]

Committed r210324: <http://trac.webkit.org/changeset/210324>