Bug 135454 – Select popup is not relaunched on the WebKit2 port

• Component: WebKit2
• Status: NEW

[Ryuan Choi]

Reproduce step is below in MiniBrowser/EFL
1. click the select tag and observe that select popup is launched.
2. click the out of select tag and observe that select popup is disappeared.(After crash is fixed by Bug 135378)
3. click the select tag once more.

In (2), HTMLSelectElement instance calls menuList.hidePopup() in the menuListDefaultEventHandler()
because m_popupIsVisible of menuList is true.
And WebPopupMenu::hide() clears m_activePopupMenu, but m_popupIsVisible of menuList remains as true.
From then, HTMLSelectElement instance can't call showPopup() because it does not have a way to change m_popupIsVisible.

[Ryuan Choi]

Created attachment 235812 [details]
Patch

[Build Bot]

Comment on attachment 235812 [details]
Patch

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

New failing tests:
media/track/add-and-remove-track.html

[Build Bot]

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

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

[Ryuan Choi]

Created attachment 235873 [details]
retry

[Sanghyup Lee]

*** Bug 136817 has been marked as a duplicate of this bug. ***

[Ryuan Choi]

Created attachment 253987 [details]
Patch

[KwangHyuk]

*** Bug 137634 has been marked as a duplicate of this bug. ***

[Gyuyoung Kim]

Comment on attachment 253987 [details]
Patch

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

> Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:125
> +    m_popupClient->popupDidHide();

Is this call fine for other ports too ?

[Darin Adler]

Comment on attachment 253987 [details]
Patch

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

>> Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:125
>> +    m_popupClient->popupDidHide();
> 
> Is this call fine for other ports too ?

Another way to put this is: We need a comment explaining why this is helpful, or at least harmless, for non-EFL ports. Either in the change log or in source code comments.

[Ryuan Choi]

Created attachment 254047 [details]
testpage.html

I uploaded test page that reproduces this issue on the all webkit2 ports.
I tested this and reproduced the issue on safari without my patch.

But, I am not sure how I makes the unit testcase or layout testcase for the automation except EFL port.
If I am right, show() and hide() of PopupMenu and related routines are not exposed to test frameworks or API layer except EFL port.
WebKit/Efl exposes them as API because WebKit/Efl does not have a power to draw popup without higher level ui library which it does not depend on.

[KwangHyuk]

Just short idea.

May be another approach would be applying GyuYoung's idea. (see Bug 137634)
- Inherit WebPopupMenu as WebPopupMenuEfl, then add EFL implementation. :)
- However, this approach may touch access modifier of WebPopupMenu's some apis.

There will be one more option.
Like GTK port, Webkit EFL can allow applying of negative value for item selection when user clicks the area out of elm_menu.
Then it may try to call popupDidHide() method.

[Ryuan Choi]

(In reply to comment #11)
> Just short idea.
> 
> May be another approach would be applying GyuYoung's idea. (see Bug 137634)
> - Inherit WebPopupMenu as WebPopupMenuEfl, then add EFL implementation. :)
> - However, this approach may touch access modifier of WebPopupMenu's some
> apis.
> 
As I mentioned above, I think that this is common issue.

> There will be one more option.
> Like GTK port, Webkit EFL can allow applying of negative value for item
> selection when user clicks the area out of elm_menu.
> Then it may try to call popupDidHide() method.

I think that it's not helpfull.
The problem is that select element receives blur event and call hide() but m_popupVisible is still true so that next trial to show the popup is denied and also UIProcess can't call popupDidHide because activePopupMenu set to nullptr.

[Gyuyoung Kim]

Comment on attachment 253987 [details]
Patch

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

>>> Source/WebKit2/WebProcess/WebCoreSupport/WebPopupMenu.cpp:125
>>> +    m_popupClient->popupDidHide();
>> 
>> Is this call fine for other ports too ?
> 
> Another way to put this is: We need a comment explaining why this is helpful, or at least harmless, for non-EFL ports. Either in the change log or in source code comments.

r- because of missing to reply our question and Darin's suggestion.