Summary: | Implement KeyboardEvent.code from the UI Event spec | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Rebert <webkit> | ||||||||||||||||||||||
Component: | UI Events | Assignee: | Chris Dumez <cdumez> | ||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||
Severity: | Normal | CC: | ap, benjamin, buildbot, cdumez, cmarcelo, commit-queue, darin, dbates, enrica, esprehn+autocc, kangil.han, kondapallykalyan, mikolaj.konarski, phistuck, rniwa, sam | ||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||
Hardware: | All | ||||||||||||||||||||||||
OS: | All | ||||||||||||||||||||||||
URL: | https://w3c.github.io/uievents/#dom-keyboardevent-code | ||||||||||||||||||||||||
Bug Depends on: | |||||||||||||||||||||||||
Bug Blocks: | 162852 | ||||||||||||||||||||||||
Attachments: |
|
Description
Chris Rebert
2015-09-27 00:17:19 PDT
Looks like the specification is stable now. Firefox shipped KeyboardEvent.key and KeyboardEvent.code. Internet Explorer 9+ shipped KeyboardEvent.key (though its implementation is not up to date with the current specification). Chrome will be shipping KeyboardEvent.code in Chrome 48 and KeyboardEvent.key should also follow soon after (or even in the same version). Perhaps this is a good time to reconsider implementing this. (Sorry for the spam on multiple tickets, but it's dire): Additionally, Chrome will soon drop keyIdentifier https://www.chromestatus.com/features/5316065118650368 so JS that uses keyIdentifier (because of webkit) will no longer work on Chrome, so it would be incredibly useful if webkit implemented the current standard https://w3c.github.io/uievents/#events-keyboardevents *** Bug 162853 has been marked as a duplicate of this bug. *** Spec for code: https://www.w3.org/TR/uievents-code/ This one is more up to date - https://w3c.github.io/uievents-code/ There were some changed since December - https://github.com/w3c/uievents-code/commits/gh-pages Created attachment 290536 [details]
Early WIP patch
Attachment 290536 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:395: Tab found; better to use spaces [whitespace/tab] [1]
Total errors found: 1 in 12 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 290536 [details] Early WIP patch Attachment 290536 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2214251 New failing tests: imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors.html Created attachment 290550 [details]
Archive of layout-test-results from ews114 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 290536 [details] Early WIP patch Attachment 290536 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2214274 New failing tests: imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors.html Created attachment 290551 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 290536 [details] Early WIP patch Attachment 290536 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2214340 New failing tests: imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors.html Created attachment 290552 [details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 290553 [details]
Patch
I also uploaded a PR to fix the web-platform-tests as they do not match exactly the latest specification: - https://github.com/w3c/web-platform-tests/pull/3864 Comment on attachment 290553 [details] Patch Attachment 290553 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2214683 New failing tests: imported/w3c/web-platform-tests/dom/events/Event-subclasses-constructors.html fast/events/keyboardevent-code.html fast/events/constructors/keyboard-event-constructor.html Created attachment 290562 [details]
Archive of layout-test-results from ews126 for ios-simulator-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.6
Created attachment 290566 [details]
Patch
Comment on attachment 290566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290566&action=review > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:293 > + /* Keys in the alphanumeric section. */ This use of /* */ seems a bit peculiar. > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:353 > + case kVK_Return: return ASCIILiteral("Enter"); // Labelled Return on Apple keyboards. Spelling error: labeled doesn’t have a double "l". Also there’s an extra space there. > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:376 > + // Insert: Not present on Apple keyboards. I don’t understand fully how that is relevant. What about third party keyboards not specifically designed for Mac? > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:387 > + case kVK_ANSI_KeypadClear: return ASCIILiteral("NumLock"); // On the Mac, the "NumLock" code should be used for the numpad Clear key. Comment says what should be done, but not why. > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:399 > + // NumpadBackspace: Found on the Microsoft Natural Keyboard. Can’t you plug one of those into a Mac? > Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:400 > + // NumpadClear: On the Mac, the numpad Clear key should always be encoded as "NumLock". Again, what but not why. Comment on attachment 290566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290566&action=review >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:293 >> + /* Keys in the alphanumeric section. */ > > This use of /* */ seems a bit peculiar. Trying to differentiate with codes that are in the specification but commented below, but OK. I'll fix. >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:353 >> + case kVK_Return: return ASCIILiteral("Enter"); // Labelled Return on Apple keyboards. > > Spelling error: labeled doesn’t have a double "l". Also there’s an extra space there. Copied from the spec, I'll fix. >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:376 >> + // Insert: Not present on Apple keyboards. > > I don’t understand fully how that is relevant. What about third party keyboards not specifically designed for Mac? Insert / Help have the same key code. We already map the code to "help" above. >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:387 >> + case kVK_ANSI_KeypadClear: return ASCIILiteral("NumLock"); // On the Mac, the "NumLock" code should be used for the numpad Clear key. > > Comment says what should be done, but not why. This is what the spec says. I do not know why. I am merely trying to match the specification and other browsers (verified on Chrome/Mac). >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:399 >> + // NumpadBackspace: Found on the Microsoft Natural Keyboard. > > Can’t you plug one of those into a Mac? I do not know. My guess is that you can but those extra keys will not necessarily work. What I know is that: - There is no corresponding constant in Events.h - Chrome/Mac does not map NumpadBackspace on Mac either (checked their code). - Mozilla does not list it here: https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code This comment applies for other special keys on the Microsoft natural keyboard as well. >> Source/WebCore/platform/mac/PlatformEventFactoryMac.mm:400 >> + // NumpadClear: On the Mac, the numpad Clear key should always be encoded as "NumLock". > > Again, what but not why. These comments are copy/pasted from the specification. I do not know why, I just know that this is what the specification says we should do. Created attachment 290677 [details]
Patch
Created attachment 290684 [details]
Patch
Created attachment 290685 [details]
Patch
Comment on attachment 290685 [details] Patch Clearing flags on attachment: 290685 Committed r206803: <http://trac.webkit.org/changeset/206803> All reviewed patches have been landed. Closing bug. We are making history. Mass move bugs into the DOM component. |