119326 – Add support for KeyboardEvent.location attribute

Bug 119326 - Add support for KeyboardEvent.location attribute

Summary: Add support for KeyboardEvent.location attribute

Status:	RESOLVED FIXED

Alias:	None

Product:	WebKit
Classification:	Unclassified
Component:	WebCore Misc. (show other bugs)
Version:	528+ (Nightly build)
Hardware:	Unspecified Unspecified

Importance:	P2 Normal
Assignee:	Chris Dumez

URL:	http://dev.w3.org/2006/webapi/DOM-Lev...
Keywords:	BlinkMergeCandidate, WebExposed

Depends on:
Blocks:	119341
	Show dependency tree / graph

Reported:	2013-07-31 00:14 PDT by Chris Dumez
Modified:	2013-08-09 11:55 PDT (History)
CC List:	11 users (show)

See Also:

Attachments
WIP Patch (41.32 KB, patch) 2013-07-31 01:24 PDT, Chris Dumez	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
WIP patch (43.68 KB, patch) 2013-07-31 03:32 PDT, Chris Dumez	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
WIP patch (45.30 KB, patch) 2013-07-31 04:06 PDT, Chris Dumez	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
WIP patch (45.33 KB, patch) 2013-07-31 04:32 PDT, Chris Dumez	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
WIP patch (51.33 KB, patch) 2013-07-31 05:23 PDT, Chris Dumez	buildbot: commit-queue-	Details \| Formatted Diff \| Diff
WIP patch (53.17 KB, patch) 2013-07-31 05:59 PDT, Chris Dumez	no flags	Details \| Formatted Diff \| Diff
Patch (59.42 KB, patch) 2013-07-31 06:34 PDT, Chris Dumez	no flags	Details \| Formatted Diff \| Diff
Patch (58.94 KB, patch) 2013-08-08 14:00 PDT, Chris Dumez	no flags	Details \| Formatted Diff \| Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Chris Dumez 2013-07-31 00:14:46 PDT

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

Comment 1 Chris Dumez 2013-07-31 01:24:09 PDT

Created attachment 207818 [details]
WIP Patch

Comment 2 WebKit Commit Bot 2013-07-31 01:26:59 PDT

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.

Comment 3 Build Bot 2013-07-31 01:46:03 PDT

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

Comment 4 Build Bot 2013-07-31 03:01:21 PDT

Comment on attachment 207818 [details]
WIP Patch

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

Comment 5 Chris Dumez 2013-07-31 03:32:58 PDT

Created attachment 207827 [details]
WIP patch

Attempt to fix ObjC bindings.

Comment 6 WebKit Commit Bot 2013-07-31 03:34:30 PDT

Please wait for approval from timothy@apple.com (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.

Comment 7 WebKit Commit Bot 2013-07-31 03:34:42 PDT

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.

Comment 8 Build Bot 2013-07-31 04:00:36 PDT

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

Comment 9 Chris Dumez 2013-07-31 04:06:39 PDT

Created attachment 207830 [details]
WIP patch

Another attempt to fix ObjC bindings

Comment 10 WebKit Commit Bot 2013-07-31 04:08:46 PDT

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.

Comment 11 Build Bot 2013-07-31 04:20:40 PDT

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

Comment 12 Chris Dumez 2013-07-31 04:32:49 PDT

Created attachment 207834 [details]
WIP patch

Comment 13 WebKit Commit Bot 2013-07-31 04:34:20 PDT

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.

Comment 14 Build Bot 2013-07-31 05:11:27 PDT

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

Comment 15 Chris Dumez 2013-07-31 05:23:11 PDT

Created attachment 207840 [details]
WIP patch

Comment 16 WebKit Commit Bot 2013-07-31 05:24:36 PDT

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.

Comment 17 Build Bot 2013-07-31 05:37:01 PDT

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

Comment 18 Chris Dumez 2013-07-31 05:59:27 PDT

Created attachment 207845 [details]
WIP patch

Comment 19 WebKit Commit Bot 2013-07-31 06:01:50 PDT

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.

Comment 20 Chris Dumez 2013-07-31 06:34:11 PDT

Created attachment 207849 [details]
Patch

Comment 21 WebKit Commit Bot 2013-07-31 06:35:36 PDT

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.

Comment 22 Chris Dumez 2013-07-31 06:37:45 PDT

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.

Comment 23 Alexey Proskuryakov 2013-07-31 09:54:07 PDT

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.

Comment 24 Chris Dumez 2013-07-31 10:03:29 PDT

(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.

Comment 25 Chris Dumez 2013-08-06 23:34:53 PDT

Any feedback on this patch?

Comment 26 Benjamin Poulain 2013-08-07 13:51:35 PDT

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.

Comment 27 Chris Dumez 2013-08-07 22:38:00 PDT

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.

Comment 28 Chris Dumez 2013-08-08 14:00:09 PDT

Created attachment 208367 [details]
Patch

Comment 29 WebKit Commit Bot 2013-08-08 14:02:39 PDT

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.

Comment 30 Benjamin Poulain 2013-08-08 14:14:13 PDT

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

Comment 31 Ryosuke Niwa 2013-08-09 10:02:52 PDT

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.

Comment 32 Chris Dumez 2013-08-09 10:17:07 PDT

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.

Comment 33 WebKit Commit Bot 2013-08-09 11:55:08 PDT

Comment on attachment 208367 [details]
Patch

Clearing flags on attachment: 208367

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

Comment 34 WebKit Commit Bot 2013-08-09 11:55:13 PDT

All reviewed patches have been landed.  Closing bug.