Bug 119326 – Add support for KeyboardEvent.location attribute

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Add support for KeyboardEvent.location attribute as per the latest specification:
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#events-KeyboardEvent

IE10, Firefox 22 and recently Blink all support KeyboardEvent.location already.

I am planning to keep the outdated KeyboardEvent.keyLocation so as to not break backward compatibility.

Corresponding Blink revision:
https://src.chromium.org/viewvc/blink?revision=155170&view=revision

[Chris Dumez]

Created attachment 207818 [details]
WIP Patch

[WebKit Commit Bot]

Attachment 207818 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl']" exit_code: 1
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 17 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 207818 [details]
WIP Patch

Attachment 207818 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1289691

[Build Bot]

Comment on attachment 207818 [details]
WIP Patch

Attachment 207818 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1292601

[Chris Dumez]

Created attachment 207827 [details]
WIP patch

Attempt to fix ObjC bindings.

[WebKit Commit Bot]

Please wait for approval from [e-mail] (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.

[WebKit Commit Bot]

Attachment 207827 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 207827 [details]
WIP patch

Attachment 207827 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1300540

[Chris Dumez]

Created attachment 207830 [details]
WIP patch

Another attempt to fix ObjC bindings

[WebKit Commit Bot]

Attachment 207830 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 207830 [details]
WIP patch

Attachment 207830 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1291642

[Chris Dumez]

Created attachment 207834 [details]
WIP patch

[WebKit Commit Bot]

Attachment 207834 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 207834 [details]
WIP patch

Attachment 207834 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1300557

[Chris Dumez]

Created attachment 207840 [details]
WIP patch

[WebKit Commit Bot]

Attachment 207840 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl', u'Tools/DumpRenderTree/mac/EventSendingController.mm']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 207840 [details]
WIP patch

Attachment 207840 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1296687

[Chris Dumez]

Created attachment 207845 [details]
WIP patch

[WebKit Commit Bot]

Attachment 207845 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl', u'Tools/DumpRenderTree/mac/EventSendingController.mm']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 207849 [details]
Patch

[WebKit Commit Bot]

Attachment 207849 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl', u'Tools/ChangeLog', u'Tools/DumpRenderTree/mac/EventSendingController.mm']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

mac-ews is finally green \o/

Could someone from Apple (xenon?) please carefully review my ObjC bindings changes? It builds but there is a good chance it is wrong since I have no clue what I'm doing there. Help would very much be appreciated.

[Alexey Proskuryakov]

This patch appears to simply alias keyLocation to location. But is this actually correct behavior? I was assuming that the property was renamed in spec because new one's values were not the same.

[Chris Dumez]

(In reply to comment #23)
> This patch appears to simply alias keyLocation to location. But is this actually correct behavior? I was assuming that the property was renamed in spec because new one's values were not the same.

This is correct, this patch is doing an alias from keyLocation to location. Unlike some other attributes that were also renamed in KeyboardEvent, this one kept the same meaning and values.

Old:
http://www.w3.org/TR/2007/WD-DOM-Level-3-Events-20071221/events.html#Events-KeyboardEvent-keylocation
http://www.w3.org/TR/2007/WD-DOM-Level-3-Events-20071221/events.html#ID-KeyboardEvent-KeyLocationCode

New:
http://www.w3.org/TR/DOM-Level-3-Events/#events-KeyboardEvent-location
http://www.w3.org/TR/DOM-Level-3-Events/#events-ID-KeyboardEvent-KeyLocationCode

One difference is that the location supports a few more values (MOBILE and JOYSTICK). We currently do not support those 2 values and neither does Blink.

Note that the DOM_KEY_LOCATION_* constants are missing from our KeyboardEvent interface as well. I am planning to add them in a follow up patch.

[Chris Dumez]

Any feedback on this patch?

[Benjamin Poulain]

Comment on attachment 207849 [details]
Patch

Looks reasonable to me.

r- because you kill all coverage of keyLocation by renaming everything to location. IMHO, most of these tests should cover both location and the deprecated keyLocation.

[Chris Dumez]

Comment on attachment 207849 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207849&action=review

> LayoutTests/fast/events/constructors/keyboard-event-constructor.html:80
> +// test for deprecated 'keyLocation'.

I kept tests for the deprecated keyLocation here.

[Chris Dumez]

Created attachment 208367 [details]
Patch

[WebKit Commit Bot]

Attachment 208367 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/events/arrow-keys-on-body-expected.txt', u'LayoutTests/fast/events/arrow-keys-on-body.html', u'LayoutTests/fast/events/constructors/keyboard-event-constructor-expected.txt', u'LayoutTests/fast/events/constructors/keyboard-event-constructor.html', u'LayoutTests/fast/events/init-events-expected.txt', u'LayoutTests/fast/events/js-keyboard-event-creation.html', u'LayoutTests/fast/events/keydown-leftright-keys-expected.txt', u'LayoutTests/fast/events/keydown-leftright-keys.html', u'LayoutTests/fast/events/keydown-numpad-keys-expected.txt', u'LayoutTests/fast/events/script-tests/init-events.js', u'LayoutTests/fast/events/script-tests/keydown-numpad-keys.js', u'LayoutTests/platform/mac/fast/events/objc-keyboard-event-creation.html', u'LayoutTests/platform/win/fast/events/keyLocation-numpad.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/objc/PublicDOMInterfaces.h', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/dom/KeyboardEvent.cpp', u'Source/WebCore/dom/KeyboardEvent.h', u'Source/WebCore/dom/KeyboardEvent.idl', u'Source/WebKit/win/ChangeLog', u'Source/WebKit/win/DOMEventsClasses.cpp', u'Source/WebKit/win/DOMEventsClasses.h', u'Source/WebKit/win/Interfaces/DOMEvents.idl', u'Tools/ChangeLog', u'Tools/DumpRenderTree/mac/EventSendingController.mm']" exit_code: 1
Source/WebKit/win/DOMEventsClasses.h:407:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/dom/KeyboardEvent.cpp:133:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 2 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

Comment on attachment 208367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208367&action=review

> LayoutTests/fast/events/constructors/keyboard-event-constructor.html:74
> +["location", "keyLocation"].forEach(function (propertyName) {

smart

[Ryosuke Niwa]

Comment on attachment 208367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208367&action=review

> Source/WebCore/ChangeLog:16
> +        No new tests, covered by existing tests.

That doesn't make much sense. At minimum, we need tests to ensure location and keyLocation both exist and both return the same value.

[Chris Dumez]

Comment on attachment 208367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=208367&action=review

>> Source/WebCore/ChangeLog:16
>> +        No new tests, covered by existing tests.
> 
> That doesn't make much sense. At minimum, we need tests to ensure location and keyLocation both exist and both return the same value.

Yes, I updated the existing tests to do that so it is covered.

[WebKit Commit Bot]

Comment on attachment 208367 [details]
Patch

Clearing flags on attachment: 208367

Committed r153905: <http://trac.webkit.org/changeset/153905>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.