Currently only way of saving to disk a link is through the context menu. Non-showable/navigatable documents behind links lead to no action in MiniBrowser. We would like to start automatically the download of such documents when left clicking it their link.
Created attachment 204425 [details] Patch
Comment on attachment 204425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204425&action=review Overall LGTM > Tools/ChangeLog:11 > + itself on link left clicking. The patch description normally goes just after the Reviewed by paragraph. These lines are normally used to comment about specific implementation stuff in some functions > Tools/MiniBrowser/gtk/BrowserWindow.c:371 > + mimeType = webkit_uri_response_get_mime_type(response); It's normally preferred to initialize the values in the declarations so: WebKitResponsePolicyDecision *responseDecision = WEBKIT_RESPONSE_POLICY_DECISION(decision); WebKitURIResponse *response = webkit_response_policy_decision_get_response(responseDecision); and the like > Tools/MiniBrowser/gtk/BrowserWindow.c:379 > + if (g_strcmp0 (webkit_web_resource_get_uri(mainResource), requestURI)) remove the blank after g_strcmp0
Comment on attachment 204425 [details] Patch 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.
Created attachment 204428 [details] Patch
Comment on attachment 204428 [details] Patch Perfect!
(In reply to comment #2) > (From update of attachment 204425 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204425&action=review > > Overall LGTM (In reply to comment #5) > (From update of attachment 204428 [details]) > Perfect! Thanks for the reviews, guys!
Comment on attachment 204428 [details] Patch Clearing flags on attachment: 204428 Committed r151496: <http://trac.webkit.org/changeset/151496>
All reviewed patches have been landed. Closing bug.