Bug 110209 – [EFL][WK2] ewk_popup_menu_close() does not work as intended

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Chris Dumez]

ewk_popup_menu_close() is called by the browser to notify WebKit that the popup menu was closed. However, the current implementation calls a smart function asking the browser to hide the popup menu instead of passing the information along to the WebProcess.

[Chris Dumez]

Created attachment 189055 [details]
Patch

[Chris Dumez]

Could I please get informal review?

[Gyuyoung Kim]

Comment on attachment 189055 [details]
Patch

It seems to me existing implementation is weird. LGTM. BTW, should we provide ewk_popup_menu_close()? When browser lets webkit know that popup menu is closed ? Any use case ?

[Chris Dumez]

(In reply to comment #3)
> (From update of attachment 189055 [details])
> It seems to me existing implementation is weird. LGTM. BTW, should we provide ewk_popup_menu_close()? When browser lets webkit know that popup menu is closed ? Any use case ?

Yes, we need to provide ewk_popup_menu_close() so that the browser lets us know when the popup menu gets hidden (so that WebCore updates its state). See MiniBrowser implementation in Bug 108934.

Some other ports do not need such API because they run an event loop while displaying the menu. However, this is not appropriate for EFL port as EFL has issues with nested event loops.

[Gyuyoung Kim]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 189055 [details] [details])
> > It seems to me existing implementation is weird. LGTM. BTW, should we provide ewk_popup_menu_close()? When browser lets webkit know that popup menu is closed ? Any use case ?
> 
> Yes, we need to provide ewk_popup_menu_close() so that the browser lets us know when the popup menu gets hidden (so that WebCore updates its state). See MiniBrowser implementation in Bug 108934.
> 
> Some other ports do not need such API because they run an event loop while displaying the menu. However, this is not appropriate for EFL port as EFL has issues with nested event loops.

I see. I talk with Byungwoo. Yes, we need to provide this API because of EFL nested event loop issue. Tizen WebKit also has same problem. BTW, I considered that we can add a new C API to hide popup menu. But, it looks it isn't good to add new C APIs for only EFL specific issue. LGTM.

[Benjamin Poulain]

Comment on attachment 189055 [details]
Patch

Ok for WebKit2.

[Laszlo Gombos]

Comment on attachment 189055 [details]
Patch

r=me.

[WebKit Review Bot]

Comment on attachment 189055 [details]
Patch

Clearing flags on attachment: 189055

Committed r145676: <http://trac.webkit.org/changeset/145676>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.