[Webkit-unassigned] [Bug 117533] [GTK] MiniBrowser to automatically download "non-showable" documents when left click in link

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117533


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #204425|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #3 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-06-12 05:05:08 PST ---
(From update of attachment 204425)
View in context: https://bugs.webkit.org/attachment.cgi?id=204425&action=review

Looks great, thanks! There are just a few minor nits.

> Tools/MiniBrowser/gtk/BrowserWindow.c:344
> +        {

This brace should be in the previous line.

> Tools/MiniBrowser/gtk/BrowserWindow.c:347
> +                || webkit_navigation_policy_decision_get_mouse_button(navigationDecision) != 2)

Now that we depend on recent GTK+ we can use GDK_BUTTON_MIDDLE instead of 2 :-)

> Tools/MiniBrowser/gtk/BrowserWindow.c:350
> +            // Opening a new window if link clicked with the middle button

Nit: Comments should finish with a period.

> Tools/MiniBrowser/gtk/BrowserWindow.c:352
> +            webkit_web_view_set_settings(newWebView, webkit_web_view_get_settings(webView));

I think this is no longer needed, since both web views are in the same web view group (the default one) and then sharing the settings already.

> Tools/MiniBrowser/gtk/BrowserWindow.c:361
> +        {

This brace should be in the previous line.

> Tools/MiniBrowser/gtk/BrowserWindow.c:367
> +            WebKitResponsePolicyDecision *responseDecision;
> +            WebKitURIResponse *response;
> +            WebKitURIRequest *request;
> +            WebKitWebResource *mainResource;
> +            const char *mimeType;
> +            const char *requestURI;

You can declare the variables when used instead of having this block.

> Tools/MiniBrowser/gtk/BrowserWindow.c:379
> +            if (g_strcmp0 (webkit_web_resource_get_uri(mainResource), requestURI))

Extra space between function name and parentheses.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.