Bug 149584 – Implement KeyboardEvent.code from the UI Event spec

• Status: RESOLVED
• Resolution: FIXED
• Component: UI Events

[Chris Rebert]

WebKit does not yet implement the KeyboardEvent.code property from the W3C UI Events Specification. See:
* https://w3c.github.io/uievents/#widl-KeyboardEvent-code
* https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/code

[PhistucK]

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

[Chris Dumez]

*** Bug 162853 has been marked as a duplicate of this bug. ***

[Chris Dumez]

Spec for code:
https://www.w3.org/TR/uievents-code/

[PhistucK]

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

[Chris Dumez]

Created attachment 290536 [details]
Early WIP patch

[WebKit Commit Bot]

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.

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Manual test: http://w3c-test.org/uievents/keyboard/key-101en-us-manual.html

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Created attachment 290553 [details]
Patch

[Chris Dumez]

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

[Build Bot]

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

[Build Bot]

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

[Chris Dumez]

Created attachment 290566 [details]
Patch

[Darin Adler]

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.

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 290677 [details]
Patch

[Chris Dumez]

Created attachment 290684 [details]
Patch

[Chris Dumez]

Created attachment 290685 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 290685 [details]
Patch

Clearing flags on attachment: 290685

Committed r206803: <http://trac.webkit.org/changeset/206803>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

We are making history.

[Lucas Forschler]

Mass move bugs into the DOM component.