[Webkit-unassigned] [Bug 57944] [GTK] Implement WKView with a common widget that can be used by both C and gtk APIs

Wed Apr 27 00:29:10 PDT 2011

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

--- Comment #14 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-04-27 00:29:10 PST ---
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (From update of attachment 89758 [details] [details] [details])
> > > I'm going to r- this patch. On discussing this with Alex, it seems that we should be able to leave our current WebKit2 C-API design mostly unchanged.
> > 
> > what design?
> 
> This patch does a lot of different things at once:

yes, I didn't split it into different patches because it doesn't introduce new code, it simply moves existing one, so you don't need to review the code. 

> 1. It moves the WebKitViewWidget class to a different directory and gives it a different name.
> 2. It moves a lot of the logic from the PageClient / WebKit into the WebKitViewWidget. 
> 3. It changes the interface that's exposed in the existing API. Instead of a WKViewRef being an opaque C++ class pointer, it's now a GtkWidget.

Yes, that's what the webkit2 design docs says: "WKView[Ref]: Native view that hooks into the platform's toolkit. On Windows, this wraps a HWND. On the Mac, it inherits from NSView." So for GTK+ looks natural to me that WKView inherits from a GtkWidget. We discussed it already and we agreed on that.

> 
> I think that some of these things are a good idea and I'm not yet convinced about others.

which ones? it's also difficult to rewrite a patch without knowing what's wrong.

> Can you split this patch up a bit so we can discuss each bit separately? First let's move the WebKitViewWidget GObject. Then we can approach each of the other tasks separately.

Sure, I'll do it.

> In particular, I want to move the logic for handling key bindings to a separate class so WebKit1 and WebKit2 can share it. Once I finish that, the second part of this patch will be much smaller.

Makes a lot of sense.

> As for the third point, I feel we should discuss it and reach consensus before making the change.

Ok, let's do it again in different bugs.

> > what's wrong with my alternative?
> 
> It's hard to answer this question, because there are multiple things happening in your patch. It took me quite a while to review it because it was so large. Would you be willing to follow the approach to splitting this patch I suggested above? It will be easier for me and perhaps others to make decisions about your suggested changes if orthogonal parts are in separate patches and bugs. First you could just move and rename the WebKitViewWidget and we could go from there on different bugs.

Sure.

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