[webkit-dev] Clean way to allow delegates for PopupMenu?

[Gustavo Sverzut Barbieri]

On Fri, Apr 16, 2010 at 10:30 AM, Luiz Agostini
<luiz.agostini at openbossa.org> wrote:
>
> 2010/4/15 Gustavo Sverzut Barbieri <barbieri at profusion.mobi>
>>
>> Hello all,
>>
>> I'm part of the EFL port team and we're implementing the PopupMenu,
>> however EFL is a different platform as for our port it is just a
>> state-full canvas, not relying on any widget set/toolkit, we do not
>> have the concept of a menu at all!
>>
>> Looking at all existing implementations, they all go straight to
>> native platform menus. But we can't as we want to avoid such
>> dependencies. What we'd like to have is a delegate, that WebCore calls
>> the ChromeClient that is overloaded in WebKit, giving our View user a
>> callback with all data.
>>
>> The good news is: we did it and it works quite well.
>>
>> The bad news is: we cheated and include WebKit/efl/WebCoreSupport from
>> PopupMenuEfl.cpp, what a shame! (although some ports *cough qt*
>> include in WebCore defines from WebKit)
>
> In Qt we have an abstract class for popup delegates in WebCore/platform/qt
> and we do the actual popup delegate implementation in
> WebKit/qt/WebCoreSupport. The delegates are implemented in WebKit layer and
> PopupMenuQt has no knowledge about any specific delegates. The only reason
> to include WebKit/qt/WebCoreSupport in PopupMenuQt is to make the call that
> creates the delegate object. It could be avoided adding a method to Chrome
> but at the time I was working on it it was decided that it was not needed.

Yes, it is kinda similar to our, since we looked at your
implementation before doing our. But the problem here is you still
include WebKit files from WebCore.  For *me* it looks like a hack, but
if people do not complain then I can kindly keep it as is, after all
it is working already :-)


>> In order to have a clean design, we'd like to know the general opinion
>> on how to do it. From what I see Mac already defines a similar call in
>> ChromeClient.h:
>>
>> #if PLATFORM(MAC)
>> ...
>>        virtual void willPopUpMenu(NSMenu *) { }
>> #endif
>>
>> in our case, we'd need:
>>
>> #if PLATFORM(EFL)
>>        virtual void showPopupMenu(const IntRect& rect, FrameView*
>> view, int index) { }
>>        virtual void hidePopupMenu() { }
>> #endif
>>
>> so our PopupMenuEfl.cpp will just proxy to these calls.
>>
>> However, although Mac does that it may not be the best solution, so to
>> avoid endless patches to be discussed at bugzilla I'd like to know
>> your opinion on the best way so our patch is right from beginning.
>
>
> I think that we could add methods to Chrome and ChromeClient to create the
> delegates. For example:
> PopupMenuDelegate* Chrome::createPopupMenuDelegate(PopupMenuClient* c) {
> return m_client->createPopupMenuDelegate(c); }
> virtual PopupMenuDelegate*
> ChromeClient::createPopupMenuDelegate(PopupMenuClient*) { return 0; }
> Each port could then provide its typedef for PopupMenuDelegate and
> override ChromeClient::createPopupMenuDelegate.

Well, as I said we just need methods to show/hide to keep it done, but
if you want to implement another class "PopupMenuDelegate" to do it,
then we could do such work as well. We'd like to avoid this delegate
class as our port is C, so we'd need to do it all in C++ plus what we
have done in C already, just to proxy it.


BR,

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: barbieri at gmail.com
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202