Bug 171492

Expectations updated in r216011

This started failing after the following set of fixes which I made for fixing the following popup-related bugs: * Bug #145866: [GTK] Attach popup menu to web view widget Landed: http://trac.webkit.org/changeset/215225 * Bug #170553: [GTK] Misplaced right click menu on web page due to deprecated gtk_menu_popup() Landed: http://trac.webkit.org/changeset/215190 * Bug #170680: [GTK] Opening a popup menu does not pre-select the active item Landed: http://trac.webkit.org/changeset/215188 Reverting r215225 makes the typeahead find elements again, but the items being found sometimes are not exactly the ones from the test expectation. The diff is: --- /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/gtk/fast/forms/menulist-typeahead-find-expected.txt +++ /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/gtk/fast/forms/menulist-typeahead-find-actual.txt @@ -3,8 +3,8 @@ Cannot run interactively. Expected Apple: was Apple Expected Carrot: was Carrot -Expected Cauliflower: was Cauliflower -Expected Corn: was Corn +Expected Cauliflower: was Carrot +Expected Corn: was Cauliflower Expected Carrot: was Carrot Expected Apple: was Apple Expected Cauliflower: was Cauliflower The bit that is not working is this part of the test: // This tests that iterating via pressing the same key works. testTypeAheadFind(['c', 'c'], "Cauliflower"); testTypeAheadFind(['c', 'c', 'c'], "Corn"); Checking the code of the “testTypeAheadFind()” function, I see that it does not attempt to reset the status of the form element to an initial know state, so each call to this function will behave differently depending on the state leftover from the previous call. After fixing bug #170680 now the code is pre-selecting the active item on popup (simulated here by sending a pair of button-down/button-up events): that was not happening before and the typeahead was searching always from the beginning of the list. IMHO the correct thing to do in this regard is to always reset the form element to a known state at the beginning of “testTypeAheadFind()”. I still don't know why attaching the popup menu to the web widget makes the test function always choose the “Apple” item, though. That needs more investigation.

The following is the diff with the patch in bug #171792 applied, but *not* reverting the patch to attach the popup to the web view: --- /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/gtk/fast/forms/menulist-typeahead-find-expected.txt +++ /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/gtk/fast/forms/menulist-typeahead-find-actual.txt @@ -2,10 +2,10 @@ Cannot run interactively. Expected Apple: was Apple -Expected Carrot: was Carrot -Expected Cauliflower: was Cauliflower -Expected Corn: was Corn -Expected Carrot: was Carrot -Expected Apple: was Apple -Expected Cauliflower: was Cauliflower -Expected Star fruit: was Star fruit +Expected Carrot: was +Expected Cauliflower: was +Expected Corn: was +Expected Carrot: was +Expected Apple: was +Expected Cauliflower: was +Expected Star fruit: was It's quite interesting that the first attempt of using the type ahead search works, but the rest don't. This leads me to think that it might be needed to detach the popup from the web view after the popup menu is closed (either by choosing a new value with keyboard/mouse, or by cancelling it e.g. with “Escape”). I'll give that a try, but if that doesn't solve the issue, it could be that some digging into the GTK+ code for gtk_menu_attach_to_widget() may be needed.

