1042542 - screen/window/audio sharing doorhanger pop-ups have strange behavior after minimizing Firefox

Status: RESOLVED WORKSFORME

(Firefox :: General, defect)

Description

[Bogdan Maris, Desktop QA]

Latest Nightly 34.0a1 (buildID: 20140722030201)
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:34.0) Gecko/20100101 Firefox/34.0

STR:
1. Open Firefox
2. Set media.getusermedia.screensharing.enabled to true
3. Open http://queze.net/goinfre/ff_gum_test.htm in a new tab
4. Click Video and select Share Selected Device.
5. Click on camera icon from Global Indicator
6. Minimize Firefox

Expected result: Camera pop-up minimizes with Firefox

Actual result: The pop-up remains on screen and Firefox is minimized. On Windows the pop-up stays with Firefox and 

Notes:
1. This is Linux only.
2. On Windows and Mac the issue is a bit different
Windows: after step 6, repeat step 5. The pop-up is still there and it appears faster then Firefox.
Mac: After step 6 the pop-up can be seen on screen for 1 second and then it goes away. If you repeat step 5 Firefox appears, then it goes away for 1 second and reappears.

Comment 1

[Florian Quèze [:florian]]

This sounds like we should dismiss the doorhanger automatically when the window is minimized (do we have an event fired reliably when a window is minimized?) and wait for the window to no longer be minimized if we are raising it before reshowing the doorhanger.

Comment 2

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #1)
> This sounds like we should dismiss the doorhanger automatically when the
> window is minimized (do we have an event fired reliably when a window is
> minimized?)

Yes, we do. The sizemodechange event is fired, and window.windowState has STATE_MINIMIZED.

Comment 3

[Neil Deakin]

Attachment: rolluponminimize

This is actually two different issues.

On Linux, the issue is that we don't rollup popups on window state change notifications. Now normally this is ok, because when a window is minimized, a focus out notification is also sent which causes the popup to rollup. However, in this case, the window is in the background so naturally this notification isn't sent. The simplest solution then is call CheckForRollup within nsWindow::OnWindowStateEvent.

Note that on Windows and Mac, all popups rollup unconditionally even if they belong to other windows, so this patch does the same.

Also note that this affects all popups, not just this specific popup.

Comment 4

[Marco Mucci [:MarcoM]]

Added to Iteration 34.2

Comment 5

[Neil Deakin]

On Mac, the issue isn't specific to the arrow panel; it affects others as well. The panel flickers on again briefly when the hiding transition ends. One solution is to not perform the hiding transition when minimizing.

I filed bug 1051902 on the Mac issue.

I can't see the specific issue on Windows, but for me enabling sharing slows the system down considerably so it's hard to tell.

Comment 6

[Florian Quèze [:florian]]

(In reply to Neil Deakin from comment #5)

> I can't see the specific issue on Windows, but for me enabling sharing slows
> the system down considerably so it's hard to tell.

Even if you share only the microphone?

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8470877 [details] [diff] [review]
rolluponminimize

I can't think of any problems with this but I don't understand why it is necessary.

On my system the camera is not available so I try to reproduce with the microphone.

After
5. Click on microphone icon from Global Indicator

The browser window has focus, the popup is shown, and the pointer is grabbed.
It is not possible to minimize with the mouse before clicking to remove the grab and dismiss the popup.  Minimizing with the keyboard causes the expected focus-out and the popup is dismissed.

The popup should not be shown if the pointer can't be grabbed and the pointer shouldn't be grabbed if the browser doesn't have focus.  If the pointer is not grabbed and the browser doesn't have focus, then there will be other situations where the popup will not be dismissed: clicking outside the browser app, closing the browser window ...

It seems there is focus bug here, and this is working around one symptom of the bug.  Could the focus bug already be fixed?  Or perhaps it is just that focus works differently under different window managers.

Comment 8

[Neil Deakin]

In this case the browser is focused, just a different window than that showing the popup. I'll try testing again just to made sure everything makes sense here.

Comment 9

[Karl Tomlinson (:karlt)]

Here, without any patches:

1) If another app has focus when clicking the microphone icon on the global indicator, the browser window showing ff_gum_test.html is focused and shows the popup.

2) If the browser window showing ff_gum_test.html has focus at the time of the click, then the popup is shown, and focus does not change.

3) If another window for the same browser process has focus at the time of the click, then the browser window showing ff_gum_test.html is focused, but the popup is not shown.  A second click on the global microphone icon shows the popup, with no focus change.

Comment 10

[Neil Deakin]

That sounds right.

Comment 11

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dd013a36e5d9

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Backed out for Linux mochitest-dt failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/04c1a7c2b578

