Bug 28247

Summary: [Chromium] Numpad keys and a manu key don't work in render view
Product: WebKit Reporter: Hironori Bono <hbono>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ahilliard, commit-queue, estade, evan, mbk, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Add numpad key mappings (based on mbk's code snippet)
fishd: review-
Add numpad key mappings (fixed the email address in ChangeLog!)
none
Numpad key fix for gtk with automated test
none
Numpad key fix for gtk with automated test
none
Numpad key fix for gtk with automated test (style fixed) none

Description Hironori Bono 2009-08-12 22:49:01 PDT
(Copied from <http://crbug.com/19106>.)

Chrome Version       : 3.0.197.11-r22553
OS + version : Debian Lenny
CPU architecture (32-bit / 64-bit): 32
window manager : KDE
URLs (if applicable) :
Behavior in Firefox 3.x (if applicable): Numpad arrow keys work
Behavior in Chrome for Windows (optional): Numpad arrow keys work

Goto to any page and just try to use the numpad keys to navigate (with the NumLock off). They don't work. They DO however work in the address bar. 

The numpad arrows work in all other applications, including GTK ones (e.g. Firefox).

Please provide any additional information below. Attach a screenshot and backtrace if possible.

WebCore::windowsKeyCodeForKeyEvent() (in "WebCore/platform/chromium/KeyCodeConversionGtk.cpp") doesn't have mappings for many numpad keys (e.g. GDK_KP_Up, GDK_KP_Down, etc.) and cannot send numpad-key events to a web page. Neither does "WebCore/platform/gtk/KeyEventGdk.cpp" have them.

Also, they map GDK_Menu to VKEY_MENU, which is a virtual key-code for alt keys. To improve compatibility with Windows, it is better to map GDK_Menu to VKEY_APPS, which is a virtual key-code for a menu key.
Comment 1 mbk 2009-12-10 11:31:47 PST
I can confirm the problem on Ubuntu Karmic Koala (32bit PAE) for chromium version 4.0.262.0 (33602).  Load any page and try to use the numpad arrows/pgdn/pgup/home/end and nothing happens.

I made changes to KeyCodeConversionGtk.cpp and it fixed the problem for my system.  I've put the diff below.

Index: KeyCodeConversionGtk.cpp
===================================================================
--- KeyCodeConversionGtk.cpp	(revision 51589)
+++ KeyCodeConversionGtk.cpp	(working copy)
@@ -72,6 +72,24 @@
     case GDK_KP_Divide:
         return VKEY_DIVIDE; // (6F) Divide key
 
+    case GDK_KP_Page_Up:
+	 return VKEY_PRIOR; // (21) PAGE UP key
+    case GDK_KP_Page_Down:
+	 return VKEY_NEXT; // (22) PAGE DOWN key
+    case GDK_KP_End:
+        return VKEY_END; // (23) END key
+    case GDK_KP_Home:
+        return VKEY_HOME; // (24) HOME key
+    case GDK_KP_Left:
+        return VKEY_LEFT; // (25) LEFT ARROW key
+    case GDK_KP_Up:
+        return VKEY_UP; // (26) UP ARROW key
+    case GDK_KP_Right:
+        return VKEY_RIGHT; // (27) RIGHT ARROW key
+    case GDK_KP_Down:
+        return VKEY_DOWN; // (28) DOWN ARROW key
+ 	 
+
     case GDK_BackSpace:
         return VKEY_BACK; // (08) BACKSPACE key
     case GDK_ISO_Left_Tab:
Comment 2 Kinuko Yasuda 2009-12-16 02:25:09 PST
Created attachment 44960 [details]
Add numpad key mappings (based on mbk's code snippet)
Comment 3 WebKit Review Bot 2009-12-16 02:28:12 PST
style-queue ran check-webkit-style on attachment 44960 [details] without any errors.
Comment 4 Darin Fisher (:fishd, Google) 2009-12-16 10:56:00 PST
Comment on attachment 44960 [details]
Add numpad key mappings (based on mbk's code snippet)

> +2009-12-15  Kinuko Yasuda  <kinuko@chromium.com>

^^^ this should be @chromium.org, right? :-)

otherwise, r=me  (r- for this one change)


was there no way to use the eventSender to write an automated layout test?
Comment 5 Hironori Bono 2009-12-16 17:49:40 PST
(In reply to comment #4)

> was there no way to use the eventSender to write an automated layout test?

Sorry for the lack of detailed explanation about this issue.
As you know, when we call eventSender.keyDown("0", modifiers), eventSender sends a key event for VKEY_0, not VKEY_NUMPAD_0.
To be able to send a numpad-key event through an eventSender.keyDown() call, I would like to add another parameter "location" to EventSendingController.mm (and also our event_sending_controller.cc). This new "location" parameter is the same as the "locationArg" parameter of initKeyboardEvent() and declares the key location code introduced in the "DOM 3 Event" specification (*1). The possible values for the parameters are listed below. This is WebKit Bug 32602 (*2).

  DOM_KEY_LOCATION_STANDARD      = 0x00;
  DOM_KEY_LOCATION_LEFT          = 0x01;
  DOM_KEY_LOCATION_RIGHT         = 0x02;
  DOM_KEY_LOCATION_NUMPAD        = 0x03;
  DOM_KEY_LOCATION_MOBILE        = 0x04;
  DOM_KEY_LOCATION_JOYSTICK      = 0x05;

Unfortunately, I have been busy these days and cannot afford to work for this Bug 32602 now. After a talk with estade about this issue, I decided to let Kinuko-san land this fix with a manual test before my change for WebKit Bug 32602, and land another change for WebKit Bug 32602 to replace the manual test with an automated layout test.

(*1) <http://www.w3.org/TR/DOM-Level-3-Events/>
(*2) <https://bugs.webkit.org/show_bug.cgi?id=32602>

Regards,

Hironori Bono
Comment 6 Kinuko Yasuda 2009-12-16 21:32:31 PST
Created attachment 45036 [details]
Add numpad key mappings (fixed the email address in ChangeLog!)
Comment 7 WebKit Review Bot 2009-12-16 21:34:31 PST
style-queue ran check-webkit-style on attachment 45036 [details] without any errors.
Comment 8 Eric Seidel (no email) 2009-12-22 16:22:15 PST
Comment on attachment 45036 [details]
Add numpad key mappings (fixed the email address in ChangeLog!)

I don't see why the rush.  We should have a automated test for this.
Comment 9 Kinuko Yasuda 2010-01-05 01:11:57 PST
Created attachment 45870 [details]
Numpad key fix for gtk with automated test
Comment 10 WebKit Review Bot 2010-01-05 01:15:53 PST
Attachment 45870 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:464:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1
Comment 11 Kinuko Yasuda 2010-01-05 01:15:54 PST
Created attachment 45871 [details]
Numpad key fix for gtk with automated test
Comment 12 WebKit Review Bot 2010-01-05 01:16:30 PST
Attachment 45871 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKitTools/DumpRenderTree/gtk/EventSender.cpp:464:  One line control clauses should not use braces.  [whitespace/braces] [4]
Total errors found: 1
Comment 13 Kinuko Yasuda 2010-01-05 01:18:34 PST
Created attachment 45873 [details]
Numpad key fix for gtk with automated test (style fixed)
Comment 14 WebKit Review Bot 2010-01-05 01:23:00 PST
style-queue ran check-webkit-style on attachment 45873 [details] without any errors.
Comment 15 Eric Seidel (no email) 2010-01-26 14:27:13 PST
Comment on attachment 45873 [details]
Numpad key fix for gtk with automated test (style fixed)

LGTM.
Comment 16 WebKit Commit Bot 2010-01-27 11:01:45 PST
Comment on attachment 45873 [details]
Numpad key fix for gtk with automated test (style fixed)

Clearing flags on attachment: 45873

Committed r53942: <http://trac.webkit.org/changeset/53942>
Comment 17 WebKit Commit Bot 2010-01-27 11:01:53 PST
All reviewed patches have been landed.  Closing bug.