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

RESOLVED FIXED in Firefox 49

Status

()

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

People

(Reporter: masayuki, Assigned: masayuki)

Tracking

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

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

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [parity-Chrome] btpp-active)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

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.
Whiteboard: [parity-Chrome] → [parity-Chrome] btpp-active
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.
Flags: needinfo?(fabrice)
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!
Flags: needinfo?(fabrice)
Thank you for your quick check!
Created attachment 8755671 [details]
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/
Attachment #8755671 - Flags: review?(bugs)
Attachment #8755672 - Flags: review?(bugs)
Attachment #8755673 - Flags: review?(bugs)
Created attachment 8755672 [details]
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/
Created attachment 8755673 [details]
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 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.
Attachment #8755671 - Flags: review?(bugs) → review+
Attachment #8755672 - Flags: review?(bugs) → review+
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 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
Attachment #8755673 - Flags: review?(bugs) → review+
(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.
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?
(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 16

2 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
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Depends on: 1277234

Updated

2 years ago
Keywords: dev-doc-needed, site-compat
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.
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.
Keywords: dev-doc-needed → dev-doc-complete
KeyboardEvent.code is updated.
You need to log in before you can comment on or make changes to this bug.