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

[Gustavo Sverzut Barbieri]

On Fri, Apr 16, 2010 at 11:21 AM, Luiz Agostini
<luiz.agostini at openbossa.org> wrote:
> 2010/4/16 Gustavo Sverzut Barbieri <barbieri at profusion.mobi>
>>
>> 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.
>
> I did not suggest a class for PopupMenuDelegate: "Each port could then
> provide its typedef for PopupMenuDelegate".
> Remember PlatformWidget? :-)

yes, but we still need the hide to get it hidden :-)


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