1272578 - [UI Events-key] Rename VolumeDown, VolumeUp and VolumeMute to AudioVolumeDown, AudioVolumeUp and AudioVolumeMute

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

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

Volume* key values are renamed to AudioVolume* for consistency with other audio related key names. Chromium already uses the new value. We should update the name as soon as possible.

Comment 1

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

fabrice:

Renaming key value "Volume(Down|Mute|Up)" to "AudioVolume(Down|Mute|Up)" causes something wrong in Gaia? If so, I'll use #ifdef/#ifndef MOZ_B2G for Gaia.

Comment 2

[[:fabrice] Fabrice Desré]

Hi Masayuki,

Yes, Gaia uses these events so that would be great for now if you could #ifdef them. See https://mxr.mozilla.org/gaia/search?string=VolumeUp for instance.

thanks!

Comment 3

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

Thank you for your quick check!

Comment 4

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

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

Comment 5

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

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

Comment 6

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

Attachment: MozReview Request: Bug 1272578 part.1 Rename key name VolumeDown to AudioVolumeDown except on B2G r?smaug

VolumeDown was renamed to AudioVolumeDown in the latest draft and Chromium uses the new name. Therefore, we need to update this but Gaia uses the old name. So, we shouldn't rename on B2G until Gaia is fixed.

Note that this patch changes tests but they are not used by B2G. Therefore, just replacing with new name is enough.

Only forms.js is necessary #ifdef because the main purpose of forms.js is for B2G's IME framework. However, it's available on the other platforms if chrome needs to use it.

Review commit: https://reviewboard.mozilla.org/r/54398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54398/

Comment 7

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

Attachment: MozReview Request: Bug 1272578 part.2 Rename key name VolumeUp to AudioVolumeUp except on B2G r?smaug

VolumeUp was renamed to AudioVolumeUp in the latest draft and Chromium uses the new name. Therefore, we need to update this but Gaia uses the old name. So, we shouldn't rename on B2G until Gaia is fixed.

Note that this patch changes tests but they are not used by B2G. Therefore, just replacing with new name is enough.

Only forms.js is necessary #ifdef because the main purpose of forms.js is for B2G's IME framework. However, it's available on the other platforms if chrome needs to use it.

Review commit: https://reviewboard.mozilla.org/r/54400/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54400/

Comment 8

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

Attachment: MozReview Request: Bug 1272578 part.3 Rename key name VolumeMute to AudioVolumeMute except on B2G r?smaug

VolumeMute was renamed to AudioVolumeMute in the latest draft and Chromium uses the new name. Therefore, we need to update this but Gaia uses the old name. So, we shouldn't rename on B2G until Gaia is fixed.

Note that this patch changes tests but they are not used by B2G. Therefore, just replacing with new name is enough.

Only forms.js is necessary #ifdef because the main purpose of forms.js is for B2G's IME framework. However, it's available on the other platforms if chrome needs to use it.

Review commit: https://reviewboard.mozilla.org/r/54402/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54402/

Comment 9

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

Comment on attachment 8755671 [details]
MozReview Request: Bug 1272578 part.1 Rename key name VolumeDown to AudioVolumeDown except on B2G r?smaug

https://reviewboard.mozilla.org/r/54398/#review51462

It is still mystery to me why we're making these backwards incompatible changes to the spec.

Comment 10

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

Comment on attachment 8755672 [details]
MozReview Request: Bug 1272578 part.2 Rename key name VolumeUp to AudioVolumeUp except on B2G r?smaug

https://reviewboard.mozilla.org/r/54400/#review51464

Has blink already shipped these new names, or have they just implemented them in some dev builds? Since if they haven't yet shipped, then I'm against all this renaming, but if this is all in their release builds, then fine.

Comment 11

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

Comment on attachment 8755673 [details]
MozReview Request: Bug 1272578 part.3 Rename key name VolumeMute to AudioVolumeMute except on B2G r?smaug

https://reviewboard.mozilla.org/r/54402/#review51466

Comment 12

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

(In reply to Olli Pettay [:smaug] from comment #10)
> Comment on attachment 8755672 [details]
> MozReview Request: Bug 1272578 part.2 Rename key name VolumeUp to
> AudioVolumeUp except on B2G r?smaug
> 
> https://reviewboard.mozilla.org/r/54400/#review51464
> 
> Has blink already shipped these new names, or have they just implemented
> them in some dev builds? Since if they haven't yet shipped, then I'm against
> all this renaming, but if this is all in their release builds, then fine.

Not yet, but soon (starting Chrome 51, will be shipped on May 31st, 2016).
https://www.chromium.org/developers/calendar

I've filed the issue to revert the changes but volume keys for microphone are added to the spec. Therefore, they rename the Volume* to AudioVolume* for explaining the meaning clearer with their names.
https://github.com/w3c/uievents-key/issues/14

Okay, I wait next release of Chromium because I'm requesting to put off to release their KeyboardEvent.key because of completely broken.

Comment 13

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

No need to wait for landing if blink will ship the new names. But could we still try to convince them to revert back to the old names?

Comment 14

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

(In reply to Olli Pettay [:smaug] from comment #13)
> No need to wait for landing if blink will ship the new names. But could we
> still try to convince them to revert back to the old names?

I hope so but probably Garykac won't agree because the reported issue was already disagreed and there are a lot of incompatible names with IE/Edge.

Okay, I'll land them (anyway, they're easy to be backed out).

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0604395964c
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e106633b86e
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfaea09ca341

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d0604395964c
https://hg.mozilla.org/mozilla-central/rev/8e106633b86e
https://hg.mozilla.org/mozilla-central/rev/cfaea09ca341

Comment 17

[Kohei Yoshino]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/some-keyboardevent-key-values-have-been-renamed/

Comment 18

[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 19

[Eric Shepherd [:sheppy]]

An appropriate footnote has been added to the table of audio control keys at https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key#Audio_control_keys

The "code" page still needs to be updated.

Comment 20

[Eric Shepherd [:sheppy]]

KeyboardEvent.code is updated.