[Webkit-unassigned] [Bug 48512] [GTK] Implement sample browser app for WebKit2

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #86627|review?                     |review-
               Flag|                            |




--- Comment #16 from Martin Robinson <mrobinson at webkit.org>  2011-03-23 09:57:27 PST ---
(From update of attachment 86627)
View in context: https://bugs.webkit.org/attachment.cgi?id=86627&action=review

Sorry. I missed a few things. :/ My only issues here are minor style issues, but I feel like the change to createWindow will need another round.

> Tools/MiniBrowser/gtk/main.c:29
> +// FIXME: replace bool type in the C API
> +// https://bugs.webkit.org/show_bug.cgi?id=56668

I guess we may eventually need to add the stdbool.h to the headers ourselvses. :/

> Tools/MiniBrowser/gtk/main.c:33
> +#include <WebKit2/WebKit2.h>
> +#include <gtk/gtk.h>

You shoud reorder these includes.

> Tools/MiniBrowser/gtk/main.c:44
> +static void destroyCb(GtkWidget *widget, GtkWidget *window)

Please change Cb to Callback everywhere.

> Tools/MiniBrowser/gtk/main.c:49
> +static void goBackCb(GtkWidget *widget, WKViewRef *webView)

Ditto.

> Tools/MiniBrowser/gtk/main.c:54
> +static void goForwardCb(GtkWidget *widget, WKViewRef *webView)

Ditto.

> Tools/MiniBrowser/gtk/main.c:120
> +    GtkWidget *window;
> +    window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> +    gtk_window_set_default_size(GTK_WINDOW(window), 800, 600);
> +    gtk_widget_set_name(window, "MiniBrowser");
> +
> +    WKContextRef context = WKContextGetSharedProcessContext();
> +    *webView = WKViewCreate(context, 0);
> +
> +    GtkWidget *uriEntry;
> +    uriEntry = gtk_entry_new();
> +
> +    GtkWidget *vbox;
> +    vbox = gtk_vbox_new(FALSE, 0);
> +    gtk_box_pack_start(GTK_BOX(vbox), createToolbar(uriEntry, webView), FALSE, FALSE, 0);
> +    gtk_box_pack_start(GTK_BOX(vbox), WKViewGetWindow(*webView), TRUE, TRUE, 0);
> +
> +    gtk_container_add(GTK_CONTAINER(window), vbox);
> +
> +    g_signal_connect(window, "destroy", G_CALLBACK(destroyCb), NULL);
> +
> +    return window;

Oh, sorry. I guess I wasn't totally clear. You should do:

GtkWindow* window = gtk_window_new(GTK_WINDOW_TOPLEVEL); 

and the same with context, uriEntry, vbox, etc.

> Tools/MiniBrowser/gtk/main.c:144
> +    WKViewRef webView;
> +    GtkWidget *mainWindow;
> +    mainWindow = createWindow(&webView);
> +

I think I just realized what's confusing me about this bit. Here's my suggested change:

1. Create the WKWebView externally with a helper: createWebView.
2. Pass the WebView widget to createWindow.

In the end it will look like this:

WkViewRef webView = createWebView();
GtkWidget *mainWindow = createWindow(&webView);

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