876133 - Volume Level does not change any more on HTML5 video

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/97aa3da59001
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130524 Firefox/24.0 ID:20130524050555

On youtube HTML5 video,
Using Volume Slider, volume level does not change.

Bug 875483 fixed freeze. however, this volume problem is not fixed.

Steps To Reproduce:
1. Open Youtube HTML5 video
   ex. http://www.youtube.com/watch?v=BH0BNbwqNK8&webm=1&html5=1
2. Drag Volume slider
   --- observe sound level

Actual Results:
Volume level does not change.

Expected Results:
Depending on the position of the slider, volume should change.


Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/6066b9d92032
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130522 Firefox/24.0 ID:20130522071950
Bad:
http://hg.mozilla.org/mozilla-central/rev/00b264c7cced
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130522 Firefox/24.0 ID:20130522092949
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6066b9d92032&tochange=00b264c7cced

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/75abbd7e7e24
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130521 Firefox/24.0 ID:20130521104716
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a25a93b276a5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20130521 Firefox/24.0 ID:20130521105015
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=75abbd7e7e24&tochange=a25a93b276a5


Regressed By: 
a25a93b276a5	Andrea Marchesini — Bug 872925 - Game pack keeps playing audio when running in the background, even though the app has no permission to do so. r=derf

Comment 1

[Alice0775 White]

In addition, the HTML5 video of the following pages is affected, too.
http://www.mozilla.org/projects/firefox/prerelease.html

Comment 2

[Alice0775 White]

PS
I was surprised at a fact that Mozilla did not provide an automatic test for such basic function...

Comment 3

[Alice0775 White]

I can also reproduce the problem on ubuntu 12.04LTS.
http://hg.mozilla.org/mozilla-central/rev/7a2f7a45819a
Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20130525 Firefox/24.0 ID:20130525031005

Comment 4

[Timothy B. Terriberry (:derf)]

Poor reviewing on my part in bug 872925.

In HTMLMediaElement::SetVolume():

  // Here we want just to update the volume.
  SetMutedInternal(mMuted);

The early return in SetMutedInternal() is preventing this from working.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This is against v1.0.1. Should also apply to v1.1.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Tim tested a patch equivalent to this and it worked. I will do some testing of my own.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 754310 [details] [diff] [review]
fix

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 872925
User impact if declined: Volume changes don't work :-(
Testing completed: some manual testing. More in progress.
Risk to taking this patch (and alternatives if risky): very low risk. The bug can probably be worked around in apps by toggling the "muted" attribute.
String or UUID changes made by this patch: none.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK I tested this patch and volume changes seem to work on 1.0.1.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Feel free to uplift this as soon as approval is granted.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/5df7f057e3e3
https://hg.mozilla.org/releases/mozilla-b2g18/rev/9cac5a5c4efb
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb8764489b31

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/eb8764489b31

Comment 12

[Jason Smith [:jsmith]]

Verified on trunk on 5/28.