(In reply to Adrian Perez from comment #3) > The following is the diff with the patch in bug #171792 applied, but > *not* reverting the patch to attach the popup to the web view: > > [...] > > It's quite interesting that the first attempt of using the type ahead > search works, but the rest don't [...] This happens because gtk_menu_attach_to_widget() is sinking the floating reference to the passed GtkMenu, making the GTK+ innards “own” the widget. That means that GTK+ unrefs the GtkMenu and we are left with an invalid “m_popup” pointer. We need to either sink the reference ourselves when constructing a WebPopupMenuProxyGtk, or add an additional reference. Doing one of those fixes the case of no items appearing as selectd after the first one — but I still need to figure out how to get the test to pass.

Some observations I made while trying to figure out what's going on: 1. The first time a popup is opened, a WebPopupMenuProxyGtk is instantiated, and ::showPopupMenu() called on it. 2. Once the menu is dismissed, the WebPopupMenuGtk instance is kept around. 3. The next times a menu is opened: 3.a. The object created in (1) is destroyed. 3.b. Then a new WebPopupMenuProxyGtk is created (even if the menu being opened is exactly the same one used previously). 3.c. WebPopupMenuProxyGtk::showPopupMenu() is called. I double-checked the above by debugging while using MiniBrowser. When using WebKitTestRunner to run the test, only steps (1) to (3.a) happen, and new instances of WebPopupMenuProxyGtk are not ever created. It looks like the events synthesized with “eventSender.mouse{Down,Up}()” used to programmatically open the popup menu from the test are only reaching WebCore on the first use, and then things get stalled from there.

(In reply to Adrian Perez from comment #2) > This started failing after the following set of fixes which I made for > fixing the following popup-related bugs: > > * Bug #145866: [GTK] Attach popup menu to web view widget > Landed: http://trac.webkit.org/changeset/215225 > > * Bug #170553: [GTK] Misplaced right click menu on web page due to > deprecated gtk_menu_popup() > Landed: http://trac.webkit.org/changeset/215190 > > * Bug #170680: [GTK] Opening a popup menu does not pre-select the active > item > Landed: http://trac.webkit.org/changeset/215188 > > Reverting r215225 makes the typeahead find elements again, but the > items being found sometimes are not exactly the ones from the test > expectation. The diff is: > > --- > /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/ > gtk/fast/forms/menulist-typeahead-find-expected.txt > +++ > /home/aperez/devel/WebKit/WebKitBuild/Release/layout-test-results/platform/ > gtk/fast/forms/menulist-typeahead-find-actual.txt > @@ -3,8 +3,8 @@ > Cannot run interactively. > Expected Apple: was Apple > Expected Carrot: was Carrot > -Expected Cauliflower: was Cauliflower > -Expected Corn: was Corn > +Expected Cauliflower: was Carrot > +Expected Corn: was Cauliflower > Expected Carrot: was Carrot > Expected Apple: was Apple > Expected Cauliflower: was Cauliflower > > The bit that is not working is this part of the test: > > // This tests that iterating via pressing the same key works. > testTypeAheadFind(['c', 'c'], "Cauliflower"); > testTypeAheadFind(['c', 'c', 'c'], "Corn"); > > Checking the code of the “testTypeAheadFind()” function, I see that it > does not attempt to reset the status of the form element to an initial > know state, so each call to this function will behave differently > depending on the state leftover from the previous call. > > After fixing bug #170680 now the code is pre-selecting the active item > on popup (simulated here by sending a pair of button-down/button-up > events): that was not happening before and the typeahead was searching > always from the beginning of the list. IMHO the correct thing to do in > this regard is to always reset the form element to a known state at the > beginning of “testTypeAheadFind()”. > > I still don't know why attaching the popup menu to the web widget makes > the test function always choose the “Apple” item, though. That needs more > investigation. This is because we are dismissing any menus attached to the web view for every event we inject. This was done for the context menu in r184015, but now we are also attaching popup menus. So, we show the popup menu and as soon as we try to inject a keyevent, the popup is dismissed. That was a workaround in the end, so it's time to properly fix bug #40601.

(In reply to Carlos Garcia Campos from comment #6) > This is because we are dismissing any menus attached to the web view for > every event we inject. This was done for the context menu in r184015, but > now we are also attaching popup menus. So, we show the popup menu and as > soon as we try to inject a keyevent, the popup is dismissed. That was a > workaround in the end, so it's time to properly fix bug #40601. Wow, I wasn't aware of the workaround. Thanks a lot for looking into this and figuring out the reason for the weird behavior of the event handling in WebKitTestRunner.

Created attachment 313077 [details] Patch

Committed r218391: <http://trac.webkit.org/changeset/218391>