[webkit-reviews] review granted: [Bug 48512] [GTK] Implement sample browser app for WebKit2 : [Attachment 86961] Proposed patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Mar 25 11:29:44 PDT 2011
Martin Robinson <mrobinson at webkit.org> has granted Alejandro G. Castro
<alex at igalia.com>'s request for review:
Bug 48512: [GTK] Implement sample browser app for WebKit2
https://bugs.webkit.org/show_bug.cgi?id=48512
Attachment 86961: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=86961&action=review
------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86961&action=review
Great! Please change all WKViewRef* to just be WKViewRef since WKViewRef is
already a pointer type.
> Tools/MiniBrowser/gtk/GNUmakefile.am:2
> +#MiniBrowser
I think you can omit this.
> Tools/MiniBrowser/gtk/main.c:33
> + WKViewRef *webView = g_object_get_data(G_OBJECT(entry), "web-view");
data should now just be a WKViewRef. See below.
> Tools/MiniBrowser/gtk/main.c:53
> +static GtkWidget *createToolbar(GtkWidget *uriEntry, WKViewRef *webView)
Here just use WKViewRef instead of WKViewRef*.
> Tools/MiniBrowser/gtk/main.c:64
> + /* the back button */
Please either make these comments into full sentences or just remove them. You
can probalby just remove them.
> Tools/MiniBrowser/gtk/main.c:69
> + /* The forward button */
Ditto.
> Tools/MiniBrowser/gtk/main.c:74
> + /* The URL entry */
Ditto.
> Tools/MiniBrowser/gtk/main.c:81
> + /* The go button */
Ditto.
> Tools/MiniBrowser/gtk/main.c:94
> + WKContextRef context = WKContextGetSharedProcessContext();
> +
> + return WKViewCreate(context, 0);
Please just change this to: WKViewCreate(WKContextGetSharedProcessContext(),
0);
> Tools/MiniBrowser/gtk/main.c:97
> +static GtkWidget *createWindow(WKViewRef* webView)
Here just use WKViewRef instead of WKViewRef*. It's already a pointer.
> Tools/MiniBrowser/gtk/main.c:135
> + GtkWidget *mainWindow = createWindow(&webView);
Here you should just pass the WKViewRef itself, since it's already a pointer.
> Tools/MiniBrowser/gtk/main.c:137
> + gchar *uri = (gchar*)(argc > 1 ? argv[1] : "http://www.webkitgtk.org/");
Is this cast necessary?
More information about the webkit-reviews
mailing list