Bug 110209

Summary:

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

Product:

WebKit

Reporter:

Chris Dumez <cdumez>

Component:

WebKit EFL

Assignee:

Chris Dumez <cdumez>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, benjamin, gyuyoung.kim, kenneth, laszlo.gombos, lucas.de.marchi, mikhail.pozdnyakov, rakuco, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

108934, 111543

Attachments:

Description	Flags
Patch	none

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	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2013-02-19 04:32:58 PST

Created attachment 189055 [details] Patch

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.