[webkit-reviews] review denied: [Bug 112419] [GTK][WK2] MiniBrowser custom URI scheme support : [Attachment 195949] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Apr 1 04:06:06 PDT 2013
Carlos Garcia Campos <cgarcia at igalia.com> has denied Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 112419: [GTK][WK2] MiniBrowser custom URI scheme support
https://bugs.webkit.org/show_bug.cgi?id=112419
Attachment 195949: Patch
https://bugs.webkit.org/attachment.cgi?id=195949&action=review
------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=195949&action=review
Looks better, setting r- because of the leaks and style issues.
> Tools/MiniBrowser/gtk/BrowserWindow.c:140
> + gtk_entry_set_text(GTK_ENTRY(window->uriEntry),
getExternalURI(webkit_web_view_get_uri(webView)));
This is leaking the uri, you should either use a variable to free the uri after
set_text, or cache the uri in the window structure and make getExternalURI
return a const char *
> Tools/MiniBrowser/gtk/BrowserWindow.c:301
> + titleOrURI =
getExternalURI(webkit_web_view_get_uri(window->webView));
titleOrURI is const char, getExternalURI returns a new allocated string that
you are leaking here.
> Tools/MiniBrowser/gtk/BrowserWindow.c:665
> + webkit_web_view_load_uri(window->webView, getInternalURI(uri));
Same leak here.
> Tools/MiniBrowser/gtk/main.c:212
> + contents = g_strdup_printf("<html><body><h1>WebKitGTK+
MiniBrowser</h1><p>About page example.</p><p>Version:
%d.%d.%d</p></body></html>",
We don't need to say this is an about page example, just a use a sentence like
any other about app. WebKitGTK+ MiniBrowser 2.0.0 The WebKit2 test browser of
the GTK+ port. Or something similar.
>>> Tools/MiniBrowser/gtk/main.c:215
>>> + webkit_get_micro_version ());
>>
>> Extra space before ( in function call [whitespace/parens] [4]
>
> Weird number of spaces at line-start. Are you using a 4-space indent?
[whitespace/indent] [3]
Fix all the style issue please.
More information about the webkit-reviews
mailing list