Closed Bug 694664 Opened 10 years ago Closed 10 years ago

Remove html5 audio/video volume slider

Categories

(Toolkit :: Video/Audio Controls, defect)

defect
Not set
normal

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)

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.
Attached patch Patch for bug 694664 (obsolete) — Splinter Review
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)
Blocks: 681553
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.
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-
Attached patch Patch for bug 694664 v2 (obsolete) — Splinter Review
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)
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.
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)
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)
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)
We're going to go keep the volume sliders.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.