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

Fri Apr 8 00:37:25 PDT 2011

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

--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-04-08 00:37:25 PST ---
(In reply to comment #5)
> (From update of attachment 88416 [details])
> Looks good, I'm can not see any issue in the C API handling the widget directly instead of just getting it from a function, so it is a good option.
> 
> Still not sure if we should have the widget in the C API directory, because in theory the C API should be the base API so I guess it should not depend at all in the gtk API files. Also what about using WKWebViewWidget instead of WebKitWebViewBase, to make it more like the other C API files.

WebKitWebViewBase is something in the middle of both the C and the gtk API. Martin was worried that this widget will appear in the object hierarchy in the docs of the gtk API, so I decided to use the prefix of the gtk API for consistency. Both Mac and Qt have their view widgets in UIProcess/API/mac|qt so I tried to be consistent with other ports too. And regarding the name, using the word Widget in the name of class inheriting from GtkWidget sounded a bit redundant to me, but I don't mind to rename it if you think it's better. 

> View in context: https://bugs.webkit.org/attachment.cgi?id=88416&action=review
> 
> > Source/WebKit2/UIProcess/API/C/gtk/WKView.cpp:38
> > +G_DEFINE_TYPE(WKView, wk_view, WEBKIT_TYPE_WEB_VIEW_BASE)
> 
> Currently we are implementing directly the get type functions to make the code more WebKit style, that would avoid some of the style warnings.

I think some exceptions to the coding style make sense, like this one, G_DEFINE_TYPE implementation has changed several times, using the macro improves the maintainability, and it introduces only a couple of funtions that don't follow the coding style. But again, if you think it's not worth it, I'll change it. 

> > Source/WebKit2/UIProcess/API/C/gtk/WKView.cpp:48
> > +// Public API
> 
> Add a period to the end of the comment.

Ok.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:49
> > +G_DEFINE_ABSTRACT_TYPE(WebKitWebViewBase, webkit_web_view_base, GTK_TYPE_CONTAINER)
> 
> Ditto, this will fix the style warning.
> 
> > Source/WebKit2/UIProcess/API/gtk/webkitdefines.h:42
> > +/*
> > + * Copyright (C) 2007 Holger Hans Peter Freyther
> > + * Copyright (C) 2008 Collabora Ltd.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Library General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Library General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Library General Public License
> > + * along with this library; see the file COPYING.LIB.  If not, write to
> > + * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> > + * Boston, MA 02110-1301, USA.
> > + */
> > +
> > +#ifndef webkitdefines_h
> > +#define webkitdefines_h
> > +
> > +#include <glib.h>
> > +
> > +#ifdef G_OS_WIN32
> > +    #ifdef BUILDING_WEBKIT
> > +        #define WEBKIT_API __declspec(dllexport)
> > +    #else
> > +        #define WEBKIT_API __declspec(dllimport)
> > +    #endif
> > +    #define WEBKIT_OBSOLETE_API WEBKIT_API
> > +#else
> > +    #define WEBKIT_API __attribute__((visibility("default")))
> > +    #define WEBKIT_OBSOLETE_API WEBKIT_API __attribute__((deprecated))
> > +#endif
> > +
> > +#ifndef WEBKIT_API
> > +    #define WEBKIT_API
> > +#endif
> > +
> > +#endif
> 
> Honestly I'm not sure if we should just forget about reusing code between this API and the webkit1 or try to share some code. At this point I think trying to share code could be overkill at some point so I'm ok with copying the files but open to proposals.

Yes, I wan't sure either. 

Thank you very much for the coments

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