Closed Bug 962560 Opened 6 years ago Closed 4 years ago

Audio volume UI resets to full volume after toggling the screen size of a video


(Toolkit :: Video/Audio Controls, defect)

28 Branch
Not set



Tracking Status
firefox49 --- fixed


(Reporter: avaida, Assigned: jaws)




(1 file, 3 obsolete files)

User agents:
[1] Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
[2] Mozilla/5.0 (Windows NT 6.3; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
[3] Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0
[4] Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
[5] Mozilla/5.0 (Machintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0

Reproducible on:
Windows 7 64-bit [1], Windows 8 64-bit [2], Ubuntu 12.04 32-bit [3], Ubuntu 13.10 64-bit [4] and Mac OS X 10.9 [5] platforms, using:
- the latest Aurora (Build ID: 20140122004004)
- the latest Nightly (Build ID: 20140122030521)

Steps to reproduce:
1. Open one of the VP9-encoded video samples attached to Bug 949525
* e.g.
2. Adjust the audio volume of the video using the controls bar
3. Change the video's window size to full screen using the associated button
4. Change the video's screen size back to the default one

Expected results: The audio volume remains as configured at Step 2 no matter how many times the user changes the video's screen size.

Actual results: In terms of actual sound, the audio volume of the video remains the same, but the UI from the controls bar displays full volume instead of actual one.
I'm guessing this isn't VP9 specific, but a videocontrols issue.
Component: Video/Audio → Video/Audio Controls
Product: Core → Toolkit
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → jaws
Attachment #8749800 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8749800 [details] [diff] [review]

Review of attachment 8749800 [details] [diff] [review]:

Nice find, thanks!

::: toolkit/content/widgets/videocontrols.xml
@@ +412,5 @@
>                      this.controlBar.hidden = true;
>                      this.adjustControlSize();
> +
> +                    // Must compute _volumeControlWidth first since the
> +                    // slider implementation requires it.

very-pedantic-nit: Because this comment is at the second thing out of the 2 order-dependent things we're doing, I think it makes more sense if it says something like:

"Can only update the volume controls once we've computed _volumeControlWidth, since the volume slider implementation depends on it."

Or something.
Attachment #8749800 - Flags: review?(gijskruitbosch+bugs) → review+
Still doesn't apply cleanly to inbound.
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.