1753641 - Mute/Unmute is applied instead of volume change when using up/down arrow

Status: VERIFIED FIXED

(Toolkit :: Picture-in-Picture, defect, P2)

Description

[Andrei Purice]

Attachment: pip up down.mp4

Affected platform:
ALL

Affected versions:
Nightly 98.0a1

Steps to reproduce:

1. Launch Firefox
2. Go to YouTube and start a random video
3. Click on the PiP Toggle (or use the shortcuts to open it)
4. While inside the PIP window press up/down arrow in order to change the volume

Expected result:
Volume increases/decreases with 5% for every up/down tap.

Actual Result:
The mute/unmute toggle is applied instead of volume change.

Notes:
Setting a severity of S4.
Not a regression as the volume button was added in Firefox version 98.

Comment 1

[Ania]

Hi Andrei,

I'm a little confused about this one - the volume button has been available in Nightly for a while, it wasn't added in 98, and we list arrow up/down as an existing shortcut since version 83 in the /about page - https://support.mozilla.org/en-US/kb/about-picture-picture-firefox#w_keyboard-shortcuts
So it looks like a regression to me (although I have not used this shortcut myself and am not sure when it stopped working), no?

Comment 2

[kpatenio]

Looks like this occurred after adding specific site adapters. Regression log: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b82a2d152d936bb88467ae296331455a06a5d2b2&tochange=320b13d2bea918a32d093402213c7d5fd649c500

We end up toggling mute after pressing up or down arrows when watching a video from YouTube.

Comment 3

[Micah [:mtigley] (she/her)]

(In reply to kpatenio from comment #2)

Looks like this occurred after adding specific site adapters. Regression log: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b82a2d152d936bb88467ae296331455a06a5d2b2&tochange=320b13d2bea918a32d093402213c7d5fd649c500

We end up toggling mute after pressing up or down arrows when watching a video from YouTube.

Ah you're right. Since we call setMuted when increase/decreasing volume it ends up calling this site wrapper method we defined: https://searchfox.org/mozilla-central/rev/4615b544a0f7166233c409c619b426c4025467a7/browser/extensions/pictureinpicture/video-wrappers/youtube.js#11 and toggles the mute/unmute state.

Perhaps we can revert that back to video.volume = false. Or, we remove those lines and update the handler for "volumechange" to also check whether or not video.volume < 0.1 / video.volume > 0.1: https://searchfox.org/mozilla-central/rev/4615b544a0f7166233c409c619b426c4025467a7/toolkit/actors/PictureInPictureChild.jsm#1234-1238

Comment 4

[Ania]

Thank you, Katherine.
Does this look like a big lift to fix? It would be good to fix in 100 when we release site-specific adapters, and the issue becomes much more noticeable.

Comment 5

[Ania]

(In reply to amininkova from comment #4)

Micah, Katherine, thank you.
Does this look like a big lift to fix? It would be good to fix in 100 when we release site-specific adapters, and the issue becomes much more noticeable.

Comment 6

[Micah [:mtigley] (she/her)]

Attachment: Bug 1753641 - Update setMuted method for Youtube video wrapper to only toggle when setting a new muted state. r?mhowell,kpatenio

Comment 7

[Andrei Purice]

Sorry for the confusion, indeed this was a regression and I'd like to thank kpatenio for taking the time to find its range.
I'll be of any further assistance if needed.

Comment 8

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1670108

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3dbc7f2e58dd
Update setMuted method for Youtube video wrapper to only toggle when setting a new muted state. r=mhowell,kpatenio

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/3dbc7f2e58dd

Comment 11

[Pascal Chevrel (relman team) -> :pascalc]

Micah, is that still a disabled feature in 98 or should we uplift this patch to beta? Thanks

Comment 12

[Micah [:mtigley] (she/her)]

(In reply to Pascal Chevrel:pascalc from comment #11)

Micah, is that still a disabled feature in 98 or should we uplift this patch to beta? Thanks

Yes this feature is still disabled for non-Nightly, so we don't need to uplift to beta. Thanks!

Comment 13

[Giorgia Nichita, Release Desktop QA]

I've reproduced this issue on an affected nightly build (2022-02-04) on Windows 10 x64.

Verified as fixed on Firefox 99.0b7 (20220322185926) on Windows 10 x64, Ubuntu 20.04 and macOS 10.15.