WebCore.windowsKeyCodeForKeyEvent() in KeyCodeConversionAndroid.cpp converts Android keycode to WebKit keycode. This file imports the Android keycode definition from <android/keycodes.h>. Some keycodes triggered by normal QWERTY keyboards are not defined in <android/keycodes.h>. A key event is mapped to WebKit keycode zero if its Android keycode is not defined and handled here. To handle Android keycode not defined in <android/keycodes.h>, KeyCodeConversionAndroid.cpp currently defines and handles two Android keycodes locally, AKEYCODE_MEDIA_PAUSE and AKEYCODE_VOLUME_MORE. The following keycodes could be handled in the same way: AKEYCODE_ESCAPE AKEYCODE_FORWARD_DEL AKEYCODE_CTRL_LEFT AKEYCODE_CTRL_RIGHT AKEYCODE_CAPS_LOCK AKEYCODE_SCROLL_LOCK AKEYCODE_META_LEFT AKEYCODE_META_RIGHT AKEYCODE_BREAK AKEYCODE_INSERT AKEYCODE_MEDIA_PLAY AKEYCODE_F1 AKEYCODE_F2 AKEYCODE_F3 AKEYCODE_F4 AKEYCODE_F5 AKEYCODE_F6 AKEYCODE_F7 AKEYCODE_F8 AKEYCODE_F9 AKEYCODE_F10 AKEYCODE_F11 AKEYCODE_F12 AKEYCODE_NUM_LOCK AKEYCODE_NUMPAD_0 AKEYCODE_NUMPAD_1 AKEYCODE_NUMPAD_2 AKEYCODE_NUMPAD_3 AKEYCODE_NUMPAD_4 AKEYCODE_NUMPAD_5 AKEYCODE_NUMPAD_6 AKEYCODE_NUMPAD_7 AKEYCODE_NUMPAD_8 AKEYCODE_NUMPAD_9 AKEYCODE_NUMPAD_DIVIDE AKEYCODE_NUMPAD_MULTIPLY AKEYCODE_NUMPAD_SUBTRACT AKEYCODE_NUMPAD_ADD AKEYCODE_NUMPAD_DOT AKEYCODE_CHANNEL_UP AKEYCODE_CHANNEL_DOWN
Created attachment 133387 [details] Patch that adds Android keycodes
Created attachment 133396 [details] Patch that adds Android keycodes
Did you mean to mark this patch for review?
Comment on attachment 133396 [details] Patch that adds Android keycodes View in context: https://bugs.webkit.org/attachment.cgi?id=133396&action=review Thanks! I've got some questions/comments on the patch (following up on the patch failing to apply in the e-mail thread). > WebKit/Source/WebCore/ChangeLog:3 > + Add Android keycodes nit: Since this is Chromium-specific code, we usually prepend the title with [Chromium]. > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:36 > +namespace { The usage of anonymous namespaces within WebKit is not really preferred (though not completely disallowed either). What is the benefit of putting these values in one? It's going to break during a future NDK update either way. > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:238 > +#if defined(GTV) What is the reason that we want to keep this specific to GTV?
(In reply to comment #4) > (From update of attachment 133396 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=133396&action=review > > Thanks! I've got some questions/comments on the patch (following up on the patch failing to apply in the e-mail thread). > > > WebKit/Source/WebCore/ChangeLog:3 > > + Add Android keycodes > > nit: Since this is Chromium-specific code, we usually prepend the title with [Chromium]. I will change the title, if this is possible. > > > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:36 > > +namespace { > > The usage of anonymous namespaces within WebKit is not really preferred (though not completely disallowed either). What is the benefit of putting these values in one? It's going to break during a future NDK update either way. The intention of the anonymous namespace is to limit the visibility of the enum to this file. I will remove it. > > > WebKit/Source/WebCore/platform/chromium/KeyCodeConversionAndroid.cpp:238 > > +#if defined(GTV) > > What is the reason that we want to keep this specific to GTV? The guarded part uses GTV specific VKEY_OEM_103 and VKEY_OEM_104 defined elsewhere. I will take this chance to add the definition of these two codes, then this is not GTV specific anymore.
Created attachment 133850 [details] Patch that adds Android keycodes New patch that includes recommendation from Peter's review.
Created attachment 133906 [details] Patch
Comment on attachment 133906 [details] Patch Thanks for the patch!
Comment on attachment 133906 [details] Patch Clearing flags on attachment: 133906 Committed r112174: <http://trac.webkit.org/changeset/112174>
All reviewed patches have been landed. Closing bug.