[webkit-reviews] review denied: [Bug 25899] [Chromium] PC Chromium: keyboard selection in Hebrew select element doesn't work : [Attachment 30687] patch (version 2)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 1 12:03:18 PDT 2009


Eric Seidel <eric at webkit.org> has denied Xiaomei Ji <xji at chromium.org>'s
request for review:
Bug 25899: [Chromium] PC Chromium: keyboard selection in Hebrew select element
doesn't work
https://bugs.webkit.org/show_bug.cgi?id=25899

Attachment 30687: patch (version 2)
https://bugs.webkit.org/attachment.cgi?id=30687&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.


More information about the webkit-reviews mailing list