Closed
Bug 694664
Opened 13 years ago
Closed 13 years ago
Remove html5 audio/video volume slider
Categories
(Toolkit :: Video/Audio Controls, defect)
Toolkit
Video/Audio Controls
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: fryn, Assigned: jaws)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
18.61 KB,
patch
|
Details | Diff | Splinter Review |
In bug 649490, we decided to remove the html5 audio/video volume slider.
The mute button will be retained.
The keyboard shortcuts (up/down arrow keys) for changing volume will be retained.
The API for changing volume will be retained as according to the spec.
The OS already provides sufficient volume controls.
The consensus is that we don't want this in primary UI, and, because of bug 513876, we don't have a sensible proposal for volume controls in secondary UI. In particular, we dislike YouTube-style volume controls, primarily because they move the volume controls between the play/pause button and the time scrubber and secondarily because they are horizontal.
Assignee | ||
Comment 1•13 years ago
|
||
I have removed the volume slider from desktop. I will create a follow up bug for touch UI since I don't have a touch-based UI debugging environment set up to verify my changes.
I also added the fix for bug 694696 in this patch since it was trivial.
Please let me know if I misinterpreted the goal of this bug.
Assignee: fryn → jwein
Attachment #567553 -
Flags: review?(dolske)
Comment 2•13 years ago
|
||
I still think this is a bad idea. I asked my contacts at YouTube, and their data suggests that in-player volume control is heavily used: only ~55% of videos start with the volume set to 100% with mute off. The other 45% have all been adjusted to some other setting. Now YouTube's volume setting is remembered across videos/pages/sessions, so the 45% of people with non 100% volume aren't changing the volume manually on every YouTube page/video they load, but they have changed the volume manually at least once and left it at that setting.
Reporter | ||
Comment 3•13 years ago
|
||
Comment on attachment 567553 [details] [diff] [review]
Patch for bug 694664
The patch looks great except for this part:
> case "volumechange":
> - var volume = this.video.muted ? 0 : Math.round(this.video.volume * 100);
> - this.setMuteButtonState(this.video.muted);
> - this.volumeControl.value = volume;
> + this.setMuteButtonState(this.video.muted || this.video.volume == 0);
> break;
I see what you're trying to do here (make the mute button display muted if the user holds down the down arrow key until the media's volume is zero), but the code above results in the mute button becoming broken, i.e. clicking on it does nothing. If we want to have the default mute button display this composite volume+mute status (which I think is a good idea), then clicking the mute button while the volume is 0 should reset it to 100 (actually an idea that Jared proposed in a conversation and with which I agree :).
I suggest cancelling review until the patch also includes that bit.
Attachment #567553 -
Flags: feedback-
Assignee | ||
Comment 4•13 years ago
|
||
Fixed the issue that fryn and I had talked about with the mute button.
Attachment #567553 -
Attachment is obsolete: true
Attachment #567553 -
Flags: review?(dolske)
Attachment #567786 -
Flags: review?(dolske)
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 567786 [details] [diff] [review]
Patch for bug 694664 v2
Firstly, thanks for taking the bug! I appreciate the work you've been putting in it.
> + this.setMuteButtonState(this.video.muted || this.video.volume == 0);
> + if (!this.video.muted && this.video.volume == 0) {
> + this.video.muted = true;
> + }
...
> + if (!this.video.muted && this.video.volume == 0)
> + this.video.volume = 1;
This creates a bit of wonkiness. For example, pressing accel+uparrow when the volume is zero doesn't reset the volume, while clicking the mute button does. This keyboard shortcut behavior is how OS X volume keys work, and I'm fine with this, but I think that we could use some visual feedback when toggling the mute button. I guess that's something for bug 495162.
There's already a bit of wonkiness in the existing UI with keyboard shortcuts: while the volume is zero and the video is muted, pressing the down arrow unmutes the video, and this fixes that (which is great).
Nit: let's be consistent and remove the braces around the aforementioned `if` statement when pushing this.
Assignee | ||
Comment 6•13 years ago
|
||
Comment on attachment 567786 [details] [diff] [review]
Patch for bug 694664 v2
I'm going to refactor the auto-switching to muted code, but would appreciate feedback on the rest of the patch in the meantime.
Attachment #567786 -
Flags: review?(dolske) → feedback?(dolske)
Assignee | ||
Comment 7•13 years ago
|
||
This patch fixes the bug as well as adds the visual feedback when the volume is set to zero.
I understand that there are two parties that want separate outcomes of this bug (WONTFIX vs. FIXED). We will need to come to an agreement over the prominence of volume controls.
Here is a recap of pros & cons
Pros:
- Due to Bug 513876 we cannot draw controls outside of the control bar. This patch brings parity between <audio> and <video>.
- Most users should be happy with just using the mute button.
Cons:
- Removing the controls will remove parity with IE and Chrome.
- Based on the feedback from :cpearce, ~45% of YouTube users use the per-video volume controls often.
- Some users may complain that they are losing granular control (whether they needed it or not).
Please add more pros & cons if I left some out.
Attachment #567786 -
Attachment is obsolete: true
Attachment #567786 -
Flags: feedback?(dolske)
Attachment #567949 -
Flags: ui-review?(faaborg)
Attachment #567949 -
Flags: review?(dolske)
Assignee | ||
Comment 8•13 years ago
|
||
Pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=65e1b820560e
Comment 9•13 years ago
|
||
Comment on attachment 567949 [details] [diff] [review]
Patch for bug 694664 v3
I really don't want to remove it entirely. :(
I'm going to reopen the other bug, I think we should try harder before taking this option.
Attachment #567949 -
Flags: ui-review?(faaborg)
Attachment #567949 -
Flags: review?(dolske)
Assignee | ||
Comment 10•13 years ago
|
||
We're going to go keep the volume sliders.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•