Closed Bug 936313 Opened 11 years ago Closed 9 years ago

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

Categories

(Core :: DOM: Events, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: masayuki, Assigned: masayuki)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-complete, site-compat)

Attachments

(2 files, 1 obsolete file)

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.
Smaug:

Don't you mind we will drop them from nsIDOMKeyEvent and KeyboardEvent.webidl?
Flags: needinfo?(bugs)
I think that should be ok.
Flags: needinfo?(bugs)
I think that we can fix this bug easier after bug 865649 (KeyboardEvent.code) is fixed.
Status: NEW → ASSIGNED
* 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.)
Attachment #8553185 - Attachment is obsolete: true
Attachment #8553640 - Flags: superreview?(bugs)
Attachment #8553640 - Flags: review?(nchen)
Attachment #8553640 - Flags: review?(bugs)
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.
Attachment #8553188 - Flags: review?(nchen)
Attachment #8553188 - Flags: review?(mwu)
Attachment #8553188 - Flags: review?(bugs)
Comment on attachment 8553640 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK

(didn't review android parts)
Attachment #8553640 - Flags: review?(bugs) → review+
Attachment #8553188 - Flags: review?(bugs) → review+
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?
Attachment #8553640 - Flags: superreview?(bugs) → superreview+
(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.
And also, looks like that Chromiun doesn't support _MOBILE and _JOYSTICK:
http://mxr.mozilla.org/chromium/search?string=DOM_KEY_LOCATION
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 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
Attachment #8553188 - Flags: review?(mwu) → review+
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?)
Attachment #8553188 - Flags: review?(cpeterson)
Attachment #8553640 - Flags: review?(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.
Attachment #8553640 - Flags: review?(nchen)
Attachment #8553640 - Flags: review?(cpeterson)
Attachment #8553640 - Flags: review+
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).
Attachment #8553188 - Flags: review?(nchen)
Attachment #8553188 - Flags: review?(cpeterson)
Attachment #8553188 - Flags: review+
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.
https://hg.mozilla.org/mozilla-central/rev/3f291f100e26
https://hg.mozilla.org/mozilla-central/rev/53cf3295596e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.