Bug 32008 – [Chromium] Keyboard shortcut in dropdown not working.

• Component: Platform
• Status: RESOLVED FIXED

[James Su]

See chromium issue: http://crbug.com/28776:

Chrome Version       : 4.0.257.0~svn20091124r32926-0ubuntu1~ucd1
OS + version : Ubuntu 9.10
CPU architecture (32-bit / 64-bit): 32-bit
window manager : Gnome
URLs (if applicable) : http://www.xe.com/
Behavior in Firefox 3.x (if applicable): OK
Behavior in Chrome for Windows (optional): OK

What steps will reproduce the problem?
1. Enter http://www.xe.com/
2. Click on the dropdown to select currency
3. Type the first letter of a currency, for example E (for EUR)

What is the expected result?

EUR should be selected.

What happens instead?

Nothing. The only way to change the dropdown with the keyboard is using the 
arrows.

[James Su]

Created attachment 44055 [details]
Patch to fix this issue.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44055 [details] without any errors.

[Evan Martin]

[I am not a WebKit reviewer.] This seems fine to me.

Is there a way to test this?  Can you make the description of what you're fixing more specific in the ChangeLog?  (For someone who doesn't know what "keyboard shortcut" means -- I think you mean that pressing a key currently doesn't select the entry starting with that letter?)

[James Su]

Created attachment 44058 [details]
Patch to fix this issue.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44058 [details] without any errors.

[James Su]

I updated the ChangeLog a little.

The "keyboard shortcut" is actually not accurate. This issue is about the "type ahead find" feature of the popup listbox, or so called prefix search: you can type in a popup listbox to search the items by prefix. It works on Chromium windows port. The attached patch fixes the Linux and Mac ports.

[Hironori Bono]

(In reply to comment #3)
> [I am not a WebKit reviewer.] This seems fine to me.
> 
> Is there a way to test this?

If I recall correctly, we need to change eventSender (and some other code) to write a layout test for this issue so PopupMenu can receive keyboard events sent from eventSender. Even though I have been writing a change to dispatch keyboard events from eventSender to PopupMenu, it is not a high priority issue. (This is the reason why I have not sent any fixes for this issue.) So, I wish James also changes eventSender and adds layout tests.

Regards,

[James Su]

(In reply to comment #7)
> (In reply to comment #3)
> > [I am not a WebKit reviewer.] This seems fine to me.
> > 
> > Is there a way to test this?
> 
> If I recall correctly, we need to change eventSender (and some other code) to
> write a layout test for this issue so PopupMenu can receive keyboard events
> sent from eventSender. Even though I have been writing a change to dispatch
> keyboard events from eventSender to PopupMenu, it is not a high priority issue.
> (This is the reason why I have not sent any fixes for this issue.) So, I wish
> James also changes eventSender and adds layout tests.
> 
> Regards,

I'm not familiar with this part. I'd very appreciate if you could help on it.

[James Su]

Created attachment 44063 [details]
Patch to fix this issue.

Merged the case-sensitive type ahead find fix from WebCore/dom/SelectElement.cpp. See https://bugs.webkit.org/show_bug.cgi?id=29103.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44063 [details] without any errors.

[Darin Fisher (:fishd, Google)]

Comment on attachment 44063 [details]
Patch to fix this issue.

r=me, but given this and other recent bug fixes in the popup menu code, I think it would be great to invest time in enabling layout tests for this code.

[WebKit Commit Bot]

Comment on attachment 44063 [details]
Patch to fix this issue.

Clearing flags on attachment: 44063

Committed r52519: <http://trac.webkit.org/changeset/52519>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.