After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 752000 - GtkApplication vs. _NET_WM_USER_TIME
GtkApplication vs. _NET_WM_USER_TIME
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-07-06 00:06 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2015-07-15 01:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gapplication: terrible launching hacks (3.67 KB, patch)
2015-07-06 00:06 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
gtkapplication: terrible launching hacks (3.02 KB, patch)
2015-07-06 00:07 UTC, Jasper St. Pierre (not reading bugmail)
none Details | Review
GtkApplication: avoid using stale timestamps (4.80 KB, patch)
2015-07-07 13:23 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2015-07-06 00:06:04 UTC
When a GtkApplication launches a window remotely, we still specify a stale _NET_WM_USER_TIME if the user has never interacted with the application before.

This was discussed in https://bugzilla.gnome.org/show_bug.cgi?id=149028#c32 -- applications should pass the timestamp through, but we don't have a way to pass a _NET_WM_USER_TIME through the stack outside of the scope of launching an application.

Nevertheless, we really can't fix applications like Chromium which simply shell out to xdg-open. We should just make xdg-open work.

I have a terrible idea that's crazy enough to just work -- when using the Activate, CommandLine, or Open actions, don't specify a _NET_WM_USER_TIME on the window. This shouldn't break focus stealing from actual launch contexts, since in those cases, startup-notification / desktop-startup-id should be used instead.

Two patches, one for GLib, one for Gio, are attached here. They're not production-ready (they need proper symbol versioning and docs, and perhaps should just be made entirely private) but they show the idea well enough, I think.
Comment 1 Jasper St. Pierre (not reading bugmail) 2015-07-06 00:06:52 UTC
Created attachment 306880 [details] [review]
gapplication: terrible launching hacks
Comment 2 Jasper St. Pierre (not reading bugmail) 2015-07-06 00:07:02 UTC
Created attachment 306881 [details] [review]
gtkapplication: terrible launching hacks
Comment 3 Jasper St. Pierre (not reading bugmail) 2015-07-06 00:08:16 UTC
Owen / Ryan / Alex: thoughts?
Comment 4 Alexander Larsson 2015-07-06 08:51:51 UTC
Here is how this should work:

app "A" spawns app "B" using dbus activation as per: http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s07.html

1) A sets the desktop-startup-id field in the platform_data argument in the Open call based on its  _NET_WM_USER_TIME of the launching window
2) B takes this and sets it as the  _NET_WM_USER_TIME on the new window

app "A" spawns "B" with fork+exec

3) "A" creates a startup notification operation by sending x messages to the WM. These involve specifying the last timestamp of "A".
4) A sets DESKTOP_STARTUP_ID in the forked process to the name of the startup nofication thing created in 3, it was historically a pure unique id, but at some point it change to be of the form "<whatever>_TIME<timestamp>" thus also encoding the timestamp to use for the child.
5) "B" takes the timestamp from the DESKTOP_STARTUP_ID an applies it to _NET_WM_USER_TIME on the new window. It also sets _NET_STARTUP_ID to the full startup id. If the _TIMExxx prefix was not there, the WM itself is supposed to know from the _NET_STARTUP_ID and the TIME set in 3 the time to use for focus-stealing prevention.

If we fork+exec a GtkApplication app we will get *both* of these, as the initiating app will do 3-5 and then the forked app will do 1-2, propagating the _TIME prefix into the platform_data. This breaks if _TIME prefix is not there, as it fails to propagate the _NET_STARTUP_ID.

So, how is this traditionally done in glib/gtk? 
3-4: Spawning is done via GDesktopAppInfo, which takes a GAppLaunchContext, which gets the current time in gdk_x11_app_launch_context_get_startup_notify_id() and is later set as an env var in g_desktop_app_info_launch_uris_with_spawn()
5: gdk_x11_display_make_default() calls gdk_x11_display_set_startup_notification_id() with the DESKTOP_STARTUP_ID env var which is then unset. This parses the _TIME prefix to set latest user time, as well as setting _NET_STARTUP_ID on the first mapped window.

Similarily, with GtkApplication use via DBusActivate=true in desktop file

1: 
  GApplaunchContext (via X11 impl) sends X messages and creates startup id with time prefix.
  g_desktop_app_info_launch_uris_with_dbus() builds the platform data from the above launch context startup id and passes it to the target
