Closed Bug 1232919 Opened 9 years ago Closed 8 years ago

[UI Events] Reorganize KeyNameList.h and add new key names which was added to the latest draft

Categories

(Core :: DOM: Events, defect)

Product:
Core
Component:
DOM: Events
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla49
Tracking Flags:
Tracking Status
firefox46 --- affected
firefox49 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the latest UI Evnets KeyboardEvent key value spec, add new key values and drop legacy key values which are never used internally r?smaug
58 bytes, text/x-review-board-request
smaug
: review+
Details
MozReview Request: Bug 1232919 part.2 Reorganize NativeKeyToDOMKeyName.h for using same definition order of the latest UI Events KeyboardEvent key values r?smaug
58 bytes, text/x-review-board-request
smaug
: review+
Details 
At TPAC 2015, a lot of key values were added for supporting all keys of Android. And the key name groups are reorganized. We should update KeyNameList.h for the latest spec.
I'm asking about Volume* -> AudioVolume*:
https://github.com/w3c/uievents-key/issues/14

That's too bad for backward compatibility.
(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #3)
> I'm asking about Volume* -> AudioVolume*:
> https://github.com/w3c/uievents-key/issues/14
> 
> That's too bad for backward compatibility.

Okay, we've discussed this issue in today's meeting. Currently, MicrophoneVolume* are added to the spec. Therefore, Volume* becomes much ambiguous name, so, for clearing their meaning, Audio prefix is a good idea. Currently, Chromium hasn't support .key nor .code without changing their settings and IE already has a lot of older draft's key values. So, breaking backward compatibility won't break the (actual) web.
This patch reorganizes KeyNameList.h with the latest UI Events KeyboardEvent key values spec.

Additionally adding new key values:
* "Hyper"
* "Super"
* "Key11"
* "Key12"
* "AudioBassDown"
* "AudioBassBoostToggle"
* "AudioBassUp"
* "AudioTrebleDown"
* "AudioTrebleUp"
* "MicrophoneToggle"
* "MicrophoneVolumeDown"
* "MicrophoneVolumeUp"
* "MicrophoneVolumeMute"
* "SpeechCorrectionList"
* "SpeechInputToggle"
* "LaunchContacts"
* "LaunchPhone"
* "AppSwitch"
* "Call"
* "CameraFocus"
* "EndCall"
* "GoBack"
* "GoHome"
* "HeadsetHook"
* "LastNumberRedial"
* "Notification"
* "MannerMode"
* "VoiceDial"
* "TV3DMode"
* "TVAntennaCable"
* "TVAudioDescription"
* "TVAudioDescriptionMixDown"
* "TVAudioDescriptionMixUp"
* "TVContentsMenu"
* "TVDataService"
* "TVInput"
* "TVInputComponent1"
* "TVInputComponent2"
* "TVInputComposite1"
* "TVInputComposite2"
* "TVInputHDMI1"
* "TVInputHDMI2"
* "TVInputHDMI3"
* "TVInputHDMI4"
* "TVInputVGA1"
* "TVMediaContext"
* "TVNetwork"
* "TVNumberEntry"
* "TVRadioService"
* "TVSatellite"
* "TVSatelliteBS"
* "TVSatelliteCS"
* "TVSatelliteToggle"
* "TVTerrestrialAnalog"
* "TVTerrestrialDigital"
* "TVTimer"
* "DVR"
* "MediaAudioTrack"
* "MediaSkipBackward"
* "MediaSkipForward"
* "MediaStepBackward"
* "MediaStepForward"
* "MediaTopMenu"
* "NavigateIn"
* "NavigateNext"
* "NavigateOut"
* "NavigatePrevious"

Remove deprecated and unused key values:
* "Separator"

Pending to remove:
* "MozHomeScreen" (Bug 1272599)
* "MozCameraFocusAdjust" (Bug 1272599)
* "MozPhoneCall" (Bug 1272599)
* "OS" (Bug 1232918)
* "MediaSelect" (Bug 1272592)

Review commit: https://reviewboard.mozilla.org/r/52734/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52734/
Attachment #8752705 - Flags: review?(bugs)
Attachment #8752706 - Flags: review?(bugs)
Note that this does NOT change any behavior of KeyboardEvent.key. Just updating the headers for the latest draft.
Comment on attachment 8752705 [details]
MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the latest UI Evnets KeyboardEvent key value spec, add new key values and drop legacy key values which are never used internally r?smaug

https://reviewboard.mozilla.org/r/52734/#review49702

So this removes MediaSkip. I guess that is fine.
Attachment #8752705 - Flags: review?(bugs) → review+
Comment on attachment 8752705 [details]
MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the latest UI Evnets KeyboardEvent key value spec, add new key values and drop legacy key values which are never used internally r?smaug

https://reviewboard.mozilla.org/r/52734/#review49704

::: dom/events/KeyNameList.h:247
(Diff revision 1)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBalanceRight)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassDown)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostDown)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostToggle)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostUp)
> +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassUp)

