Last Comment Bug 936313 - Drop KeyboardEvent.DOM_LOCATION_MOBILE and KeyboardEvent.DOM_LOCATION_JOYSTICK of KeyboardEvent.location since they have been dropped from D3E spec
: Drop KeyboardEvent.DOM_LOCATION_MOBILE and KeyboardEvent.DOM_LOCATION_JOYSTIC...
Status: RESOLVED FIXED
: dev-doc-complete, site-compat
Product: Core
Classification: Components
Component: DOM: Events (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla38
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
:
Mentors:
Depends on: 1017383 865649
Blocks: 680829 166240 756504 1119609
  Show dependency treegraph
 
Reported: 2013-11-07 18:06 PST by Masayuki Nakano [:masayuki] (Mozilla Japan)
Modified: 2015-02-28 08:43 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK (20.20 KB, patch)
2015-01-22 09:19 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
no flags Details | Diff | Review
part.2 Compute DOM key location from code value on Android and Gonk (6.56 KB, patch)
2015-01-22 09:20 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
mwu.code: review+
cpeterson: review+
Details | Diff | Review
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK (25.20 KB, patch)
2015-01-23 01:52 PST, Masayuki Nakano [:masayuki] (Mozilla Japan)
bugs: review+
cpeterson: review+
bugs: superreview+
Details | Diff | Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-11-07 18:06:28 PST
https://dvcs.w3.org/hg/dom3events/raw-file/tip/html/DOM3-Events.html#events-keyboardevents

DOM_LOCATION_MOBILE and DOM_LOCATION_JOYSTICK have been dropped from D3E completely. Instead of them, use DOM_LOCATION_STANDARD if location of a key is not sure.
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2013-11-07 18:11:17 PST
Smaug:

Don't you mind we will drop them from nsIDOMKeyEvent and KeyboardEvent.webidl?
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2013-11-08 13:08:14 PST
I think that should be ok.
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2014-04-23 21:49:48 PDT
I think that we can fix this bug easier after bug 865649 (KeyboardEvent.code) is fixed.
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-22 09:19:39 PST
Created attachment 8553185 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-22 09:20:16 PST
Created attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

I'll test them tomorrow.
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-23 01:52:57 PST
Created attachment 8553640 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

* Let's remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK. Even if keys on such devices are pressed, .location value should be DOM_KEY_LOCATION_STANDARD.
* On Android and Gonk, the key location should be computed from its .code value. We should do that in the next patch. (I.e., Android part can get rid of all code which computes .location value.)
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-23 01:55:45 PST
Comment on attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

Okay, then, KeyboardEvent.location should be computed from the .code value.

Perhaps, this could be available on some other platforms, but for now, we should use it only on Android and Gonk.
Comment 8 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2015-01-23 05:35:09 PST
Comment on attachment 8553640 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

(didn't review android parts)
Comment 9 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2015-01-23 05:47:08 PST
Comment on attachment 8553640 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

hmm, we have blogged about the Android behavior
http://cpeterso.com/blog/02012/10/firefox-18-for-android-adds-dom_key_location_joystick-support-for-game-controllers/

I think we need to be a bit careful with this stuff.
So please make sure Android and Gaming folks are ok with this.
(I don't know how we could warn about use of these constants)


What was reason to drop this stuff from D3E?
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-23 06:15:50 PST
(In reply to Olli Pettay [:smaug] from comment #9)
> Comment on attachment 8553640 [details] [diff] [review]
> part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK
> 
> hmm, we have blogged about the Android behavior
> http://cpeterso.com/blog/02012/10/firefox-18-for-android-adds-
> dom_key_location_joystick-support-for-game-controllers/
> 
> I think we need to be a bit careful with this stuff.
> So please make sure Android and Gaming folks are ok with this.
> (I don't know how we could warn about use of these constants)

We already announced this bug in MDN too...

> What was reason to drop this stuff from D3E?

https://www.w3.org/Bugs/Public/show_bug.cgi?id=22834
Because MOBILE isn't useful, cannot define it clearly. On the other hand, JOYSTICK shouldn't be a scope of this bug because there is GamePad API.
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-23 06:18:39 PST
And also, looks like that Chromiun doesn't support _MOBILE and _JOYSTICK:
http://mxr.mozilla.org/chromium/search?string=DOM_KEY_LOCATION
Comment 12 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-23 06:26:30 PST
http://jsfiddle.net/d_toybox/gLkdjgtn/1/

I tested on this. _MOBILE and _JOYSTICK are supported only on IE and Gecko. But I don't think that IE actually uses the values.
Comment 13 Michael Wu [:mwu] 2015-01-23 12:16:09 PST
Comment on attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

Review of attachment 8553188 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for gonk parts
Comment 14 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-27 22:09:26 PST
Comment on attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

cpeterson:

It seems that Jim Chen on vacation or something. Can you review this and next patch instead of him? (Or, it might be you are better person for this?)
Comment 15 Chris Peterson [:cpeterson] 2015-01-28 00:01:14 PST
Comment on attachment 8553640 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

Review of attachment 8553640 [details] [diff] [review]:
-----------------------------------------------------------------

Android bits LGTM.
Comment 16 Chris Peterson [:cpeterson] 2015-01-28 00:09:43 PST
Comment on attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

Review of attachment 8553188 [details] [diff] [review]:
-----------------------------------------------------------------

Android bits LGTM.

::: widget/WidgetEventImpl.cpp
@@ +428,5 @@
> +    case CODE_NAME_INDEX_NumpadEqual:
> +    // case CODE_NAME_INDEX_NumpadMemoryAdd:
> +    // case CODE_NAME_INDEX_NumpadMemoryClear:
> +    // case CODE_NAME_INDEX_NumpadMemoryRecall:
> +    // case CODE_NAME_INDEX_NumpadMemoryStore:

Why are these numpad codes commented out? Consider adding a comment explanation at the top of the switch statement (or on every commented-out case statement).
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) 2015-01-28 00:22:01 PST
Thanks!!

The commented out cases are not actually defined in Gecko but spec defines them. When we enable it, the commented out cases help us when we define it. I'll add like this comment to there.

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