1412130 - Permission panels stay visible on top of non-Firefox windows

Status: RESOLVED WONTFIX

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

Description

[Florian Quèze [:florian]]

Steps to reproduce:
1. Open a permission prompt (eg. go to https://appear.in/five-quelea and click the "Allow cam/mic access" button).
2. Press Cmd+tab to select another application (eg. the terminal).

Expected result:
The Firefox window is now under the other window.

Actual result:
The Firefox window is now under the other window, but the permission prompt is still visible on top.

This is a recent regression in Nightly, mozregression says:

Last good revision: ac5d290791da28e9aaaefcded47f6967fe04ea40
First bad revision: 9adb0bc561c3c5d16f8c31640a4ff2570feb2cf3
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ac5d290791da28e9aaaefcded47f6967fe04ea40&tochange=9adb0bc561c3c5d16f8c31640a4ff2570feb2cf3 --> bug 1406032

In case this matters, I tested this on OS X 10.12.6 (16G29)

[Tracking Requested - why for this release]:
Regression uplifted to 57 three days ago.

Comment 1

[Julien Cristau [:jcristau]]

Seems worth getting to the bottom of this before it ships.

Comment 2

[Mike Taylor [:miketaylr]]

Just found this bug via mozregression.

Here's a dumb page to test this with that only calls Notification.requestPermission(): https://miketaylr.com/bzla/perm.html

Comment 3

[Stephen A Pohl [:spohl] (OOO until 2/27)]

Attachment: Patch

Will jot down some thoughts about this patch inline with the code in a minute.

Comment 4

[Stephen A Pohl [:spohl] (OOO until 2/27)]

Comment on attachment 8922962 [details] [diff] [review]
Patch

Review of attachment 8922962 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/cocoa/nsCocoaWindow.mm
@@ +891,2 @@
>        }
> +      SetPopupWindowLevel();

We should call SetPopupWindowLevel() here rather than setting the window level directly.

@@ -1376,5 @@
>    // Reparent child windows.
>    enumerator = [childWindows objectEnumerator];
>    while ((child = [enumerator nextObject])) {
>      [mWindow addChildWindow:child ordered:NSWindowAbove];
> -    [mWindow setLevel:NSPopUpMenuWindowLevel];

I added this code because this was the only other place in nsCocoaWindow where we added a child window with an order of NSWindowAbove. However, I've now realized that I haven't been able to hit this code during my testing. It also doesn't seem to be restricted to popups. So we should revert this until we know for sure that it's needed.

@@ +2498,2 @@
>    }
> +  [mWindow setHidesOnDeactivate:YES];

We need to always hide popups when we deactivate the app or we will keep showing the permission panel when we switch to another app now that we're setting the popup's window level to NSPopUpMenuWindowLevel. I can't think of any situation where we wouldn't want to hide a popup window when the app becomes inactive, as the code previously did. Please let me know if I missed something here.

Comment 5

[Markus Stange [:mstange]]

(In reply to Stephen A Pohl [:spohl] from comment #4)
> I can't think of
> any situation where we wouldn't want to hide a popup window when the app
> becomes inactive, as the code previously did. Please let me know if I missed
> something here.

The toolbar customization panel in Thunderbird / Seamonkey comes to mind. I can't think of a case in Firefox where we use a panel in a similar context.

Comment 6

[Markus Stange [:mstange]]

There's also the noautohide XUL attribute. I don't know if that's set on these arrow panels; it's probably set on the customization panel in Thunderbird. I also don't know how the value of that attribute makes it into widget land.

Comment 7

[Stephen A Pohl [:spohl] (OOO until 2/27)]

I'm not familiar with the Thunderbird / Seamonkey toolbar customization panels and I can't seem to find it in Thunderbird right now. But all that aside, should such a panel really keep showing on top of any other apps when the original app (Thunderbird or Seamonkey) may be completely hidden from view because another app is displayed in front of it?

Comment 8

[Markus Stange [:mstange]]

In Thunderbird, right click the toolbar and choose "Customize...". A panel will appear which looks like a sheet window but is actually a panel.
No, this panel should not be visible in front of other apps. It should stick close to its parent window in the z-order.

Comment 9

[Stephen A Pohl [:spohl] (OOO until 2/27)]

(In reply to Markus Stange [:mstange] from comment #8)
> No, this panel should not be visible in front of other apps. It should stick
> close to its parent window in the z-order.

Okay, great! I think this patch should do the trick then.

Comment 10

[Stephen A Pohl [:spohl] (OOO until 2/27)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9152b22a5482b7c0c3dfa7dab5ab1e81b7a17e87

Comment 11

[Stephen A Pohl [:spohl] (OOO until 2/27)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4df790d1477feb77c5ec92102ba23715b2d65cf
Bug 1412130: Fix window levels on macOS to ensure that popups hide when the application is deactivated. r=mstange

Comment 12

[Eliza Balazs [:ebalazs_]]

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

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this ASAP.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Backed out at Stephen's request.

https://hg.mozilla.org/mozilla-central/rev/29d166cfe2752f6977f76f658be8259a48fc7e8d

Comment 15

[Florian Quèze [:florian]]

The backout commit message announces a better fix, where can I follow progress on it if this bug is wontfix?

Comment 16

[Stephen A Pohl [:spohl] (OOO until 2/27)]

This bug here was fixed by a backout of bug 1406032. The better fix will come in the form of better handling of opening new windows when in fullscreen. I expect this to land as part of bug 1405577.