https://tbpl.mozilla.org/php/getParsedLog.php?id=46055803&tree=Mozilla-Inbound

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Also the likely cause of a big spike in bug 968208.

Comment 14

[Neil Deakin]

Attachment: Only do this for toplevel windows

For some reason, popups are receiving GDK_WINDOW_STATE_ICONIFIED state change notifications, sometimes two in a row, so I changed this to only rollup for toplevel and dialog windows.

Comment 15

[Karl Tomlinson (:karlt)]

(In reply to Neil Deakin from comment #10)
> That sounds right.

What do you observe on your system, in those situations, without any patch?

Can you see what has focus?

What is the effect of clicks outside the browser while the popup is shown,
before the browser window is minimized?

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #0)
> 5. Click on camera icon from Global Indicator
> 6. Minimize Firefox

How is the browser window minimized?

If the popup shows in step 5, then it should not be possible to minimize
firefox with the mouse because the first click will dismiss the popup.

If the keyboard can minimize firefox, then I assume the browser has focus.

(In reply to Neil Deakin from comment #14)
> For some reason, popups are receiving GDK_WINDOW_STATE_ICONIFIED state
> change notifications, sometimes two in a row, so I changed this to only
> rollup for toplevel and dialog windows.

That could happen if SetSizeMode(nsSizeMode_Minimized) is called on a popup,
even if while hidden.

It's also possible that the window manager is sending some unexpected
messages.

toplevel and dialog is at least consistent with OnConfigureEvent.

Comment 16

[Neil Deakin]

With or without the patch, I get the same results as in comment 9.

Point 3 should show the popup, but doesn't as the window isn't focused. But that's a different bug.

Comment 17

[Karl Tomlinson (:karlt)]

I don't understand what https://bugzilla.mozilla.org/attachment.cgi?id=8475197 is fixing then.

Is Neil not able to reproduce the bug?

Or does the camera icon behave differently from the microphone icon?
If so, what is the camera button doing differently?

If not, how do you get to a situation where the patch changes behavior?

Bogdan, can you retest with the latest nightly, please, and answer the questions in comment 15?

Comment 18

[Bogdan Maris, Desktop QA]

(In reply to Karl Tomlinson (:karlt) from comment #15)
> (In reply to Neil Deakin from comment #10)
> > That sounds right.
> 
> What do you observe on your system, in those situations, without any patch?
> 
> Can you see what has focus?
> 
> What is the effect of clicks outside the browser while the popup is shown,
> before the browser window is minimized?
> 

(In reply to Karl Tomlinson (:karlt) from comment #15)
> (In reply to Bogdan Maris, QA [:bogdan_maris] from comment #0)
> > 5. Click on camera icon from Global Indicator
> > 6. Minimize Firefox
> 
> How is the browser window minimized?
> 
> If the popup shows in step 5, then it should not be possible to minimize
> firefox with the mouse because the first click will dismiss the popup.

Linux:
Using Nightly build from 2014-07-23 it was possible to minimize Firefox from (-) button while a pop-up was shown, on the latest Nightly 2014-08-20 this thing is not possible anymore (something has changed). If I click the (-) button the pop-up disappears, same behavior if minimizing Firefox using keyboard shortcut. If I click another app the pop-up disappears (right behavior).

On Windows and Mac, the behaviors are the same as explained in comment 0. For Mac i see that Neil logged bug 1051902, should I log a separate bug for Windows or will be fixed in this one?

Comment 19

[Karl Tomlinson (:karlt)]

(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #18)
> Linux:
> Using Nightly build from 2014-07-23 it was possible to minimize Firefox from
> (-) button while a pop-up was shown, on the latest Nightly 2014-08-20 this
> thing is not possible anymore (something has changed). If I click the (-)
> button the pop-up disappears, same behavior if minimizing Firefox using
> keyboard shortcut. If I click another app the pop-up disappears (right
> behavior).

Thanks, Bogdan.  Most of the discussion in this bug has been about the Linux behavior, which seems to be fixed now, so let's resolve this bug WFM.

> On Windows and Mac, the behaviors are the same as explained in comment 0.
> For Mac i see that Neil logged bug 1051902, should I log a separate bug for
> Windows or will be fixed in this one?

Yes, please log a separate bug for Windows.  All the comments about Linux behavior would just confuse analysis of the Windows issue.

Comment 20

[Karl Tomlinson (:karlt)]

Comment on attachment 8475197 [details] [diff] [review]
Only do this for toplevel windows

This shouldn't be necessary, as rollup on grab failure should handle this.
And this comes with some risk of effects from changes in unrelated windows, so it is not worth taking as a safety net.