The docshell/base/navigation mochitests fail on GTK3 builds due to GDK incorrectly skipping signaling focus out events on XInput2 when there is pointer focus (see upstream GTK bug https://bugzilla.gnome.org/show_bug.cgi?id=750244). A potential workaround would be to listen to GDK_WINDOW_STATE_FOCUSED changes from GDK_WINDOW_STATE events instead (introduced in GTK3).

This patch works around the hanging issue by using focus updates from window state updates on GTK3 (GTK2 does not provide focus updates in the window state signal). Thanks!

Comment on attachment 8613737 [details] [diff] [review] Use window state signal to set focus on GTK3. Some early testing has revealed this causes other bustage on try- going to look into it first.

Unfortunately, listening for focus with GDK_WINDOW_STATE_FOCUS doesn't work on WMs that don't support _NET_WM_STATE_FOCUSED (including the version of compiz on the tryserver)- we'll have to look for another workaround to get focus to work consistently.

This patch works around the issues with GDK's XInput2 event handler by translating XI2 GDK events ourselves. It fixes the issues with focus on try (https://treeherder.mozilla.org/#/jobs?repo=try&revision=0feb1d11d25b, note mochitest-1). More info can be found on this GTK bug: https://bugzilla.gnome.org/show_bug.cgi?id=677329 Unfortunately it doesn't seem that there's much we can do without interecepting the X events themselves. Thanks!

Updated to make translated events use the client pointer device to stop GDK complaints.

I've looked into this and have some insight into why this might be happening. When an X11 window hands off focus to an inferior window, the parent receives a FocusIn w/ NotifyAncestor event indicating that the child window will be focusing in. This is where we switch the parent's has_pointer_focus, allowing it to send the focus out event through GDK. For whatever reason, this event doesn't occur on the X11 Window's focus_window- only the toplevel. I suspect this should be propagating down to the focus window, but doesn't with XI2's focus events.

(In reply to Andrew Comminos [:acomminos] from comment #6) > I've looked into this and have some insight into why this might be happening. > > When an X11 window hands off focus to an inferior window, the parent > receives a FocusIn w/ NotifyAncestor event indicating that the child window > will be focusing in. Are you referring to focus_window with "inferior window"? Doesn't its parent get a FocusOut w/ NotifyInferior, or FocusIn with NotifyVirtual? > This is where we switch the parent's has_pointer_focus, > allowing it to send the focus out event through GDK. For whatever reason, > this event doesn't occur on the X11 Window's focus_window- only the > toplevel. The early return in _gdk_device_manager_core_handle_focus when (toplevel->focus_window == original) skips any handling of focus events on the focus_window. https://git.gnome.org/browse/gtk+/tree/gdk/x11/gdkdevicemanager-core-x11.c?id=3.16.5#n827

Comment on attachment 8614328 [details] [diff] [review] Implement custom handling for XInput2 events. Getting this kind of approach right would take some care. I'm concerned about changing what parts of GDK code see because that could mean that end up with unexpected consequences if/when the GDK code changes. If we can identify the cause of the bug and fix it, then we can look at this kind of workaround because we can limit it to past software where we know the internals. I'm not sure this bug is bad enough that it is worth a workaround as risky as this. We may be able to come up with a general approach that does not depend on GDK internals, such as consistently modifying all related events to trick GDK into thinking that the ancestor/root window never has focus. But first I'd like to know what the bug is that we are trying to work around. One observation of the patch here is that it still lets NotifyAncestor and NotifyVirtual and crossing events affect GDK internal state, including the setting of focus, but NotifyNonlinear and NotifyNonlinearVirtual no longer do, possibly leading to inconsistencies if the focus in and out paths are not symmetric. It also doesn't handle grabs in the same way as GDK. I don't think the device for focus is necessarily the client pointer. I would expect focus to be related to a master keyboard, which may be the master keyboard paired with the client pointer, but I'm not sure that it necessarily is. gdk_device_manager_get_client_pointer requires a round trip, which I'd like to avoid. Not directly related to this patch, but I don't know how multiple master keyboards each with their own focus are meant to be handled. I would guess that we are to either pick one master keyboards or show focus if any are currently providing focus. Does XSetInputFocus() select a single master device to change focus or all of them or none of them? If none of them, then that would explain why focus is not moving to the focus_window, and so pointer focus would remain until another app has focus.

Focus is handled in GDK by the same functions (handle_focus_change and _gdk_device_manager_core_handle_focus) regardless of whether the core or xi2 device manager is used, so it seems likely this is an X server bug. [1] and [2] seem to indicate that NotifyPointer events are missing with XI2. [3] points out that at least some NotifyPointer FocusIn events are received, but at least some NotifyPointer FocusOut events are missing. Are you able to reproduce the situation in [3] on a modern server including the patch in [4]? Use "xinput list" to identify the core keyboard. If it says "id=3" then use "xinput test-xi2 3" to skip being spammed with pointer events. I can't reproduce with kwin or metacity, which I assume is because focus never moves to the root window. Which window manager are you using? If you can't reproduce that, then can you reproduce any problems with Firefox on a modern server? If so, can you use xtrace [5] or modify attachment 519832 [details] to also log XI2 focus events, and record the series of events please? [1] https://bugzilla.gnome.org/show_bug.cgi?id=677329#c20 [2] https://bugzilla.gnome.org/show_bug.cgi?id=677329#c22 [3] https://bugzilla.gnome.org/show_bug.cgi?id=677329#c27 [4] http://lists.x.org/archives/xorg-devel/2013-February/035331.html [5] http://xtrace.alioth.debian.org/

I don't think Andrew updated his bugzilla e-mail, I'm going to contact him on the side.

Sorry for the delay in responding! I can reproduce the issue in [3] with gnome-shell 3.16 and Xserver 1.17.2. We do, in fact, only receive a NotifyPointer detail on the focus in event. I can also still reproduce this issue locally on Firefox trunk with using XInput2 (by unsetting GDK_USE_CORE_DEVICE_EVENTS in runtests.py) and running the plain mochitests in docshell/test/ *with the pointer inside the test harness window*. Curiously, the focus handling appears to work fine if the pointer is outside the window. I'll look into this some more and get some event logs.

I spoke with :jrmuizel about this on IRC and we agreed that disabling XI2 is a good choice for now to get GTK3 to ship due to bugs like bug 1182700 and bug 1196777. This patch uses gdk_disable_multidevice to turn off GDK's XI2 device manager entirely, falling back to the core device manager. Thanks!

Remove mochitest workaround.

Comment on attachment 8664548 [details] [diff] [review] Disable XInput2 by default on GTK3. Yes, I agree, and this is a nice way to do it, thanks!

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2f5aa2ece423

Looks like GTK 3.4 on the test infrastructure expects GLib to be initialized before calling gdk_display_get_default; let's skip that display check. It's possible to have a device manager initialized before the display anyway. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8c4593e4077

https://hg.mozilla.org/integration/mozilla-inbound/rev/e7017f284e77 https://hg.mozilla.org/integration/mozilla-inbound/rev/274691ea288e

https://hg.mozilla.org/mozilla-central/rev/e7017f284e77 https://hg.mozilla.org/mozilla-central/rev/274691ea288e

Comment on attachment 8664633 [details] [diff] [review] Disable XInput2 by default on GTK3. Carry r=karlt Approval Request Comment [Feature/regressing bug #]: GTK3 [User impact if declined]: bugs 1182700, 1170342 [Describe test coverage new/current, TreeHerder]: Testing on treeherder has already had XInput2 disabled [Risks and why]: Very low risk

I'm having trouble following the comments here, but mozregression pointed me to this bug. Is it possible that it broke smooth trackpad scrolling in Linux? I used to be able to scroll a few pixels at a time. Since the latest nightly I can only scroll in big jumps. It's not really a functionality issue, but it feels awful if you're used to the old behavior.

(In reply to Bill McCloskey (:billm) from comment #21) > I'm having trouble following the comments here, but mozregression pointed me > to this bug. Is it possible that it broke smooth trackpad scrolling in > Linux? I used to be able to scroll a few pixels at a time. Since the latest > nightly I can only scroll in big jumps. It's not really a functionality > issue, but it feels awful if you're used to the old behavior. Yes, breaking smooth trackpad scrolling is expected. We had some bugs with XInput2 and didn't want to block shipping GTK3 on them. Bug 1207973 adds an environment variable that will allow you to have it back until these bugs are dealt with.

Is this something we would ship on release? It seems like a pretty major regression from GTK2. It makes Firefox feel really sluggish on laptops. I expect to be able to directly manipulate the scroll position, and when it doesn't happen, it feels like jank to me (even though it isn't). Does the focus problem happen to just a subset of users? If so, is there a way we could detect those users and only then disable XInput2?

(In reply to Bill McCloskey (:billm) from comment #23) > Is this something we would ship on release? It seems like a pretty major > regression from GTK2. It makes Firefox feel really sluggish on laptops. I > expect to be able to directly manipulate the scroll position, and when it > doesn't happen, it feels like jank to me (even though it isn't). > > Does the focus problem happen to just a subset of users? If so, is there a > way we could detect those users and only then disable XInput2? My understanding was that this should not be a regression relative to GTK2. i.e. XInput2 is required for smooth scrolling, GTK2 doesn't support XInput2, and so I'd expect current GTK2 to not have smooth scrolling.

Wow, you're right, sorry. I guess I got used to the new scrolling behavior really quickly.

(In reply to Bill McCloskey (:billm) from comment #25) > Wow, you're right, sorry. I guess I got used to the new scrolling behavior > really quickly. You can enable it, see Bug 1207973 for details. Just set MOZ_USE_XINPUT2=1 env variable.

It might be good to add a release note for this as a known issue. Jeff, can you suggest wording for it if you think that's a good idea?

Comment on attachment 8664633 [details] [diff] [review] Disable XInput2 by default on GTK3. Carry r=karlt Approved for aurora uplift; we want scrolling to work

Comment on attachment 8664549 [details] [diff] [review] Don't disable XInput2 for mochitests on GTK3, off by default now. Uplift test changes for scrolling/gtk to aurora

GTK3 has been disabled in 42 (and never enabled in 41), not tracking

landed but seems there was a problem in marking the bug https://hg.mozilla.org/releases/mozilla-aurora/rev/e99258733708 https://hg.mozilla.org/releases/mozilla-aurora/rev/bdfe683a5e01

I've spent some time recently trying to dig through the X input stack; here's what I've found so far. - I've clarified that the problem is indeed missing FocusOut NotifyPointer events on the mochitest window after dialogs have been spawned. - When the pointer is in an inferior of the mochitest window, the pointer window should be notified regarding focus changes in the ancestor as per https://tronche.com/gui/x/xlib/events/input-focus/normal-and-grabbed.html. - A NotifyPointer focus out event on a window is a precondition for losing 'GDK focus'- observe the HAS_FOCUS macro in GDK's X11 core device manager. A change in this is what gets reported to Gecko. - The issue is most easily observable by logging GDK's events (with GDK_DEBUG=events) when running docshell/test/navigation/test_bug270414.html. - When running with XInput2 (MOZ_USE_XINPUT2=1), we are clearly missing three events on the mochitest window when comparing without XI2 and GDK_DEBUG=events. - Debugging without XInput2 reveals that these are indeed the focus out events we were missing. Something that caught my eye is the following comment in `dix/enterleave.c`; > NotifyPointer events may be sent if the focus changes from window A to > B. The assumption used in this model is that NotifyPointer events are only > sent for the pointer paired with the keyboard that is involved in the focus > events. For example, if F(W) changes because of keyboard 2, then > NotifyPointer events are only sent for pointer 2. This leads me to believe that perhaps when the newly created window steals focus, it's not using the correct master keyboard such that the pointer window is notified. I need to investigate this more, however.

I believe I've finally discovered the root issue here; a bug in Xorg has been filed to fix NotifyPointer not sending focus out events to the pointer window. Details are on the freedesktop bugzilla; https://bugs.freedesktop.org/show_bug.cgi?id=93539 I'll be submitting a patch upstream soon, but we should still consider working around this in Firefox by potentially blocking NotifyPointer events from reaching GTK's input handling system.

https://git.gnome.org/browse/gtk+/commit/?id=d55b815 In https://git.gnome.org/browse/gtk+/log/gdk/x11/gdkeventsource.c?h=3.19.9 not in https://git.gnome.org/browse/gtk+/log/gdk/x11/gdkeventsource.c?h=3.19.8 https://git.gnome.org/browse/gtk+/log/gdk/x11/gdkeventsource.c?h=3.18.6

https://cgit.freedesktop.org/xorg/xserver/commit/?id=2fbf5c2f91d33efbda573c4be036248b1d8ed7f1 should fix this upstream. We should aim to backport this patch to the version of X running on our test machines.

FYI, Xorg issue 93539 was fixed last year and the new Xorg 1.19 build should include that patch (not sure if it was also backported into 1.18). Considering Ubuntu as a test target, it is not clear whether Xorg 1.19 will make it into the 17.04 release in two weeks, but it is available now from this PPA: https://launchpad.net/~canonical-x/+archive/ubuntu/x-staging

The latest 2 Ubuntu version (17.10 and 18.04 LTS) are using Xorg 1.19.2, which should have included the patch to fix the issue. On the other hand, it is unclear if older system will ever get a patch on this. When would this issue be considered resolved?