1757921 - The thumbnail preview videos on Youtube have a PiP button and can be launched

Status: VERIFIED FIXED

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

Description

[Daniel Bodea [:danibodea]]

Note

• When the user loads Youtube, hover over a video thumbnail and the preview starts, a PiP button gets displayed. When clicked, a PiP window will incorrectly be launched and remain open and blank. This behavior appears intermittent with: sometimes the PiP button click will get counted as thumbnail click and Youtube will redirect to the video AFTER a flicker of the PiP window opening and closing. In any case, the thumbnail preview videos should not have a PiP button.

Affected versions

• Nightly v99.0a1

Affected platforms

• all

Steps to reproduce

1. Launch browser.
2. Load a Youtube.com
3. Hover over a video thumbnail until the preview video is displayed
4. Click the PiP button on the video thumbnail

Expected result

• The thumbnail preview videos should not have a PiP button.

Actual result

• When clicked, a PiP window will incorrectly be launched and remain open and blank. This behavior appears intermittent with: sometimes the PiP button click will get counted as thumbnail click and Youtube will redirect to the video and a flicker of the PiP window opening and closing.

Regression range

• Not a recent regression.

Additional notes
*

Comment 1

[kpatenio]

The easiest option is update the policy for youtube.com in picture_in_picture_overrides.js to ensure the pip toggle never appears. Alternatively, if we're only concerned with the thumbnail preview videos themselves, we can maybe try detecting if a video is a descendent of an element with class .ytd-video-preview.

Ex. do not show pip toggle if video.closest(".ytd-video-preview") returns an element when hovering mouse over a video.

We could even add a reference to an element id/class (that we should ignore) when we define our docState, which would then be assigned like we do here for other references. (we would need to define some property like elementAttribute in picture_in_picture_overrides.js first)

Comment 2

[Molly Howell (she/her) (mostly inactive)]

Just for the record, the options in comment 1 are listed in order of increasing complexity (probably 1 point, 2 points, and 5 points, respectively), but also in order of increasing completeness/generality/reusability.

My only concern with the first/easiest option would be its future reliability; the video previews themselves kind of sneaked up on us overnight, and that makes me a little nervous about other changes that might happen to the homepage that could end up invalidating this solution.

The middle option, of branching off of the presence of .ytd-video-preview would be more stable and I think sounds like a great idea.

I wouldn't advocate for the last, most involved option just because it's hard to see the necessity unless we identify another site that would also need this treatment (i.e., has some video elements that should not get toggles for a specific, contextual reason).

Comment 3

[Ania]

I agree with Molly about the concern over the long-term reliability of the first solution.

Comment 4

[kpatenio]

Attachment: Bug 1757921 - hide pip toggle for youtube video previews. r=#pip-reviewers!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85e3b2e53e59
hide pip toggle for youtube video previews. r=mtigley

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/85e3b2e53e59

Comment 7

[Daniel Bodea [:danibodea]]

I have verified this fix on Beta v100.0b5 and Nightly v101.0a1 on Windows 10, Mac OS 11 and Ubuntu 20. Netflix no longer shows the issue described in comment 0.