936313 - Drop KeyboardEvent.DOM_LOCATION_MOBILE and KeyboardEvent.DOM_LOCATION_JOYSTICK of KeyboardEvent.location since they have been dropped from D3E spec

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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] (he/him)(JST, +0900)]

Smaug:

Don't you mind we will drop them from nsIDOMKeyEvent and KeyboardEvent.webidl?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I think that should be ok.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that we can fix this bug easier after bug 865649 (KeyboardEvent.code) is fixed.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: part.2 Compute DOM key location from code value on Android and Gonk

I'll test them tomorrow.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: 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] (he/him)(JST, +0900)]

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][[e-mail]]]

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][[e-mail]]]

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] (he/him)(JST, +0900)]

(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] (he/him)(JST, +0900)]

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] (he/him)(JST, +0900)]

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]]

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] (he/him)(JST, +0900)]

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]]

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]]

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] (he/him)(JST, +0900)]

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.

Comment 18

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f291f100e26
https://hg.mozilla.org/integration/mozilla-inbound/rev/53cf3295596e

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=12c082b28d83

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3f291f100e26
https://hg.mozilla.org/mozilla-central/rev/53cf3295596e

Comment 20

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.location
https://developer.mozilla.org/en-US/Firefox/Releases/38

Comment 21

[Kohei Yoshino]

https://developer.mozilla.org/en-US/Firefox/Releases/38/Site_Compatibility