GNOME Bugzilla – Bug 694091
Implement get best web app icon in WebKit2
Last modified: 2013-03-06 10:39:54 UTC
Currently it's not implemented the support to select the icon for the web apps in WK2.
Created attachment 236601 [details] [review] Patch
Created attachment 236627 [details] [review] Updated patch fixing identation
Created attachment 236640 [details] [review] Updated patch fixing issue returning NULL Returning NULL values for uri or color were causing the following messages: GLib-CRITICAL **: g_variant_new_string: assertion `string != NULL' failed This has been fixed returning an empty string instead of NULL.
Created attachment 236669 [details] [review] Use async call to d-bus method
Created attachment 236722 [details] [review] Updated patch fixing warning This new version fixes a warning in the D-Bus call: warning: passing argument 7 of 'g_dbus_proxy_call' from incompatible pointer type [enabled by default]
Created attachment 237363 [details] [review] Patch 1/2
Created attachment 237365 [details] [review] Patch 2/2 Again like for bug #694144 the bug has been split in 2 patches (one for moving the DOM bindings code to ephy-web-dom-utils and the other to implement the functionality in WK2).
Created attachment 237379 [details] [review] Patch 2/2 After bug #694519 web extension proxy can be NULL, so included a check before using it .
Created attachment 237457 [details] [review] Patch 2/2
Review of attachment 237363 [details] [review]: Looks good, also needs to use the right name for the method. ::: src/window-commands.c @@ +663,3 @@ + document = webkit_web_view_get_dom_document (WEBKIT_WEB_VIEW (data->view)); + res = ephy_web_dom_get_best_icon (document, + base_uri, Indentation is off here.
I'm preparing the patches to be applied after bug #694144, as I guess previous bug will be accepted and committed before this one. Moreover, I've rebased everything against master in order to do not lose the change in commit 59b08b8c4f87afcc6b0cc65bc88a0c25da329762. So the new patches that I'm going to apply, should be applied in master + 2 patches for bug #694144.
Created attachment 237660 [details] [review] Patch 1/2 fixing naming issue and indentation
Created attachment 237661 [details] [review] Patch 2/2 fixing naming issue
Review of attachment 237660 [details] [review]: Looks good.
Review of attachment 237661 [details] [review]: ::: embed/web-extension/ephy-web-extension.c @@ +124,3 @@ + + g_dbus_method_invocation_return_value (invocation, + g_variant_new ("(bss)", result, uri ? uri : "", color ? color : "")); Hrm. this stuff about the validation of the parameters seems a bit repetitive no? Perhaps an out: label with the method error and return.
Comment on attachment 237660 [details] [review] Patch 1/2 fixing naming issue and indentation Pushed this one already to master.
Created attachment 238097 [details] [review] Patch 2/2
Review of attachment 238097 [details] [review]: Looks better to me! ::: embed/web-extension/ephy-web-extension.c @@ +85,3 @@ + if (!web_page) { + return; + } The are no braces for one line statements, but I'll fix this before landing.
All patches landed, closing.