2: 
   gtk_application_impl_x11_before_emit() gets the platform data and sets the last user timestamp and startup id for next window
   emit signal and open window
   gtk_application_after_emit calls gdk_notify_startup_complete() to ensure it was called (even if we didn't show a window i guess?)

In the fork+exec converted to dbus call case we transfer the env var to platform_data in gtk_application_add_platform_data()   

Now, there are many cases where this breaks. Here are a list of some:

* the env var or platform_data is never set, because the app was not spawned correctly (typically it was started from the terminal)
* Wayland has no implementation of any of these in glib/gtk+, as well as not really having a global time concept.
* Spawning app did not create a GdkAppLaunchContext, so we have to way to plumb in the X11 user time.
* App used the old protocol version which did not prefix the id with _TIME...

In these cases I guess the best approach is that as early as possible when we detect this case try to get a global timestamp from the Xserver and use that instead. That way any user interactions on other windows after this will be protected from focus stealing. Not sure where this could easiest be plumbed though.

I have no idea what to do on wayland though.
Comment 5 Jasper St. Pierre (not reading bugmail) 2015-07-06 16:32:24 UTC
Perhaps the easiest place to do this would be to special-case g_app_info_launch for a NULL launch context meaning "look for DESKTOP_STARTUP_ID if it exists in the environment and has a _TIME prefix", with a fallback to "fetch the most timestamp from the server".

This would fix gvfs-open and xdg-open. It wouldn't fix launching the app manually on the terminal, though.
Comment 6 Allison Karlitskaya (desrt) 2015-07-06 16:56:19 UTC
There are two solutions here.  One is the good one and one is the bad one.

First, the bad one:

If xdg-open cannot figure out a timestamp, it needs to connect to the X server, create an input-only window, watch PropertyNotify and then change a property on that window.  Property change notifications come with a timestamp, so now you have a valid timestamp.  startup-notification should then be used with that timestamp (and whether D-Bus or fork/exec is used is not really very interesting).

This is cheating and, by definition, it will do the wrong thing sometimes.

The good one:

Fix Firefox, Chrome, etc.  This simply must be done, and saying "we can't do it" doesn't change that.

While we're at it, we should fix gnome-terminal to forward the timestamp associated with the [Enter] keypress that resulted in an application being launched from the commandline.
Comment 7 Allison Karlitskaya (desrt) 2015-07-07 13:23:37 UTC
Created attachment 307009 [details] [review]
GtkApplication: avoid using stale timestamps

Avoid using a stale timestamp (from the last user interaction with the
application) when a message arrives from D-Bus requesting that a new
window be created.

In this case the most-correct thing that we can do is to use no
timestamp at all.

We modify gdk_x11_display_set_startup_notification_id() to allow a NULL
value to mean "reset everything" and then call this function
unconditionally on receipt of D-Bus activation requests.  The result
will be that a missing desktop-startup-id in the platform-data struct
will reset the timestamp.

Under their default configuration metacity and mutter will both map
windows presented with no timestamp in the foreground.  This could
result in false-positive, but there is very little we can do about that
without the original timestamp from the user event.
Comment 8 Allison Karlitskaya (desrt) 2015-07-07 13:25:24 UTC
Test:

In terminal:

 $ gedit &

   type some stuff into the gedit window

 $ sleep 5; gedit

   switch to a non-terminal window and interact with it (to avoid the WM's special treatment of terminals)



Without the patch, gedit doesn't get raised.  After the patch, it raises.

This doesn't fix the issue, however.  gedit may be being raised inappropriately in this situation, and the only real fix is to ensure that the timestamp information is forwarded correctly.
Comment 9 Alexander Larsson 2015-07-08 08:38:56 UTC
Review of attachment 307009 [details] [review]:

There is a teeny tiny race condition where if you interacted with the app, and its doing something slow which will eventually pop up a window. If inbetween you get a request to open a new window then both the new windows will get no timestamp. However, this is not a critical problem, plus it is very unlikely, so this seems like a good approach to me.
Comment 10 Jasper St. Pierre (not reading bugmail) 2015-07-15 01:34:28 UTC
Attachment 307009 [details] pushed as a00a5ed - GtkApplication: avoid using stale timestamps