[Webkit-unassigned] [Bug 48509] [GTK] Implement WebView and WebKitWebView classes for WebKit2

[bugzilla-daemon at webkit.org]

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





--- Comment #36 from Alejandro G. Castro <alex at igalia.com>  2011-02-25 03:15:26 PST ---
(In reply to comment #35)
> (From update of attachment 83285 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=83285&action=review
> 
> Looks good, but please make the following changes.
> 

Thanks for the review :).

> > Source/WebKit2/UIProcess/gtk/WebKitWebView.cpp:92
> > +#ifdef GTK_API_VERSION_2
> > +#if GTK_CHECK_VERSION(2, 20, 0)
> > +    gtk_widget_style_attach(widget);
> > +#else
> > +    widget->style = gtk_style_attach(gtk_widget_get_style(widget), window);
> > +#endif
> 
> A nice cleanup would be to make this handled by GtkVersioning.h in a followup patch.
> 

Added bug:

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

> > Source/WebKit2/UIProcess/gtk/WebKitWebView.cpp:145
> > +    GdkWindow* window = gtk_widget_get_window(widget);
> > +    cairo_t* cr = gdk_cairo_create(window);
> > +
> > +    webView->paint(widget, clipRect, cr);
> > +
> 
> This should be smarter eventually. It's probably a good idea to open a bug tracking moving the smarts from WebKit1.
>

I'm going to change it to use them.

> > Source/WebKit2/UIProcess/gtk/WebKitWebView.cpp:165
> > +static void webViewOnSizeAllocate(GtkWidget* widget, GtkAllocation* allocation)
> 
> Please rename this to webViewSizeAllocate for consistency.
>
> > Source/WebKit2/UIProcess/gtk/WebKitWebView.cpp:349
> > +    static volatile gsize gDefineTypeIdVolatile = 0;
> > +    if (g_once_init_enter(&gDefineTypeIdVolatile)) {
> 
> Please turn this into an early return.
> 
> > Source/WebKit2/UIProcess/gtk/WebKitWebView.h:29
> > +#ifndef WebKitWebView_h
> > +#define WebKitWebView_h
> 
> Let's call this WebViewWidget for now,  just so that the separation of concerns is clear. I think having a GtkWidget called WebKitWebView and a C++ class called WebView is too confusing for the moment. Later on we can focus more heavily on what kind of interface we want to expose to the end user. This will be easier once we see how this looks for the released version of Mac and Windows.

All done, thanks again, I'll try to land it.

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