(Okay, I didn't check them all. But the ones I did check were wrong. Safari gets them right.) E.g., it should send VK_MULTIPLY (keycode 106) for the numpad *. To check easily, just do something like document.body.innerHTML = '<input onkeydown="console.log(window.event);">'; from the dev console (from about:blank, say), and type in the input box. This affects Pepper plugins (and in particular Pepper Flash).
I should have pointed out that Safari gets it right.
Working on this.
Agh, everyone gets * correct. Recent Safari and Chrome get - (probably / also, dunno about +, .) wrong. Old Safari (5.1.7, on 10.6) gets - (and probably the others) right.
I confirmed that on Windows IE9 sends the numpad-specific keycodes for /, *, -, +, and . (when numlock is down). Chrome gets /, -, +, and . wrong. (Old Safari gets these right.)
Created attachment 168604 [details] Patch
For reference, here are the windows virtual key codes that changed decimal 110 -> 190 plus 107 -> 187 subtract 109 -> 189 divide 111 -> 191 This regressed in http://trac.webkit.org/changeset/57951. That change fixes key codes when the user has a different keyboard mapping (dvorak, azerty, etc..). I don't think my change regress the original because those keyboard mappings don't modify the num pad keys.
Please don't use [chromium] prefix on any bugs that touch cross-platform code. Such a prefix is a sign for non-chromium filks that they can ignore the bug.
Comment on attachment 168604 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=168604&action=review The change makes sense. Please add WebKitTestRunner and DumpRenderTree tests - there are similar ones already. > Source/WebKit/chromium/ChangeLog:8 > + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. Too many spaces here.
(In reply to comment #8) > (From update of attachment 168604 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=168604&action=review > > The change makes sense. > > Please add WebKitTestRunner and DumpRenderTree tests - there are similar ones already. Hi Alexey, would you mind pointing out the existing tests? Sorry, I'm not very familiar with WebKit. Thanks > > > Source/WebKit/chromium/ChangeLog:8 > > + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. > > Too many spaces here.
See e.g. platform/mac/fast/events/non-roman-key-code.html. Depending on how widely you implement DumpRenderTree/WebKitTestRunner support for the additional key, the test can be either in platform/mac or just in fast/events.
Created attachment 169038 [details] Patch
Added a layout test for platform/mac. Please take another look.
Alexey, Ping!
Comment on attachment 169038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169038&action=review > Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:47 > +struct KeyMappingEntry > +{ Opening brace goes in the same line as struct. > Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:120 > + for (size_t i = 0; i < arraysize(table); ++i) > + { Opening brace goes on the same line as 'for'. > Tools/DumpRenderTree/mac/EventSendingController.mm:61 > +struct KeyMappingEntry > +{ Opening brace goes on the same line as struct. > Tools/DumpRenderTree/mac/EventSendingController.mm:739 > + for (unsigned i = 0; i < WTF_ARRAY_LENGTH(table); i++) { Nit: ++i
Comment on attachment 169038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169038&action=review Looks good to me. > Source/WebCore/ChangeLog:8 > + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. This line is long, I think that it needs to be wrapped. > LayoutTests/platform/mac/fast/events/numpad-keycode-mapping-expected.txt:9 > +PASS lastKeyboardEvent.keyCode is 110 > +PASS lastKeyboardEvent.keyCode is 106 > +PASS lastKeyboardEvent.keyCode is 107 > +PASS lastKeyboardEvent.keyCode is 12 Ideally, shouldBe test output should be self-descriptive. For example, if you change the tests to something like shouldBe("getKeyCode('.', 0x6E)", "110"), the lines would not all look the same. > LayoutTests/platform/mac/fast/events/numpad-keycode-mapping.html:19 > + eventSender.keyDown(keyName, 0, 3); Which one of these constants is keypad key location? Could you please define a named constant for it for clarity?
Created attachment 170207 [details] Patch
Comment on attachment 169038 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169038&action=review Addressed review comments. Please take another look. >> Source/WebCore/ChangeLog:8 >> + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. > > This line is long, I think that it needs to be wrapped. Done. >> Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:47 >> +{ > > Opening brace goes in the same line as struct. Done. >> Source/WebKit/chromium/tests/WebInputEventFactoryTestMac.mm:120 >> + { > > Opening brace goes on the same line as 'for'. Done. >> Tools/DumpRenderTree/mac/EventSendingController.mm:61 >> +{ > > Opening brace goes on the same line as struct. Done. >> Tools/DumpRenderTree/mac/EventSendingController.mm:739 >> + for (unsigned i = 0; i < WTF_ARRAY_LENGTH(table); i++) { > > Nit: ++i Done. >> LayoutTests/platform/mac/fast/events/numpad-keycode-mapping-expected.txt:9 >> +PASS lastKeyboardEvent.keyCode is 12 > > Ideally, shouldBe test output should be self-descriptive. For example, if you change the tests to something like shouldBe("getKeyCode('.', 0x6E)", "110"), the lines would not all look the same. Done. >> LayoutTests/platform/mac/fast/events/numpad-keycode-mapping.html:19 >> + eventSender.keyDown(keyName, 0, 3); > > Which one of these constants is keypad key location? Could you please define a named constant for it for clarity? Done.
Comment on attachment 170207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170207&action=review > Source/WebKit/chromium/ChangeLog:8 > + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. This line is equally long. > LayoutTests/platform/mac/fast/events/numpad-keycode-mapping.html:42 > + finishJSTest() Didn't notice this at first. This is not an async test, and shouldn't need finishJSTest.
Created attachment 170224 [details] Patch for landing
Comment on attachment 170207 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=170207&action=review >> Source/WebKit/chromium/ChangeLog:8 >> + In r57951 we switched to mapping keys by character code. This regressed the numpad keys which no longer match the Windows virtual key codes. This CL fixes this by never mapping numpad keys by character code. > > This line is equally long. Done. Oops, fixed. >> LayoutTests/platform/mac/fast/events/numpad-keycode-mapping.html:42 >> + finishJSTest() > > Didn't notice this at first. This is not an async test, and shouldn't need finishJSTest. Done.
Comment on attachment 170224 [details] Patch for landing Clearing flags on attachment: 170224 Committed r132411: <http://trac.webkit.org/changeset/132411>
All reviewed patches have been landed. Closing bug.
Did you forget to update WebKitTestRunner (WebKit2)?
We’re seeing failure like this one: http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r138643%20(4408)/platform/mac/fast/events/numpad-keycode-mapping-pretty-diff.html --- /Volumes/Data/slave/mountainlion-release-tests-wk2/build/layout-test-results/platform/mac/fast/events/numpad-keycode-mapping-expected.txt +++ /Volumes/Data/slave/mountainlion-release-tests-wk2/build/layout-test-results/platform/mac/fast/events/numpad-keycode-mapping-actual.txt @@ -3,24 +3,24 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS getKeyCode('.') is 0x6E -PASS getKeyCode('*') is 0x6A -PASS getKeyCode('+') is 0x6B -PASS getKeyCode('clear') is 0x0C -PASS getKeyCode('/') is 0x6F -PASS getKeyCode('enter') is 0x0D -PASS getKeyCode('-') is 0x6D -PASS getKeyCode('=') is 0xBB -PASS getKeyCode('0') is 0x60 -PASS getKeyCode('1') is 0x61 -PASS getKeyCode('2') is 0x62 -PASS getKeyCode('3') is 0x63 -PASS getKeyCode('4') is 0x64 -PASS getKeyCode('5') is 0x65 -PASS getKeyCode('6') is 0x66 -PASS getKeyCode('7') is 0x67 -PASS getKeyCode('8') is 0x68 -PASS getKeyCode('9') is 0x69 +FAIL getKeyCode('.') should be 110. Was 65. +FAIL getKeyCode('*') should be 106. Was 65. +FAIL getKeyCode('+') should be 107. Was 65. +FAIL getKeyCode('clear') should be 12. Was 65. +FAIL getKeyCode('/') should be 111. Was 65. +FAIL getKeyCode('enter') should be 13. Was 65. +FAIL getKeyCode('-') should be 109. Was 65. +FAIL getKeyCode('=') should be 187. Was 65. +FAIL getKeyCode('0') should be 96. Was 48. +FAIL getKeyCode('1') should be 97. Was 65. +FAIL getKeyCode('2') should be 98. Was 65. +FAIL getKeyCode('3') should be 99. Was 65. +FAIL getKeyCode('4') should be 100. Was 65. +FAIL getKeyCode('5') should be 101. Was 53. +FAIL getKeyCode('6') should be 102. Was 65. +FAIL getKeyCode('7') should be 103. Was 55. +FAIL getKeyCode('8') should be 104. Was 65. +FAIL getKeyCode('9') should be 105. Was 57. PASS successfullyParsed is true TEST COMPLETE
> Did you forget to update WebKitTestRunner (WebKit2)? Hi Ryosuke. Looks like I did forget, sorry about that. It looks like I just have to port my change from EventSendingController.mm to Tools/WebKitTestRunner/mac/EventSenderProxy.mm Working on that right now. Thanks.
(In reply to comment #25) > > Did you forget to update WebKitTestRunner (WebKit2)? > > Hi Ryosuke. Looks like I did forget, sorry about that. > > It looks like I just have to port my change from EventSendingController.mm to Tools/WebKitTestRunner/mac/EventSenderProxy.mm > > Working on that right now. Thanks. Thanks. Note that I've filed https://bugs.webkit.org/show_bug.cgi?id=105958.