1599061 - The browser window is focused when the Picture-in-Picture is closed on macOS

Status: REOPENED

(Core :: Widget: Cocoa, defect, P3)

Description

[Cristian Craciun]

Build ID 20191124230652
User Agent Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:72.0) Gecko/20100101 Firefox/72.0

Platforms:
macOS

Steps to reproduce:

1. Launch Firefox
2. Go to any video, such as youtube: https://www.youtube.com/watch?v=IoinmjCmM-E and start the video
3. Enable the PiP window
4. Focus another application, for example Finder
5. Close the PiP window by clicking on "X" button

Expected Result:
The PiP window is closed and the current application is still displayed.

Actual Result:
The PiP windows is closed and the Firefox browser is displayed.

Note:
The current tab is displayed when the PiP window is closed, not the tab with the video that was streamed.

Comment 1

[Rares Doghi, Desktop QA]

Hi, just a quick update on this issue, this still occurs in our latest Nightly, Beta and Release on MacOS devices.

Comment 2

[Gezim Hoxha]

I can verify that this is till a bug in 88.0 (64-bit).

Comment 3

[kpatenio]

I assumed that Bug 1756001 would resolve this issue, but it seems I'm still able to reproduce it on MacOS Big Sur, interestingly enough.

Comment 4

[av]

Hi there I would like to try working on this bug. Would anyone be able to guide me on how to get started?

Comment 5

[kpatenio]

(In reply to av from comment #4)

Hi there I would like to try working on this bug. Would anyone be able to guide me on how to get started?

Two areas worth investigating are:

• PictureInPicture.jsm: https://searchfox.org/mozilla-central/source/toolkit/components/pictureinpicture/PictureInPicture.jsm. closeSinglePipWindow is the method responsible for closing the pip window overall, and is called in other methods such as focusTabAndClosePip and receiveMessage. It receives messages from player.js.

• player.js: https://searchfox.org/mozilla-central/source/toolkit/components/pictureinpicture/content/player.js#382 - this is where we define behaviour when the close button is clicked. It sends messages to PictureInPicture.jsm.

I think those two areas are a good starting point. Both interact with each other via JSActors, and more information on how they communicate can be found in the PiP Firefox Source Docs. We also have docs for JSActors if you are curious.

Comment 6

[av]

Alright thanks. I will take a look.

Comment 7

[av]

Also want to add that this bug occurs on my Windows 10 device. It seems that this issue is independent of the OS.

Comment 8

[kpatenio]

When you tested on Windows 10, did you run a local Firefox build, or a specific release version? If a specific release version, then the fix is not shipped yet (in a couple days it will).

Comment 9

[av]

It was a specific release version. I did notice the nightly build did not have that issue as well. My last post can be ignored then. Thanks.

Comment 10

[av]

So after some further investigation, I was able to figure out why this bug is occurring. I am very certain that this is the cause.

The window closing behaviour we see in bug 1599061 is something built into the OS. This behaviour can be observed with the Finder application which further points to this being the case. This can be reproduced by following the steps below, similar to the issue with PIP:

1. Open an application such as the App Store.

2. Open two finder windows

3. Layer the windows in this order: finder window #1, app store, finder window #2

4. Close finder window #2. Notice how finder window #1 is brought into focus when we would expect the App Store to come into focus

I will take a look into the tab focusing issue next.

Comment 11

[av]

Additionally this post from 9 years ago: https://apple.stackexchange.com/questions/51030/how-do-i-give-focus-to-the-correct-window-when-closing-another-applications-win

mentions the same issue.

Comment 12

[av]

Alright, so I have come up with a working solution to fix the current tab being displayed when PIP window is closed, and not the tab with the video that was streamed. I will submit my proposed fix for review soon.

Comment 13

[av]

Attachment: Bug 1599061 - Browser window focused when Picture-in-Picture is closed on macOS. r=#pip-reviewers,kpatenio

Comment 14

[Markus Stange [:mstange]]

We can prevent the window reordering using the -[NSView shouldDelayWindowOrderingForEvent:] API. I have a patch.

Comment 15

[Markus Stange [:mstange]]

Attachment: Bug 1599061 - Don't reorder windows when clicking the Picture-in-picture window. r=#mac-reviewers

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3edfe1be6842
Don't reorder windows when clicking the Picture-in-picture window. r=mac-reviewers,spohl

Comment 17

[Atila Butkovits]

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

Comment 18

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

Should we ask the sheriffs to back this one out to fix bug 1766031?

Comment 19

[Markus Stange [:mstange]]

Yes, let's do that. I've asked the sheriffs.

Comment 20

[Atila Butkovits]

Backed out for causing bug 1766031.

Backout link: https://hg.mozilla.org/integration/autoland/rev/06259d4d9ec0db7b68128dab931db463f2adfb94

Comment 21

[Andreea Pavel [:apavel]]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/06259d4d9ec0db7b68128dab931db463f2adfb94

Comment 22

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

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:mstange, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.