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

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

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

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.

Comment 1

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

Filed two issues:
https://github.com/w3c/uievents-key/issues/11
https://github.com/w3c/uievents-key/issues/12

Comment 2

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=750a55ab69ef

Comment 3

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

I'm asking about Volume* -> AudioVolume*:
https://github.com/w3c/uievents-key/issues/14

That's too bad for backward compatibility.

Comment 4

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

(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.

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2c1c8c22707

Comment 6

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

Attachment: 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

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/

Comment 7

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

Attachment: 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 commit: https://reviewboard.mozilla.org/r/52736/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52736/

Comment 8

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

Note that this does NOT change any behavior of KeyboardEvent.key. Just updating the headers for the latest draft.

Comment 9

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

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.

Comment 10

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

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 11

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

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

Comment 12

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

(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 13

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=05d3bfd1eac1

Comment 14

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

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/

Comment 15

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

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/

Comment 16

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

Today, we didn't hold telecon. So, for now, let's land the patches without AudioBass(Down|Up).

Comment 17

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

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

Comment 18

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6268a6661731
https://hg.mozilla.org/integration/mozilla-inbound/rev/31b5b7eb5c2a

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6268a6661731
https://hg.mozilla.org/mozilla-central/rev/31b5b7eb5c2a

Comment 20

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

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

Comment 21

[Kohei Yoshino]

Okay, thanks for the clarification!

Comment 22

[Kohei Yoshino]

I guess the new key values have to be documented.

Comment 23

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

(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).

Comment 24

[Eric Shepherd [:sheppy]]

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.

Comment 25

[Eric Shepherd [:sheppy]]

KeyboardEvent.key has been updated; KeyboardEvent.code is on my short list of things to do soon.

Comment 26

[Eric Shepherd [:sheppy]]

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.

Comment 27

[Eric Shepherd [:sheppy]]

(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.