RESOLVED FIXED 110209
[EFL][WK2] ewk_popup_menu_close() does not work as intended
https://bugs.webkit.org/show_bug.cgi?id=110209
Summary [EFL][WK2] ewk_popup_menu_close() does not work as intended
Chris Dumez
Reported 2013-02-19 04:22:51 PST
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.
Attachments
Patch (1.64 KB, patch)
2013-02-19 04:32 PST, Chris Dumez
no flags
Chris Dumez
Comment 1 2013-02-19 04:32:58 PST
Chris Dumez
Comment 2 2013-03-11 15:23:25 PDT
Could I please get informal review?
Gyuyoung Kim
Comment 3 2013-03-11 23:53:03 PDT
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
Comment 4 2013-03-12 00:02:37 PDT
(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
Comment 5 2013-03-12 00:25:07 PDT
(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 6 2013-03-12 14:16:43 PDT
Comment on attachment 189055 [details] Patch Ok for WebKit2.
Laszlo Gombos
Comment 7 2013-03-12 20:40:57 PDT
Comment on attachment 189055 [details] Patch r=me.
WebKit Review Bot
Comment 8 2013-03-12 20:49:12 PDT
Comment on attachment 189055 [details] Patch Clearing flags on attachment: 189055 Committed r145676: <http://trac.webkit.org/changeset/145676>
WebKit Review Bot
Comment 9 2013-03-12 20:49:16 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.