Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Description • 8 years ago

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.

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 1 • 8 years ago

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.

	[:fabrice] Fabrice Desré
	Comment 2 • 8 years ago

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!

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 3 • 8 years ago

Thank you for your quick check!

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 4 • 8 years ago

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

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 5 • 8 years ago

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

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 6 • 8 years ago

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/

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 7 • 8 years ago

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/

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 8 • 8 years ago

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/

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 9 • 8 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 10 • 8 years ago

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.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 11 • 8 years ago

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

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 12 • 8 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 13 • 8 years ago

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?

	Masayuki Nakano [:masayuki] (he/him)(JST, +0900) Assignee
	Comment 14 • 8 years ago

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

	Pulsebot
	Comment 15 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 16 • 8 years ago
bugherder

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

	Kohei Yoshino [:kohei]
	Comment 17 • 8 years ago

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

	Eric Shepherd [:sheppy]
	Comment 18 • 8 years ago

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.

	Eric Shepherd [:sheppy]
	Comment 19 • 8 years ago

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.

	Eric Shepherd [:sheppy]
	Comment 20 • 8 years ago

KeyboardEvent.code is updated.