The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla38

Status

()

Core
DOM: Events
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

(Depends on: 1 bug, {dev-doc-complete, site-compat})

Trunk
mozilla38
dev-doc-complete, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Smaug:

Don't you mind we will drop them from nsIDOMKeyEvent and KeyboardEvent.webidl?
Flags: needinfo?(bugs)

Comment 2

3 years ago
I think that should be ok.
Flags: needinfo?(bugs)
(Assignee)

Comment 3

3 years ago
I think that we can fix this bug easier after bug 865649 (KeyboardEvent.code) is fixed.
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Depends on: 865649
(Assignee)

Updated

3 years ago
Depends on: 1017383
Keywords: dev-doc-needed
(Assignee)

Updated

2 years ago
Blocks: 1119609
(Assignee)

Comment 4

2 years ago
Created attachment 8553185 [details] [diff] [review]
part.1 Remove DOM_KEY_LOCATION_MOBILE and DOM_KEY_LOCATION_JOYSTICK
(Assignee)

Comment 5

2 years ago
Created attachment 8553188 [details] [diff] [review]
part.2 Compute DOM key location from code value on Android and Gonk

I'll test them tomorrow.
(Assignee)

Comment 6

2 years ago
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.)
Attachment #8553185 - Attachment is obsolete: true
Attachment #8553640 - Flags: superreview?(bugs)
Attachment #8553640 - Flags: review?(nchen)
Attachment #8553640 - Flags: review?(bugs)
(Assignee)

Comment 7

2 years ago
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 8

2 years ago
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+

Updated

2 years ago
Attachment #8553188 - Flags: review?(bugs) → review+

Comment 9

2 years ago
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+
(Assignee)

Comment 10

2 years ago
(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.
(Assignee)

Comment 11

2 years ago
And also, looks like that Chromiun doesn't support _MOBILE and _JOYSTICK:
http://mxr.mozilla.org/chromium/search?string=DOM_KEY_LOCATION
(Assignee)

Comment 12

2 years ago
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

2 years ago
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+
(Assignee)

Comment 14

2 years ago
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)
(Assignee)

Updated

2 years ago
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+
(Assignee)

Comment 17

2 years ago
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.
(Assignee)

Comment 18

2 years ago
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
https://hg.mozilla.org/mozilla-central/rev/3f291f100e26
https://hg.mozilla.org/mozilla-central/rev/53cf3295596e
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 20

2 years ago
https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent.location
https://developer.mozilla.org/en-US/Firefox/Releases/38
Keywords: dev-doc-needed → dev-doc-complete, site-compat
https://developer.mozilla.org/en-US/Firefox/Releases/38/Site_Compatibility
You need to log in before you can comment on or make changes to this bug.