1592729 - Hide the Picture-in-Picture toggle and context menu item for MediaStream <video> elements

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, task, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

See bug 1592539 comment 1 for context.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1592729 - Hide the Picture-in-Picture toggle for MediaStream videos. r?Gijs

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1592729 - Hide the Picture-in-Picture context menu item for MediaStream videos. r=Gijs

Depends on D51156

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1592729 - Don't allow initting Picture-in-Picture for MediaStream videos. r?Gijs

Depends on D51157

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1592729 - Add tests to ensure that the Picture-in-Picture toggle and context menus do not display for MediaStream videos. r?Gijs

This also updates the canToggle=false code paths in the Picture-in-Picture
mochitest head.js file, which were obviously never really exercised until
now.

Depends on D51159

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30cd20aafa10
Hide the Picture-in-Picture toggle for MediaStream videos. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/851938f1b76d
Hide the Picture-in-Picture context menu item for MediaStream videos. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/8383db870a39
Don't allow initting Picture-in-Picture for MediaStream videos. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/48a04a323ddc
Add tests to ensure that the Picture-in-Picture toggle and context menus do not display for MediaStream videos. r=Gijs

Comment 6

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/30cd20aafa10
https://hg.mozilla.org/mozilla-central/rev/851938f1b76d
https://hg.mozilla.org/mozilla-central/rev/8383db870a39
https://hg.mozilla.org/mozilla-central/rev/48a04a323ddc

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9105340 [details]
Bug 1592729 - Hide the Picture-in-Picture toggle for MediaStream videos. r?Gijs

Beta/Release Uplift Approval Request

• User impact if declined: Users on Windows who want to use the upcoming Picture-in-Picture feature can potentially enter a state where they end up seeing a really glitchy video feed from MediaStream video sources (example: camera feeds, screen feeds).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This adds an additional condition to the methods that determine whether or not to display the PiP toggle and context menu item. It also adds an additional check within the code that triggers PiP to ensure that these types of videos can't be opened in an always-on-top player.
• String changes made/needed: None.

Comment 8

[Pascal Chevrel:pascalc]

Comment on attachment 9105340 [details]
Bug 1592729 - Hide the Picture-in-Picture toggle for MediaStream videos. r?Gijs

P1, bug fix for a 71 feature, patch with tests, approved for 71 beta 7, thanks.

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/476da247967a
https://hg.mozilla.org/releases/mozilla-beta/rev/d4ccb04f3327
https://hg.mozilla.org/releases/mozilla-beta/rev/63b79025aef3
https://hg.mozilla.org/releases/mozilla-beta/rev/3b9e006450ac

Comment 10

[Cristian Craciun]

Build ID 20191128221751
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Build ID 20191202091209
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100101 Firefox/72.0

Verified as fixed on the latest version of Firefox Beta (71.b12) and Nightly (72.0a1) using Windows 10.