Bug 171803

Summary: [GTK] Type ahead find for <option> elements inside an <optgroup> does not work in popup
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WebKitGTKAssignee: Claudio Saavedra <csaavedra>
Status: NEW    
Severity: Normal CC: bugs-noreply, buildbot, calvaris, cgarcia, csaavedra
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=186146
Bug Depends on: 171492    
Bug Blocks:    
Attachments:
Description Flags
Patch
cgarcia: review+, buildbot: commit-queue-
Archive of layout-test-results from ews116 for mac-elcapitan none

Adrian Perez
Reported 2017-05-08 06:03:36 PDT
If a <select> element contains <option>s inside an <optgroup>, the type-ahead search never picks those <option>s when the popup menu is opened. To reproduce the issue, save the following HTML snippet in a file: <!DOCTYPE html> <html> <body> <form action="#"> <label for="opt">Choose</label> <select name="opt"> <optgroup label="Fruits"> <option>Apple</option> <option>Strawberry</option> </optgroup> <optgroup label="Desserts"> <option>Cake</option> <option>Flan</option> </optgroup> </select> </form> </body> </html> Then open it (e.g. in MiniBrowser), click the element to make the popup menu appear, and try entering the first letter of one of the <option>s. For example entering “s” should select “Strawberry“, but currently this is not working.
Attachments
Patch (1.89 KB, patch)
2017-05-11 03:08 PDT, Claudio Saavedra 		cgarcia: review+
buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews116 for mac-elcapitan (1.77 MB, application/zip)
2017-05-11 04:31 PDT, Build Bot 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Adrian Perez
Comment 1 2017-05-08 06:07:59 PDT
A couple more notes: - This happens with current ”trunk”, and with the latest stable release (2.16.1). Most likely this has not ever worked well. - Both Firefox and Chromium handle type-ahead search correctly when <optgroup>s are used (tested with the snippet above). - If the <select> element is focused, but the popup is *not opened*, then the type ahead search works (likely this is case is handled in the shared WebKit code).
Claudio Saavedra
Comment 2 2017-05-11 03:08:16 PDT
Created attachment 309703 [details] Patch
Build Bot
Comment 3 2017-05-11 04:31:13 PDT
Comment on attachment 309703 [details] Patch Attachment 309703 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/3717316 New failing tests: fast/dom/Window/setTimeout-setInterval-unique.html
Build Bot
Comment 4 2017-05-11 04:31:14 PDT
Created attachment 309705 [details] Archive of layout-test-results from ews116 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Carlos Garcia Campos
Comment 5 2017-05-11 04:39:57 PDT
Comment on attachment 309703 [details] Patch Thanks! Could we add a tests for this?
Adrian Perez
Comment 6 2017-05-11 04:51:32 PDT
(In reply to Carlos Garcia Campos from comment #5) > Comment on attachment 309703 [details] > Patch > > Thanks! Could we add a tests for this? I have been trying to fix bug #171492 (without much luck so far; hint: help welcome) before trying to post a patch for this one, precisely to be able of adding the test case :-P BTW, Claudio's patch looks good to me and works.
Claudio Saavedra
Comment 7 2017-05-11 06:30:48 PDT
(In reply to Carlos Garcia Campos from comment #5) > Comment on attachment 309703 [details] > Patch > > Thanks! Could we add a tests for this? There is already a test in place, but it seems to be broken (regardless of this patch), see bug 171492.
Carlos Garcia Campos
Comment 8 2017-05-11 06:34:30 PDT
(In reply to Claudio Saavedra from comment #7) > (In reply to Carlos Garcia Campos from comment #5) > > Comment on attachment 309703 [details] > > Patch > > > > Thanks! Could we add a tests for this? > > There is already a test in place, but it seems to be broken (regardless of > this patch), see bug 171492. But does that test already check this particular issue?
Claudio Saavedra
Comment 9 2017-05-11 07:11:31 PDT
OK, you're right, it doesn't, because that test doesn't have the <optgroup> so there is no space indentation. I'll cook a test but I am afraid it will fail nonetheless for the same reasons than the other one.
Adrian Perez
Comment 10 2017-06-16 09:28:06 PDT
(In reply to Claudio Saavedra from comment #9) > OK, you're right, it doesn't, because that test doesn't have the <optgroup> > so there is no space indentation. I'll cook a test but I am afraid it will > fail nonetheless for the same reasons than the other one. We have now the test passing in trunk. Could you take a look at adding a test now for items inside an <optgroup>, so we can get this fix landed? (Thanks!)
Note You need to log in before you can comment on or make changes to this bug.