1587362 - PiP button doesn't work after putting tab back in window

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect, P2)

Description

[Valentin Gosu [:valentin] (he/him)]

I found this bug on Linux, but it reproduces on Windows too. Seems to happen every time.

STR:

1. Open youtube video in a new tab - start playing it.
2. Drag tab out of the tab list, so it opens in a new window
3. Drag youtube tab back into the original window
4. Try to click PiP button - Doesn't work. (it clicks through it, pausing the video, instead of popping out).

Comment 1

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

Attachment: Bug 1587362 - Make sure PiP toggle mouse button listeners are attached to the right WindowRoot after tab tear out / in. r?JSON_Voorhees

It seems that after doing a tab tear out, the WindowRoot ends up changing, so
the mouse button listeners for the toggle end up being attached to the wrong
thing. This patch makes it so that we monitor for tear out/in, and unattach the
mouse button listeners, and then re-attach them on the new WindowRoot after
the tab tear out/in completes.

Comment 2

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

Attachment: Bug 1587362 - Regression test. r?JSON_Voorhees

Depends on D48885

Comment 3

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

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2b259acdcbb8918f689974fba4cb4aff8d02c18

Comment 4

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

New try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1462e0f7fb8130cd4a8c2ccc3d7d11e4d28fe6a6

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c71f9a59bdfe
Make sure PiP toggle mouse button listeners are attached to the right WindowRoot after tab tear out / in. r=jaws
https://hg.mozilla.org/integration/autoland/rev/d3142d807c7f
Regression test. r=jaws

Comment 6

[Stefan Hindli [:stefan_hindli]]

Backed out 2 changesets (Bug 1587362) for causing very frequent bc failures in toolkit/mozapps/extensions/test/browser/browser_about_debugging_link.js CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=a3d7e24bd090c093fc9822a83402f7c3502a4d5b&selectedJob=270960385

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=270960385&repo=autoland&lineNumber=13817

Comment 7

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

Here's a try push showing all green with just the fix (and not the test) landed:

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=270981716&revision=56963bded2a4f2604efce19d832ea02d364d65cf

I'll re-land the fix, and examine why the test may have caused the spike in oranges.

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/1102d533350a

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c6ab26ee4cf
Backed out 2 changesets for causing very frequent bc failures in toolkit/mozapps/extensions/test/browser/browser_about_debugging_link.js CLOSED TREE

Comment 10

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

Hi stefan_hindli,

I'm curious, why did you back me out? In comment 7, I showed a try push that demonstrates that the one patch I landed does not seem to cause the orange to recur. What happened?

Comment 11

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

Oops, nevermind - looking at the revision linked to from comment 9, I think pulsebot is referring to the original backout that happened at comment 6. I guess it was just a little late.

Sorry!

Comment 12

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

I suspect with bug 1576472 fixed, the test can land now.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae63a4dc188d50ff7464b0aef9ebd07a6839c626

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f24ddb55b48b
Regression test. r=jaws

Comment 14

[Cosmin Sabou [:CosminS]]

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

Comment 15

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello! Reproduced the issue with 71.0a1 (20191009213914) on Windows 10x64 after following the STR from comment 0.
The issue is verified fixed with 71.0b11 (20191118154140) on Windows 10x64, Ubuntu 18.04 and macOS 10.15. After following STR from comment 0 the PiP window is correctly opened.