Bug 137634 – [EFL] Fix popup non-occurrence condition.

• Component: WebKit EFL
• Status: RESOLVED DUPLICATE

[KwangHyuk]

In the Webkit EFL, After the focus is out of drop-down list popup without any item selection, popup just disappears. but for the next time, drop-down list doesn't appear again.

[KwangHyuk]

Created attachment 239669 [details]
Patch

[KwangHyuk]

Created attachment 239670 [details]
Patch

[Build Bot]

Comment on attachment 239670 [details]
Patch

Attachment 239670 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5425612898435072

New failing tests:
editing/unsupported-content/list-type-before.html
editing/unsupported-content/table-type-before.html
editing/unsupported-content/list-delete-003.html
editing/unsupported-content/list-delete-001.html
editing/unsupported-content/list-type-after.html
editing/unsupported-content/table-delete-002.html
editing/unsupported-content/table-type-after.html

[Build Bot]

Created attachment 239672 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[KwangHyuk]

Created attachment 239677 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 239677 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=239677&action=review

> Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:126
> +#if PLATFORM(EFL)

I think we can move this to WebPopupMenuEfl.cpp instead.

[KwangHyuk]

(In reply to comment #6)
> Comment on attachment 239677 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=239677&action=review
> 
> > Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:126
> > +#if PLATFORM(EFL)
> 
> I think we can move this to WebPopupMenuEfl.cpp instead.

I have checked it and reach any conclusion.

This doesn't look possible without modifying WebPopupMenu since WebPopupMenu is created using static create api and its own constructor is private member.

So, I am checking whether it's ok if I modify this ?

void RenderMenuList::hidePopup()
{
#if !PLATFORM(IOS)
    if (m_popup)
        m_popup->hide();

    // call popupDidHide() in here ? 
#endif
}

[KwangHyuk]

Created attachment 240676 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 240676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240676&action=review

> Source/WebCore/ChangeLog:12
> +        Therefore, popupDidHide() is called in the RenderMenuList:dePopup() method

dePopup -> hidePopup

> Source/WebCore/rendering/RenderMenuList.cpp:399
> +    popupDidHide();

This function call can influence on Mac and GTK port as well as EFL port. So I think it would be good to add a test case to test this change.

[Gyuyoung Kim]

Comment on attachment 240676 [details]
Patch

Cleared review? from attachment 240676 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug or this bug again.

[KwangHyuk]

Created attachment 253851 [details]
Patch

[KwangHyuk]

For Webkit GTK, popup is just blocked until user choose any item and it just returned from the showPopup routine.
Therefore, popupDidHide() is just called as soon as any item on the menu or out side area of menu is clicked via m_client->valueChangedForPopupMenu() in WebPopupMenuProxyGtk::showPopupMenu().
As a result, reset of m_popupIsvisible is guaranteed.

But for the Webkit EFL, elm_menu is activated from UIProcess, but it's not blocked until user's selection.
So, the working flow of each port are different from each other.

[Ryuan Choi]

(In reply to comment #12)
> For Webkit GTK, popup is just blocked until user choose any item and it just
> returned from the showPopup routine.
> Therefore, popupDidHide() is just called as soon as any item on the menu or
> out side area of menu is clicked via m_client->valueChangedForPopupMenu() in
> WebPopupMenuProxyGtk::showPopupMenu().
> As a result, reset of m_popupIsvisible is guaranteed.
> 
> But for the Webkit EFL, elm_menu is activated from UIProcess, but it's not
> blocked until user's selection.
> So, the working flow of each port are different from each other.

Could you test whether your issues are fine with below bug ?
https://bugs.webkit.org/show_bug.cgi?id=135454

I think that this is duplicated bug(but different approach)

[KwangHyuk]

(In reply to comment #13)
> (In reply to comment #12)
> > For Webkit GTK, popup is just blocked until user choose any item and it just
> > returned from the showPopup routine.
> > Therefore, popupDidHide() is just called as soon as any item on the menu or
> > out side area of menu is clicked via m_client->valueChangedForPopupMenu() in
> > WebPopupMenuProxyGtk::showPopupMenu().
> > As a result, reset of m_popupIsvisible is guaranteed.
> > 
> > But for the Webkit EFL, elm_menu is activated from UIProcess, but it's not
> > blocked until user's selection.
> > So, the working flow of each port are different from each other.
> 
> Could you test whether your issues are fine with below bug ?
> https://bugs.webkit.org/show_bug.cgi?id=135454
> 
> I think that this is duplicated bug(but different approach)

It fixed this issue.

Why don't you get reviewer's review ? :)

[Gyuyoung Kim]

Comment on attachment 253851 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253851&action=review

> Source/WebCore/ChangeLog:3
> +        [EFL] Fix popup non-occurrence condition.

It looks this patch is not only for EFL port. Doesn't this influence on GTK port too ? If so, please add [GTK] or [Win] prefix and so on.

> Source/WebCore/ChangeLog:15
> +        No new tests, no behavior change (OOPS!).

I think this patch may change behavior...

[Gyuyoung Kim]

(In reply to comment #13)
> (In reply to comment #12)
> > For Webkit GTK, popup is just blocked until user choose any item and it just
> > returned from the showPopup routine.
> > Therefore, popupDidHide() is just called as soon as any item on the menu or
> > out side area of menu is clicked via m_client->valueChangedForPopupMenu() in
> > WebPopupMenuProxyGtk::showPopupMenu().
> > As a result, reset of m_popupIsvisible is guaranteed.
> > 
> > But for the Webkit EFL, elm_menu is activated from UIProcess, but it's not
> > blocked until user's selection.
> > So, the working flow of each port are different from each other.
> 
> Could you test whether your issues are fine with below bug ?
> https://bugs.webkit.org/show_bug.cgi?id=135454
> 
> I think that this is duplicated bug(but different approach)

Hyuki, it looks the patch of Bug 135454 looks better fix for EFL port. What do you think about it ?

[KwangHyuk]

(In reply to comment #16)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > For Webkit GTK, popup is just blocked until user choose any item and it just
> > > returned from the showPopup routine.
> > > Therefore, popupDidHide() is just called as soon as any item on the menu or
> > > out side area of menu is clicked via m_client->valueChangedForPopupMenu() in
> > > WebPopupMenuProxyGtk::showPopupMenu().
> > > As a result, reset of m_popupIsvisible is guaranteed.
> > > 
> > > But for the Webkit EFL, elm_menu is activated from UIProcess, but it's not
> > > blocked until user's selection.
> > > So, the working flow of each port are different from each other.
> > 
> > Could you test whether your issues are fine with below bug ?
> > https://bugs.webkit.org/show_bug.cgi?id=135454
> > 
> > I think that this is duplicated bug(but different approach)
> 
> Hyuki, it looks the patch of Bug 135454 looks better fix for EFL port. What
> do you think about it ?

Agree with you. :)

[KwangHyuk]

*** This bug has been marked as a duplicate of bug 135454 ***