hmm, wait, what is AudioBassUp and what is AudioBassBoostUp?
Comment on attachment 8752706 [details]
MozReview Request: Bug 1232919 part.2 Reorganize NativeKeyToDOMKeyName.h for using same definition order of the latest UI Events KeyboardEvent key values r?smaug

https://reviewboard.mozilla.org/r/52736/#review49706
Attachment #8752706 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #10)
> Comment on attachment 8752705 [details]
> MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the
> latest UI Evnets KeyboardEvent key value spec, add new key values and drop
> legacy key values which are never used internally r?smaug
> 
> https://reviewboard.mozilla.org/r/52734/#review49704
> 
> ::: dom/events/KeyNameList.h:247
> (Diff revision 1)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBalanceRight)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassDown)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostDown)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostToggle)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassBoostUp)
> > +DEFINE_KEYNAME_WITH_SAME_NAME(AudioBassUp)
> 
> hmm, wait, what is AudioBassUp and what is AudioBassBoostUp?

Ah, good point. According to the spec, they are not explicitly specified the difference. I filed a spec issue: https://github.com/w3c/uievents-key/issues/25

I'd just drop AudioBass(Down|Up) from the patch temporarily if the issue wouldn't be fixed soon.
Comment on attachment 8752705 [details]
MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the latest UI Evnets KeyboardEvent key value spec, add new key values and drop legacy key values which are never used internally r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52734/diff/1-2/
Attachment #8752705 - Flags: review?(bugs)
Comment on attachment 8752706 [details]
MozReview Request: Bug 1232919 part.2 Reorganize NativeKeyToDOMKeyName.h for using same definition order of the latest UI Events KeyboardEvent key values r?smaug

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52736/diff/1-2/
Today, we didn't hold telecon. So, for now, let's land the patches without AudioBass(Down|Up).
Attachment #8752705 - Flags: review?(bugs) → review+
Comment on attachment 8752705 [details]
MozReview Request: Bug 1232919 part.1 Reorganize KeyNameList.h with the latest UI Evnets KeyboardEvent key value spec, add new key values and drop legacy key values which are never used internally r?smaug

https://reviewboard.mozilla.org/r/52734/#review50668
https://hg.mozilla.org/mozilla-central/rev/6268a6661731
https://hg.mozilla.org/mozilla-central/rev/31b5b7eb5c2a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
> Keywords: dev-doc-needed, site-compat

No, this bug doesn't change any "visible" behavior. Even if a key name isn't registered, web developers can use it. If it's registered in Gecko, the key name is managed by 16bit integer instead of string. So, the fix may increase or decrease footprint of a key event only few bytes.

Only bug 1272578 and bug 1272592 are affected to web contents, but they change Gecko's key names to the latest spec's and Chrome's key names.
Okay, thanks for the clarification!
Keywords: dev-doc-needed, site-compat
I guess the new key values have to be documented.
Keywords: dev-doc-needed
(In reply to Kohei Yoshino [:kohei] from comment #22)
> I guess the new key values have to be documented.

They should be referred to the spec. Our document explains only the mapping of actual values and virtual keycodes of each platform (and differences between browsers).
Updated Firefox 49 for developers. I have a question in to :masayuki about some existing content in the key event docs that I need to clarify before I update the KeyboardEvent.key and KeyboardEvent.code pages as necessary; there are some things on this page that need to be cleaned up.
KeyboardEvent.key has been updated; KeyboardEvent.code is on my short list of things to do soon.
Turns out KeyboardEvent.code has its own dev bug, which hasn't been done yet. I applied a couple of fixes that I knew had already been done from other bugs, and this is now complete.
Keywords: dev-doc-needed → dev-doc-complete
(In reply to Eric Shepherd [:sheppy] from comment #26)
> Turns out KeyboardEvent.code has its own dev bug

That's bug 1264150 by the way.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: