Closed Bug 1042542 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: General, defect)

34 Branch
All
Linux
defect
Not set
Points:
3

Tracking

()

RESOLVED WORKSFORME
Iteration:
34.3

People

(Reporter: bogdan_maris, Assigned: enndeakin)

References

()

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Flags: firefox-backlog+
Blocks: 1040061
(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.
Whiteboard: [sceensharing-uplift]
Depends on: 1045000
Points: --- → 3
QA Whiteboard: [qa+]
Attached patch rolluponminimize (obsolete) — Splinter Review
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.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Added to Iteration 34.2
Iteration: --- → 34.2
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.
(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?
Attachment #8470877 - Flags: review?(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.
Attachment #8470877 - Flags: review?(karlt) → review+
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.
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.
That sounds right.
Also the likely cause of a big spike in bug 968208.
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.
Attachment #8470877 - Attachment is obsolete: true
Attachment #8475197 - Flags: review?(karlt)
Iteration: 34.2 → 34.3
QA Whiteboard: [qa+]
Flags: qe-verify+
(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.
Flags: needinfo?(enndeakin)
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.
Flags: needinfo?(enndeakin)
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?
Flags: needinfo?(enndeakin)
Flags: needinfo?(bogdan.maris)
(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?
Flags: needinfo?(bogdan.maris) → needinfo?(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.
Flags: needinfo?(karlt)
Flags: needinfo?(enndeakin)
OS: All → Linux
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
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.
Attachment #8475197 - Flags: review?(karlt) → review-
Whiteboard: [sceensharing-uplift]
You need to log in before you can comment on or make changes to this bug.