(In reply to comment #2)
> (From update of attachment 30549[details] [review])
> The patch looks sane to me, but I think we can make a test for this, no? We
> certainly can fake key events from DumpRenderTree.
>
I am not sure how to auto-test this, and seems that there is no test for PopupMenuChromium.cpp.
Let me look into it further.

Comment on attachment 30687[details]
patch (version 2)
I think this should be separated out into a static function. Then the code becomes:
if (isCharacterForPrefixString(event))
m_lastCharTime = now;
(Or some other equally suitable or better name)
And the long comment can be inside the static function, as well as the #if usage.
This also needs a test case. Even if it's just a manual test in WebCore/manual-tests/
I would expect there is a way to test this using eventSender though, so you could make an automated test.

> This also needs a test case. Even if it's just a manual test in
> WebCore/manual-tests/
>
> I would expect there is a way to test this using eventSender though, so you
> could make an automated test.
I do not believe there is a way to fully test this with an automated test. You might be able to fake a click on the select control, but then you need to fake mouse move and click events to select something. I spent a great deal of time trying this last week to fix this bug: https://bugs.webkit.org/show_bug.cgi?id=25904
In the end, I created a manual test that demonstrates the fix.

Created attachment 30845[details]
patch w/ manual test
The patched function PopListBox::typeAheadFind() is only called through WebWidgetImpl::HandleInputEvent(), never through WebviewImpl::HandleInputEvent().
If eventSender.keyDown() send events to webview, not webwidget, that means we can not use it to auto-test the patch.
Or maybe I misunderstood anything?

(In reply to comment #12)
> (From update of attachment 30970[details] [review])
> Style:
> +static bool isCharacterTypeEvent(const PlatformKeyboardEvent& event) {
>
>
> I'm confused why PopupListBox is in this file at all. Generally WebKit has
> one-class-per-file.
I do not know why it is there originally. Maybe someone more familiar with the file history knows it better.
>
> Why doesn't WebKitWin need this same code? Or does it already have it?
>
WebKitWin also has a similar function: SelectElement::typeAheadFind(), but it is called only when the event type is Char type, that is why it does not need to check inside the method.
The caller is: SelectElement::defaultEventHandler(), in which, it checks whether isPrintableChar(keyboardEvent->charCode()) is true before calling typeAheadFind().
KeyboardEvent::charCode() returns character code for keypress, 0 for keydown and keyup.
PopupListBox::typeAheadFind() is called by PopupListBox::handleKeyEvent(), which handles all the platform keyboard events (keyup, keydown, char, rawkeydown) from WebWidgetImpl::HandleInputEvent(), and differentiate Char type event is needed inside typeAheadFind().