578265 - Fullscreen mode should not exit on FocusOut events with NotifyGrab set; triggers on hardware volume or brightness keys

Status: RESOLVED WORKSFORME

(Core :: Widget: Gtk, defect)

Description

[Jake]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:2.0b1) Gecko/20100630 Firefox/4.0b1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:2.0b1) Gecko/20100630 Firefox/4.0b1

After opening a <video> in fullscreen, when using integrated volume knob (ie. on keyboard), fullscreen closes and video resumes in window  (Ubuntu 10.04/Firefox 4b1)

Reproducible: Always

Steps to Reproduce:
1.Start  playing video
2.Open fullscreen by right-clicking and clicking 'Full Screen'
3.Use keyboard volume knob to adjust volume

Actual Results:  
Fullscreen closes and returns to window

Expected Results:  
Fullscreen remains open with volume adjusted

This bug is specific to Ubuntu, and may have something to do with the volume graphic popping up in GNOME NotifyOSD (the volume widget that appears in the upper-right corner)

Comment 1

[Stephan Sokolow]

Attachment: Testcase based on python-xlib

This also happens with my volume knob on LXDE powered by Audacious Media Player and my keybindings (the ones which don't create or show new windows) powered by xbindkeys.

It seems that the problem is Firefox killing Fullscreen mode for videos on detection of any input event not directed to it. (In this case, keypresses reserved by XGrabKey)

I don't have time to determine how Firefox actually decides what a noteworthy event is, but I've attached a testcase based on python-xlib which demonstrates the issue using the keybinding Ctrl+Super+Spacebar.

Comment 2

[Josh Triplett]

I just checked with xev, and the hardware volume keys do not result in any key event delivered to Firefox.  However, hitting the volume keys causes a window to pop up showing the volume, and the current window receives a couple of FocusOut events followed by a FocusIn event.  Firefox automatically un-fullscreens when it loses focus.

The same behavior occurs for hardware brightness keys, or anything else that causes a window to pop up.  I checked with GNOME2, and KiBi on #xorg-devel confirmed that the same behavior occurs with GNOME3.  I suspect that LXDE, Unity, and other desktop environments have the same behavior.

I'd actually suggest that a more sensible fix would involve changing the volume display window and similar on-screen display popups to not take focus, since they don't need to accept input.  I don't see any way that Firefox can distinguish between losing focus to the volume display window and losing focus due to Alt-Tab or some other reason that should cause un-fullscreening.

I've marked this Firefox bug as INVALID, because I don't think Firefox can fix it.  I've filed https://bugzilla.gnome.org/show_bug.cgi?id=667110 on gnome-settings-daemon to get this fixed in GNOME 3.  Users of other desktop environments should file similar bugs.

Comment 3

[Josh Triplett]

OK, after further exploration and some discussion on #xorg-devel, it turns out that this bug really does need fixing in Firefox, not in gnome-settings-daemon and similar programs that listen for keys.

The FocusOut event occurs as an inherent part of grabbing a keyboard key.  gnome-settings-daemon or anything else listening to specific keys such as the hardware volume/brightness keys has to register a passive grab on those keys.  A passive grab causes X to start an active grab when the grabbed key occurs.  That active grab generates a FocusOut event to the currently focused window, and then the end of the grab generates a FocusIn event.

However, the FocusOut event actually indicates the nature of the lost focus, in the .mode field.  When a grab occurs, the FocusOut event will have NotifyGrab set.  Other types of FocusOut events have NotifyNormal (a normal focus change) or NotifyWhileGrabbed (focus changed while under a grab, so the window won't get it back when the grab ends; occurs in Alt-Tab for instance).

xterm, as usual, gets this right.  It shows its cursor as hollow when it loses focus, and solid when it has focus.  Hitting a grabbed key such as the hardware volume/brightness keys or Alt-Tab does not cause xterm to show the hollow cursor, but actually switching to another window (with Alt-Tab or by clicking on the other window) does.  Looking at the source to xterm, in misc.c, in the FocusOut case of HandleFocusChange:

	/*
	 * XGrabKeyboard() will generate NotifyGrab event that we want to
	 * ignore.
	 */
	if (event->mode != NotifyGrab) {
	    unselectwindow(screen,
			   ((event->detail == NotifyPointer)
			    ? INWINDOW
			    : FOCUS));
	}


I think Firefox needs to apply the same logic: ignore FocusOut events if they have NotifyGrab set.  Any actual focus change that Firefox cares about will occur with one of the other flags set.  NotifyGrab just means some other program just captured a magic hotkey that Firefox shouldn't care about; that shouldn't cause un-fullscreening.

Unfortunately, GDK doesn't currently expose the .mode field of FocusOut events in its GdkEventFocus structure.  So, handling this case requires touching the X event directly.  Firefox already does this for several other types of events.  The simplest fix seems a GDK event filter (gdk_window_add_filter) which returns GDK_FILTER_REMOVE for any FocusOut with mode NotifyGrab.

I've discussed this issue with people in the Xorg and GTK+ communities, who also plan to look into the broader issue with other GTK+ applications getting this wrong; for instance, unlike xterm, gnome-terminal shows its hollow not-focused cursor momentarily when pressing grabbed keys.  A future GDK may start translating the .mode field of the FocusOut event as part of GdkEventFocus, and GTK+ may change its generation of focus-out-event to handle NotifyGrab differently.  However, I still think it makes sense to go ahead and fix this in Firefox.

Comment 4

[alexhultman]

Adobe Flash player had a similar bug for years that made it exit fullscreen when changing volume because of the appearing notification. If I watch HTML5 video in fullscreen in Firefox 10b3 and change volume, it too exits fullscreen.

Comment 5

[Josh Triplett]

(In reply to alexhultman from comment #4)
> Adobe Flash player had a similar bug for years that made it exit fullscreen
> when changing volume because of the appearing notification. If I watch HTML5
> video in fullscreen in Firefox 10b3 and change volume, it too exits
> fullscreen.

In this case, though, Firefox exits because of the focus change, not the notification window.  That focus change happens independently of the notification window.

Comment 6

[Constantine]

Why does Firefox need to exit fullscreen mode when the focus is lost? I think it is a bad idea because of many reasons.
1. People with multimonitor configurations can't watch a video in fullscreen and work on another monitor at the same time.
2. There are some problems with popups.
3. (annoying for me) When I switch to another desktop with compiz or minimize window to do some little task focus is lost and I need to return to fullscreen again and again...

Comment 8

[Pawel Chmielowski]

Attachment: Add filter removing offending events

This patch adds extra filtering function, which drops processing of grab notifications events.

Comment 10

[Karl Tomlinson (:karlt)]

Comment on attachment 652764 [details] [diff] [review]
Add filter removing offending events

Thanks very much for everyone's analysis here.

Bear in mind that GrabKeyboard can also cause a FocusOut event with NotifyGrab
mode.  While passive grabs are typically short, GrabKeyboard would normally be
for longer.

I see that Flash remains fullscreen after FocusOut with NotifyGrab, but it
does exit fullscreen on a subsequent FocusOut with NotifyWhileGrabbed.

Unfortunately, we can't just ignore NotifyGrab because GDK will ignore NotifyWhileGrabbed and we won't get a FocusOut event if focus changes during the grab.

Perhaps this approach could be adjusted to synthesize a NotifyGrab FocusOut
when one with NotifyWhileGrabbed is received, but I fear that the change in
focus behaviour could confuse GTK widgets and menus that use gdk_keyboard_grab
and may expect the grab window to have focus.  These are used only in dialogs
that Gecko opens.

My preferred fix here would be to fix bug 724554 by not exiting fullscreen on
focus out.  (Even if a message is shown on focus in, that need not require
explicit acceptance of fullscreen again and so could be transient, or it could even be shown only if focus is lost for some period of time.)

If that doesn't work out, I'd prefer a timeout in browser-fullScreen.js, so
that it only exits fullscreen if focus is lost for some period of time.

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Karl Tomlinson (:karlt) from comment #10)
> My preferred fix here would be to fix bug 724554 by not exiting fullscreen on
> focus out.  (Even if a message is shown on focus in, that need not require
> explicit acceptance of fullscreen again and so could be transient, or it
> could even be shown only if focus is lost for some period of time.)

Not exiting fullscreen upon blur and showing the "$domain is now fullscreen" warning for a short period of time when when focus returns to a fullscreen window seems like an acceptable compromise to me. I've gotten a lot of feedback from users that they want to be able to blur a fullscreen window (primarily use case is watching a fullscreen video while doing something else in another window/app), so I think this is a good idea.

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #11)
> (In reply to Karl Tomlinson (:karlt) from comment #10)
> > My preferred fix here would be to fix bug 724554 by not exiting fullscreen on
> > focus out.  (Even if a message is shown on focus in, that need not require
> > explicit acceptance of fullscreen again and so could be transient, or it
> > could even be shown only if focus is lost for some period of time.)
> 
> Not exiting fullscreen upon blur and showing the "$domain is now fullscreen"
> warning for a short period of time when when focus returns to a fullscreen
> window seems like an acceptable compromise to me. I've gotten a lot of
> feedback from users that they want to be able to blur a fullscreen window
> (primarily use case is watching a fullscreen video while doing something
> else in another window/app), so I think this is a good idea.

I posted to mozilla.dev.security to request feedback from the security guys about changing to not exit fullscreen on blur, and show a warning on re-focus of fullscreen windows:

http://groups.google.com/group/mozilla.dev.security/browse_thread/thread/bcb45e15657af746#

I'll see if there's any opposition from the security guys, otherwise I will move ahead with making this change.

Comment 17

[nortexoid]

Has this issue been fixed post Firefox 20?

Comment 18

[Hubert Figuiere [:hub]]

It works fine in Nightly now, which is Firefox 23.

Comment 19

[Josh Triplett]

It's mildly annoying that changing the volume or brightness with dedicated hardware keys causes Firefox to show the "site is now fullscreen, ESC to exit" warning, but it's no longer a serious problem, since the site remains fullscreen.

Comment 20

[Daniel Holbert [:dholbert]]

Note: bug 1209374 covers the symptom described in comment 19. As xidorn just noted over there (bug 1209374 comment 2), that symptom could be fixed either by fixing this bug here, or by fixing bug 1196588.

Comment 22

[Tracy Walker [:tracy]]

(In reply to Josh Triplett from comment #19)
> It's mildly annoying that changing the volume or brightness with dedicated
> hardware keys causes Firefox to show the "site is now fullscreen, ESC to
> exit" warning, but it's no longer a serious problem, since the site remains
> fullscreen.

wfm per that