1777456 - PiP description should not overlap the player controls

Status: VERIFIED FIXED

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

Description

[Ina Popescu, Desktop QA]

Attachment: 2022-06-30_13h12_48.mp4

Note

• Please use a new Firefox profile where PiP was never toggled before

Found in

• Firefox 103.0b2

Affected versions

• Firefox 103.0b2
• Nightly 104.0a1

Affected platforms

• Windows 10 X64
• macOS 12

Steps to reproduce

1. Go to https://www.youtube.com/watch?v=gF1x9oupG14
2. Hover over the PiP toggle button
3. Observe the PiP description displayed on screen

Expected result

• The PiP toggle should be moved to a higher position on screen or the description text should be slightly modified so that the description doesn't overlap the player controls.

Actual result

• The PiP description, when hovering over the PiP toggle on first interaction, overlaps the player controls.
  In the video given as example, user can launch PiP window by clicking the Theater mode button from the player controls bar.
  Same issue occurs with other websites streaming videos when we resize the window size to a smaller size.

Comment 1

[Ina Popescu, Desktop QA]

Attachment: 2022-06-30_12h49_39.png

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

:epopescu, if you think that's a regression, could you try to find a regression range using for example mozregression?

Comment 3

[Ina Popescu, Desktop QA]

Attachment: browser window size_win10.png

Comment 4

[Ina Popescu, Desktop QA]

Attachment: browser window size_mac12.png

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

:mhowell, since you are the author of the regressor, bug 1774688, could you take a look?
For more information, please visit auto_nag documentation.

Comment 7

[Molly Howell (she/her) (no longer active)]

This is something we'd have to be doing on a per-site basis, because we can't predict or detect the height of each player's controls, and it's hard to see this as being worth all that.

Comment 8

[Ania]

Molly, what do you think about making the toggle are a tiny bit wider? This would remedy the control problem and fix the challenge with "Picture-in-Picture" splitting between two lines.
Old toggle has 29 symbols in the longest line in EN locale, and had a decent margin on the left of the text.

What if we did the following?
Pop out this video
More screens are more fun. Play
this video in Picture-in-Picture
while you browse.

31 char
32 char
17 char

Comment 9

[Molly Howell (she/her) (no longer active)]

I don't mind it being wider at all. It's tricky to make this work in general, if only because we can't predict what height anybody's controls are going to be, but making it wider than it is so that we know it's at least going to fit the height we want seems totally reasonable to me.

Comment 10

[Ania]

Awesome, thank you, Molly!

Let's go for it then, and uplift it into 103, so we can have a pretty new first-time toggle together with the subtitles settings!

Comment 11

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1777456 - Add some width to the PIP explainer box. r=#pip-reviewers!

With the new description strings added in bug 1774688, it's easier for the
explainer widget to run into the video controls. Making it wider should
make that problem less likely, and also make it read a bit better now.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b7fc1589f81
Add some width to the PIP explainer box. r=pip-reviewers,kpatenio

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/6b7fc1589f81

Comment 14

[Molly Howell (she/her) (no longer active)]

Comment on attachment 9284781 [details]
Bug 1777456 - Add some width to the PIP explainer box. r=#pip-reviewers!

Beta/Release Uplift Approval Request

• User impact if declined: See comment 0; picture-in-picture toggle explanatory text can run into the video controls, causing at least an ugly cosmetic issue and at worst a usability problem for either the video controls or our toggle.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small CSS-only patch; the patch shouldn't be able to cause any regressions that are any worse than the one it fixes.
• String changes made/needed:
• Is Android affected?: No

Comment 15

[Donal Meehan [:dmeehan]]

Comment on attachment 9284781 [details]
Bug 1777456 - Add some width to the PIP explainer box. r=#pip-reviewers!

Approved for 103.0b8, thanks.

Comment 16

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7e9798bbc908

Comment 17

[Peter Magyari (Desktop QA)]

The toggle is now wider, so it is less likely that it overlaps the player controls even on smaller resolutions and the text also reads better.
Verified on Beta 103.0b8 (20220712185923) and Nightly 104.0a1 (20220712215641)