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 752258 - GNOME Shell application tracking broken by change in how GDK sets xdg_surface.set_app_id
GNOME Shell application tracking broken by change in how GDK sets xdg_surface...
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.17.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2015-07-11 05:19 UTC by Stefan Nuxoll
Modified: 2018-04-15 00:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Rename .desktop file (2.77 KB, patch)
2015-07-11 05:25 UTC, Stefan Nuxoll
none Details | Review
Looking Glass screenshot (47.21 KB, image/png)
2015-07-12 06:37 UTC, Stefan Nuxoll
  Details

Description Stefan Nuxoll 2015-07-11 05:19:27 UTC
Epiphany is not displaying the correct application name in gnome-shell, displaying the application id "org.gnome.Epiphany" instead of "Web" from the .desktop file. Renaming the desktop file from epiphany.desktop to org.gnome.Epiphany.desktop resolves the issue and allows gnome-shell to detect the correct application name.
Comment 1 Stefan Nuxoll 2015-07-11 05:25:02 UTC
Created attachment 307290 [details] [review]
Rename .desktop file

Proposed patch, renames .desktop file to org.gnome.Epiphany.desktop
Comment 2 Michael Catanzaro 2015-07-11 14:22:56 UTC
Er, any idea why? I certainly do not experience that issue.

We do have to rename the desktop file to this eventually, but I wasn't expecting to do so prior to adding support for D-Bus activation.
Comment 3 Michael Catanzaro 2015-07-11 14:33:11 UTC
Florian, could this be caused by https://git.gnome.org/browse/gnome-shell/commit/?id=eac303f84c83b550afc19dc6076f4563e4039d93 ?

From the downstream bug: "This appears to be related to changes in how gnome-shell is mapping applications using GApplication to their respective desktop files to determine the name to be used in the app menu. Renaming epiphany.desktop to org.gnome.Epiphany.desktop causes gnome-shell to display the correct name."
Comment 4 Florian Müllner 2015-07-11 16:06:06 UTC
(In reply to Michael Catanzaro from comment #2)
> Er, any idea why? I certainly do not experience that issue.

No idea off-hand - we do use the GApplication ID for window matching[0], but if there is no matching .desktop file, we just go on with the heuristics and try the WM_CLASS next. So I would expect the existing .desktop file to work just fine, and that indeed appears to be the case here (jhbuild).


(In reply to Michael Catanzaro from comment #3)
> Florian, could this be caused by
> https://git.gnome.org/browse/gnome-shell/commit/
> ?id=eac303f84c83b550afc19dc6076f4563e4039d93 ?

No. That commit is only relevant for matches based on the StartupWMClass property, which will only be used after we failed to find a .desktop file based on either GApplication ID or WM_CLASS, and only changes a corner case with regard to the previous (3.10+) behavior (multiple .desktop files with a matching StartupWMClass property). Even if the WM_CLASS heuristic fails for epiphany (which it shouldn't), I still don't see how it would affect epiphany which sets either no (main app) or a unique (webapps) StartupWMClass.

[0] https://git.gnome.org/browse/gnome-shell/tree/src/shell-window-tracker.c#n360
Comment 5 Florian Müllner 2015-07-11 16:13:55 UTC
(In reply to Stefan Nuxoll from comment #0)
> Epiphany is not displaying the correct application name in gnome-shell

Could you please open looking glass (alt-f2 lg) and switch to the Windows tab, and see what appears under wmclass and app when that is happening?
Comment 6 Michael Catanzaro 2015-07-12 03:05:06 UTC
(In reply to Florian Müllner from comment #4)
> Even if the WM_CLASS heuristic fails for
> epiphany (which it shouldn't)

Yeah it shouldn't, our WM_CLASS is "epiphany" so there's not really any way that can go wrong....

> , I still don't see how it would affect
> epiphany which sets either no (main app) or a unique (webapps)
> StartupWMClass.

Wild speculation: I guess the reporter is using rawhide, so probably Wayland rather than X. And surely WM_CLASS is not a thing in Wayland. I think we're supposed to use wl_shell_surface_set_class() instead. I see mutter looks at that in the callback in meta-wayland-surface.c, but I don't see GTK+ set it anywhere. Seems like a good theory, the only problem with it is that I would expect lots of other applications to be broken, not just Epiphany.... Ah, well GTK+ does call xdg_surface_set_app_id(), which mutter looks at as well, so theory busted. Yawn.
Comment 7 Stefan Nuxoll 2015-07-12 03:07:57 UTC
(In reply to Michael Catanzaro from comment #6)
> (In reply to Florian Müllner from comment #4)
> > Even if the WM_CLASS heuristic fails for
> > epiphany (which it shouldn't)
> 
> Yeah it shouldn't, our WM_CLASS is "epiphany" so there's not really any way
> that can go wrong....
> 
> > , I still don't see how it would affect
> > epiphany which sets either no (main app) or a unique (webapps)
> > StartupWMClass.
> 
> Wild speculation: I guess the reporter is using rawhide, so probably Wayland
> rather than X. And surely WM_CLASS is not a thing in Wayland. I think we're
> supposed to use wl_shell_surface_set_class() instead. I see mutter looks at
> that in the callback in meta-wayland-surface.c, but I don't see GTK+ set it
> anywhere. Seems like a good theory, the only problem with it is that I would
> expect lots of other applications to be broken, not just Epiphany.... Ah,
> well GTK+ does call xdg_surface_set_app_id(), which mutter looks at as well,
> so theory busted. Yawn.

Sorry, yes, I am using Fedora Rawhide, I was unable to reproduce the issue in my VM (wayland doesn't seem to be working) so I'm reinstalling it on a removable flash drive. I noticed the same problem with Geary too, allow me a bit to see if I can reproduce.
Comment 8 Stefan Nuxoll 2015-07-12 06:37:09 UTC
Created attachment 307303 [details]
Looking Glass screenshot

I've attached a screenshot of the looking glass Windows tab while running under wayland in Fedora rawhide. It appears the WMClass is getting set to the App ID, I'm having the same issue with Geary as well.
Comment 9 Florian Müllner 2015-07-12 08:52:03 UTC
(In reply to Michael Catanzaro from comment #6)
> Ah, well GTK+ does call xdg_surface_set_app_id(), which mutter looks at as well,
> so theory busted.

But that's it - GDK prefers the GApplication ID over the prgname for the app_id[0], which mutter uses as WM_CLASS on wayland[1]. Not using the GApplication ID there (which is already exposed via gtk_surface_set_dbus_properties() and preferred by the shell's heuristics if available) would bring the wayland backend back in line with X11, but then the GApplication ID clearly makes a better app ID than prgname (which simply refers to the executable name). Time to bring in some GTK+ people I guess - changing GDK would smooth the wayland transition, while renaming .desktop files is something we expect apps to do eventually anyway, so one more incentive to do so isn't necessarily a bad thing ...

[0] https://git.gnome.org/browse/gtk+/tree/gdk/wayland/gdkwindow-wayland.c#n1012
[1] https://git.gnome.org/browse/mutter/tree/src/wayland/meta-wayland-surface.c#n913
Comment 10 Stefan Nuxoll 2015-07-12 14:02:05 UTC
Looks like this is the relevant commit to GTK+

https://git.gnome.org/browse/gtk+/commit/gdk/wayland/gdkwindow-wayland.c?id=71256a0f94972be8220049b1c99dc2db61e56b5a

This will affect a large number of applications if Fedora 23 does make a switch to using Wayland by default. I think the change makes sense, and I support it, but is there another way to great shell to match the App ID without renaming the desktop file (is there an additional property I can set?), patching the desktop file with an extra line is a much easier patch to maintain than renaming it if an upstream isn't responsive.
Comment 11 Michael Catanzaro 2015-07-12 14:35:20 UTC
(In reply to Stefan Nuxoll from comment #10)
> Looks like this is the relevant commit to GTK+
> 
> https://git.gnome.org/browse/gtk+/commit/gdk/wayland/gdkwindow-wayland.
> c?id=71256a0f94972be8220049b1c99dc2db61e56b5a
> 
> This will affect a large number of applications if Fedora 23 does make a
> switch to using Wayland by default.

Since it only affects GtkApplications, they will mostly be GNOME applications that we can fix ourselves, so I think it's an acceptable transition to make if we really want to....

(In reply to Florian Müllner from comment #9)
> Time to bring in some GTK+ people I guess -
> changing GDK would smooth the wayland transition, while renaming .desktop
> files is something we expect apps to do eventually anyway, so one more
> incentive to do so isn't necessarily a bad thing ...

I have no strong opinion, but I mostly see disadvantages to preferring the application ID over prgname:

* The only reason to ever call g_set_prgname() is to override WM_CLASS, so this change makes that function useless for GtkApplications. I guess that's not necessarily a bad thing.
* The behavior of the X11 and Wayland backends really ought to match. I see no reason for them to differ.
* This makes it harder to tell at a glance if an application supports D-Bus activation. Currently it's a safe bet that any desktop file with reverse-DNS notation does.

Besides that it creates a big transition that has to occur in the next two months.
Comment 12 Matthias Clasen 2015-07-15 04:30:42 UTC
I've asked Jonas to comment here; he implemented this change.
Comment 13 Jonas Ådahl 2015-07-15 06:41:50 UTC
The linked GTK+ commit is an attempt to implement the client<->shell Wayland protocol (xdg_shell) more properly. The reason that triggered the change was that applications that had transitioned te being D-Bus activatable (i.e. renamed their .desktop file) would no longer match correctly because the prgname would no longer match the .desktop file and gnome-shell was not able to figure out what window was part of what application (i.e. the same as this bug).

One of the problems is that in the future, for GtkApplications the application id passed there will most likely always be correct, but there is no way for the GDK backend to know if it actually is that. The most "future proof" was then decided to be to default to the GtkApplication ID when available, and fallback to prgname.

I don't think reverting the commit is preferable since it would make GTK+ to more or less always set the incorrect application ID (according to the xdg_shell protocol). I know that the same ID is available via the set_dbus_properties request in gtk_shell, but that does not change the fact that we should properly implement the xdg_shell protocol.

While the IMHO "proper" fix is to fix all the GtkApplications, a possible transition-phase work-around is to add a "set_legacy_prgname" to gtk_shell or "gtk_legacy_shell" or something that can then be propogated to gnome-shell so that it can match applications who has not updated to the new conventions.

It'd introduce multiple ID's per application in Wayland similar to how it is in X, but at least it'd be more clearly marked as "legacy".
Comment 14 Michael Catanzaro 2015-07-15 13:04:38 UTC
(In reply to Jonas Ådahl from comment #13)
> The linked GTK+ commit is an attempt to implement the client<->shell Wayland
> protocol (xdg_shell) more properly. The reason that triggered the change was
> that applications that had transitioned te being D-Bus activatable (i.e.
> renamed their .desktop file) would no longer match correctly because the
> prgname would no longer match the .desktop file and gnome-shell was not able
> to figure out what window was part of what application (i.e. the same as
> this bug).

First up: if an application calls g_set_prgname(), it must set the prgname to the name of its desktop file. That's been the case for as long as I remember. It sounds like your change is designed to "fix" applications that renamed their desktop files without changing g_set_prgname accordingly?

I think applications doing the transition to D-Bus activation should not call g_set_prgname() at all: that's redundant with the GApplication ID. The only reason to set the prgname is to override the GApplication ID to match the desktop file, which is pointless if the desktop file uses reverse-DNS already. With this approach, we don't break applications that have not yet transitioned to D-Bus activation. And the smaller set of applications that have done the transition can be responsible for their own breakage if they have a bogus call to g_set_prgname().

> One of the problems is that in the future, for GtkApplications the
> application id passed there will most likely always be correct, but there is
> no way for the GDK backend to know if it actually is that. The most "future
> proof" was then decided to be to default to the GtkApplication ID when
> available, and fallback to prgname.
> 
> I don't think reverting the commit is preferable since it would make GTK+ to
> more or less always set the incorrect application ID (according to the
> xdg_shell protocol). I know that the same ID is available via the
> set_dbus_properties request in gtk_shell, but that does not change the fact
> that we should properly implement the xdg_shell protocol.

Two questions from me:

* Why do you think preferring the GtkApplication ID with fallback to prgname (new behavior that has broken applications) is preferable to the previous behavior preferring prgname with fallback to GtkApplication ID? (Note, GtkApplication actually sets prgname to the GtkApplication ID if it has not already been set, so the GDK X11 backend might not actually be checking the app ID, but the effect is the same as if it did.)
* If we keep this change in the Wayland backend, should we not change the X11 backend as well to match? It seems like a bad idea to have the behavior differ between the two backends.

> While the IMHO "proper" fix is to fix all the GtkApplications, a possible
> transition-phase work-around is to add a "set_legacy_prgname" to gtk_shell
> or "gtk_legacy_shell" or something that can then be propogated to
> gnome-shell so that it can match applications who has not updated to the new
> conventions.
> 
> It'd introduce multiple ID's per application in Wayland similar to how it is
> in X, but at least it'd be more clearly marked as "legacy".

Oh gosh, it would surely be easier to just fix all the GtkApplications. :p
Comment 15 Jasper St. Pierre (not reading bugmail) 2015-07-15 14:52:03 UTC
(In reply to Michael Catanzaro from comment #14)
> First up: if an application calls g_set_prgname(), it must set the prgname
> to the name of its desktop file. That's been the case for as long as I
> remember. It sounds like your change is designed to "fix" applications that
> renamed their desktop files without changing g_set_prgname accordingly?
> have a bogus call to g_set_prgname().

No, it must set the prgname to the name of your binary. If you have a binary called "gedit" and a desktop file of org.gnome.gedit.desktop, it is entirely legal to keep your prgname as gedit, because then "killall gedit" works fine.

(In reply to Jonas Ådahl from comment #13)
> The linked GTK+ commit is an attempt to implement the client<->shell Wayland
> protocol (xdg_shell) more properly. The reason that triggered the change was
> that applications that had transitioned te being D-Bus activatable (i.e.
> renamed their .desktop file) would no longer match correctly because the
> prgname would no longer match the .desktop file and gnome-shell was not able
> to figure out what window was part of what application (i.e. the same as
> this bug).

I don't believe that.

With X11, we have two fields: WM_CLASS, and _GTK_APPLICATION_ID.

None of this is ever documented officially, because application matching is all heuristics, because we never put in real ways for people to tie X11 Windows, DBus names, .desktop files and PIDs together. Up until now, we've been relying on WM_CLASS matching and _GTK_APPLICATION_ID (along with a few other heuristics like StartupWMClass thrown in for good measure -- please ignore them).

For DBus activatable apps, we use _GTK_APPLICATION_ID. For any other app, we use WM_CLASS.

With Wayland, we have two equivalent fields: xdg_surface.set_app_id, and gtk_surface.set_gtk_application_id. xdg_surface.set_app_id should match WM_CLASS / the .desktop file for non-DBus activatable apps. That means it should match the prgname, and set_gtk_application_id will catch the other cases. What were the cases where it was breaking before?

Lots of things use the desktop file ID as a key into storage, or use it to determine desktop settings (app-specific notification settings, for instance, are based on it), so renaming it has a cost. If the user adds it to a list of favorites, for instance, they'll lose that when the app updates.
Comment 16 Michael Catanzaro 2015-07-15 18:27:06 UTC
(In reply to Jasper St. Pierre from comment #15)
> No, it must set the prgname to the name of your binary. If you have a binary
> called "gedit" and a desktop file of org.gnome.gedit.desktop, it is entirely
> legal to keep your prgname as gedit, because then "killall gedit" works fine.

I can say for sure that GNOME Shell application matching in previous GNOME releases does not work unless prgname is set to the name of the desktop file. See for instance GNOME Chess 3.10, with a binary named gnome-chess and a desktop file glchess.desktop. I set the prgname to glchess, or else GNOME Shell displayed a blurry icon and the name of the binary (or was it the desktop file?) rather than "GNOME Chess" in the app menu when running gnome-chess in jhbuild. (I later renamed the desktop file to match the binary and got rid of the call that sets prgname at the same time.)

We also have a wiki page instructing developers to set prgname to match the desktop file: https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased
Comment 17 Jasper St. Pierre (not reading bugmail) 2015-07-15 19:19:42 UTC
Right, we will match on either the application ID or the WM_CLASS reported. So a desktop file of either org.gnome.gedit.desktop or gedit.desktop is OK. Something completely unrelated, like glchess.desktp, is not.
Comment 18 Stefan Nuxoll 2015-07-15 19:48:48 UTC
(In reply to Jasper St. Pierre from comment #17)
> Right, we will match on either the application ID or the WM_CLASS reported.
> So a desktop file of either org.gnome.gedit.desktop or gedit.desktop is OK.
> Something completely unrelated, like glchess.desktp, is not.

Okay, but the problem is GTK sets the WM_CLASS to the application ID when running with the Wayland backend.
Comment 19 Jasper St. Pierre (not reading bugmail) 2015-07-15 20:22:01 UTC
Right. I'm saying we should revert that patch, because we already have the _GTK_APPLICATION_ID equivalent.
Comment 20 Jonas Ådahl 2015-07-16 01:37:55 UTC
So, according to Jasper in comment 15 "prgname" should be (or is completely valid to be) the name of the binary, and according to Michael in comment 16 "prgname" should always be the basename of the .desktop file. For the org.gnome.gedit.desktop case, *should* the prgname be "gedit" or "org.gnome.gedit"?

If it should be "gedit" then we cannot use prgname to set the xdg_surface app_id since it should mean that for D-Bus activatable applications always set the wrong ID. Using the GtkApplication ID isn't very good either, since it'd be wrong for all non-D-Bus-activatable applications, but at least it isn't incorrect forever.

If it should be "org.gnome.gedit" it'd be helpful to fix the documentation of glib (not counting a wiki page) to say that applications need to set the "prgname" to the basename of the .desktop file. In this case we can use prgname to set the xdg_surface app_id (i.e. revert the patch and apply the original rejected patch from the same bug) and also fix all the applications that doesn't yet do this.

So my question is, should "prgname" *always* be the .desktop file basename?
Comment 21 Michael Catanzaro 2015-07-16 15:04:28 UTC
(In reply to Jonas Ådahl from comment #20)
> So, according to Jasper in comment 15 "prgname" should be (or is completely
> valid to be) the name of the binary, and according to Michael in comment 16
> "prgname" should always be the basename of the .desktop file. For the
> org.gnome.gedit.desktop case, *should* the prgname be "gedit" or
> "org.gnome.gedit"?

It should be "org.gnome.gedit" based on our current guidance at https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased but I defer to Jasper as to whether we should change the guidance. We have to change some applications no matter what....
Comment 22 Florian Müllner 2015-07-16 15:15:13 UTC
(In reply to Michael Catanzaro from comment #21)
> It should be "org.gnome.gedit" based on our current guidance at
> https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased

For dbus-activatable apps using GApplication (like gedit), it doesn't matter - we won't use the WM_CLASS at all in that case, as matching by _GTK_APPLICATION_ID already turns up the .desktop file.
Comment 23 Jasper St. Pierre (not reading bugmail) 2015-07-16 15:54:55 UTC
(In reply to Jonas Ådahl from comment #20)
> So, according to Jasper in comment 15 "prgname" should be (or is completely
> valid to be) the name of the binary, and according to Michael in comment 16
> "prgname" should always be the basename of the .desktop file. For the
> org.gnome.gedit.desktop case, *should* the prgname be "gedit" or
> "org.gnome.gedit"?
> 
> If it should be "gedit" then we cannot use prgname to set the xdg_surface
> app_id since it should mean that for D-Bus activatable applications always
> set the wrong ID. Using the GtkApplication ID isn't very good either, since
> it'd be wrong for all non-D-Bus-activatable applications, but at least it
> isn't incorrect forever.

For gedit, xdg_surface.set_app_id should be called with "gedit", and gtk_surface.set_gtk_application_id should be called with "org.gnome.gedit".
Comment 24 Jonas Ådahl 2015-07-17 00:48:42 UTC
(In reply to Jasper St. Pierre from comment #23)
> (In reply to Jonas Ådahl from comment #20)
> > So, according to Jasper in comment 15 "prgname" should be (or is completely
> > valid to be) the name of the binary, and according to Michael in comment 16
> > "prgname" should always be the basename of the .desktop file. For the
> > org.gnome.gedit.desktop case, *should* the prgname be "gedit" or
> > "org.gnome.gedit"?
> > 
> > If it should be "gedit" then we cannot use prgname to set the xdg_surface
> > app_id since it should mean that for D-Bus activatable applications always
> > set the wrong ID. Using the GtkApplication ID isn't very good either, since
> > it'd be wrong for all non-D-Bus-activatable applications, but at least it
> > isn't incorrect forever.
> 
> For gedit, xdg_surface.set_app_id should be called with "gedit", and
> gtk_surface.set_gtk_application_id should be called with "org.gnome.gedit".

That'd go against any version of "set_app_id" I've seen. The .desktop file of gedit is "org.gnome.gedit.desktop", gedit is a D-Bus activatable applicatoin. According to xdg_surface.set_app_id it should definitely not be "gedit".

If you think xdg_surface.set_app_id is wrong and should be some "binary name" or similar, unrelated to .desktop file names and D-Bus names, then we'd need to change the spec (and probably not call it anything with "ID".

(In reply to Florian Müllner from comment #22)
> (In reply to Michael Catanzaro from comment #21)
> > It should be "org.gnome.gedit" based on our current guidance at
> > https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased
> 
> For dbus-activatable apps using GApplication (like gedit), it doesn't matter
> - we won't use the WM_CLASS at all in that case, as matching by
> _GTK_APPLICATION_ID already turns up the .desktop file.

The question is rather, how do we get a valid name to pass to xdg_surface.set_app_id, which id should be the base name of the .desktop file. Can we force GtkApplication apps to set the prgname to this and put that use case in the API docs?
Comment 25 Florian Müllner 2015-07-17 01:58:42 UTC
(In reply to Jonas Ådahl from comment #24)
> The question is rather, how do we get a valid name to pass to
> xdg_surface.set_app_id, which id should be the base name of the .desktop
> file. Can we force GtkApplication apps to set the prgname to this?

To the basename of the .desktop file? No, we don't know that. GtkApplication does set a default prgname if the app didn't set it itself[0] (which happens to match the pre-dbus-activation convention for .desktop file names), and apps with a different naming scheme (hopefully) call it themselves to hint at the right name. So using g_get_prgname() for set_app_id() should be a good pick if the goal is to associate the app with its .desktop file, or just go back to the previously used gdk_get_program_class() - that's what's used on X, and some fringe app might call gdk_set_program_class() directly instead of g_set_prgname()

[0] https://git.gnome.org/browse/glib/tree/gio/gapplication.c#n2271
Comment 26 Jonas Ådahl 2015-07-17 02:08:13 UTC
(In reply to Florian Müllner from comment #25)
> (In reply to Jonas Ådahl from comment #24)
> > The question is rather, how do we get a valid name to pass to
> > xdg_surface.set_app_id, which id should be the base name of the .desktop
> > file. Can we force GtkApplication apps to set the prgname to this?
> 
> To the basename of the .desktop file? No, we don't know that. GtkApplication
> does set a default prgname if the app didn't set it itself[0] (which happens
> to match the pre-dbus-activation convention for .desktop file names), and
> apps with a different naming scheme (hopefully) call it themselves to hint
> at the right name. So using g_get_prgname() for set_app_id() should be a
> good pick if the goal is to associate the app with its .desktop file, or
> just go back to the previously used gdk_get_program_class() - that's what's
> used on X, and some fringe app might call gdk_set_program_class() directly
> instead of g_set_prgname()
> 

Won't this mean it'll be incorrect for D-Bus activatable applications with no possibility to fix it? Or do you mean D-Bus activatable applications should set the prgname to the interface name?

gdk_get_program_class() will most likely never be correct. Prior to the patch, gedit would be "Gedit" and gnome-terminal would be "Gnome-terminal" IIRC.
Comment 27 Florian Müllner 2015-07-17 03:11:28 UTC
(In reply to Jonas Ådahl from comment #26)
> (In reply to Florian Müllner from comment #25)
> Won't this mean it'll be incorrect for D-Bus activatable applications with
> no possibility to fix it? Or do you mean D-Bus activatable applications
> should set the prgname to the interface name?

Yes, those can set prgname, though it wouldn't matter for GTK+ apps under GNOME (because _GTK_APPLICATION_ID/gtk_surface.application_id already matches the .desktop file in that case).


> gdk_get_program_class() will most likely never be correct. Prior to the
> patch, gedit would be "Gedit" and gnome-terminal would be "Gnome-terminal"
> IIRC.

The gedit case doesn't really matter (see above), and gnome-terminal is fine - the capitalization difference is covered by our heuristics.
Comment 28 Jonas Ådahl 2015-07-17 03:18:58 UTC
(In reply to Florian Müllner from comment #27)
> (In reply to Jonas Ådahl from comment #26)
> > (In reply to Florian Müllner from comment #25)
> > Won't this mean it'll be incorrect for D-Bus activatable applications with
> > no possibility to fix it? Or do you mean D-Bus activatable applications
> > should set the prgname to the interface name?
> 
> Yes, those can set prgname, though it wouldn't matter for GTK+ apps under
> GNOME (because _GTK_APPLICATION_ID/gtk_surface.application_id already
> matches the .desktop file in that case).

If this is the case, then the patch should be reverted, the original patch of the bug should be applied instead. Can we also articulate this convention somewhere, and add that to the glib API docs? So that every D-Bus activatable application in GNOME calls g_set_prgname() with the bus name. It seems to be the only way to make it possible to make it future proof without adding more API.

> 
> 
> > gdk_get_program_class() will most likely never be correct. Prior to the
> > patch, gedit would be "Gedit" and gnome-terminal would be "Gnome-terminal"
> > IIRC.
> 
> The gedit case doesn't really matter (see above), and gnome-terminal is fine
> - the capitalization difference is covered by our heuristics.

Yea, but we still wouldn't follow the specification we wrote ourself.
Comment 29 Jasper St. Pierre (not reading bugmail) 2015-07-17 05:41:25 UTC
(In reply to Jonas Ådahl from comment #24)
> That'd go against any version of "set_app_id" I've seen. The .desktop file
> of gedit is "org.gnome.gedit.desktop", gedit is a D-Bus activatable
> applicatoin. According to xdg_surface.set_app_id it should definitely not be
> "gedit".
> 
> If you think xdg_surface.set_app_id is wrong and should be some "binary
> name" or similar, unrelated to .desktop file names and D-Bus names, then
> we'd need to change the spec (and probably not call it anything with "ID".

Yes. It's technically against spec. But it's basically all we have.

The issue is that we have a prgname of "gedit" and an application ID of "org.gnome.gedit". The desktop file can be either one of "gedit.desktop" or "org.gnome.gedit.desktop" and it will work absolutely fine under X11. The problem is that we have no idea which is which. Some apps, like gedit, use their proper bus name application ID as their desktop filename. Others, like epiphany, use their prgname is their desktop filename. Both are, in their own sense, "application IDs". GTK+ doesn't have any API for you to tell it which one it is.

The spec was not written for the "real world", in which case matching applications and desktop files and DBus names are a messy, unspecified business applying new meaning to old things like WM_CLASS roles. It was written for the new world of DBusActivatable applications, in which you have one ID, which is org.gnome.gedit. Epiphany is not an app that does this... yet?.

Perhaps that means the spec is bad. But xdg-shell.xml isn't the place for explaining shell-specific application matching behavior.

We have one piece of the puzzle, which is gtk_surface.set_application_id, which is guaranteed to always be "org.gnome.gedit" in this case.

In mutter, xdg_surface.set_application_id is wired up to wm_class, which is what would normally be filled with "gedit" under X11. So it only makes sense to me to match it.

If you want to instead also be fine with a suggestion to add a private gtk_surface.set_prgname method, and figure out what to do with set_application_id later, that's fine by me.
Comment 30 Michael Catanzaro 2015-08-09 07:30:46 UTC
Pinging stakeholders on GtkApplication (Ryan, Matthias) for this:

I had a long discussion about this bug yesterday with Jonas and Carlos Garnacho. We agreed that our solution should (a) not require changes in applications except D-Bus activatable applications, since there are relatively few such applications and we can fix them easily, and our solution should also (b) result in matching behavior between mutter's X11 and Wayland backends, i.e. we agree that it's wrong to prefer the GtkApplication ID in the Wayland backend but WM_CLASS/xdg_surface.set_application_id in the X11 backend.

Our preferred solution is to simply revert this change. Then WM_CLASS/xdg_surface.set_application_id will be preferred always. D-Bus activatable applications will need to set their prgname to the full name of the desktop file, e.g. g_set_prgname("org.gnome.gedit"). This is already required by our current documentation of GNOME's application matching [1] so it is not really any change in our guidance. We would then fix all D-Bus activatable apps.

To make this simpler, it would be ideal for applications to not need to call g_set_prgname, but for it to be set properly automatically. This requires GTK+ to know whether an application is D-Bus activatable. Is there already a solution for that? If not, we discussed one possibility: adding a new enum value to GApplicationFlags, say G_APPLICATION_IS_DBUS_ACTIVATABLE. GTK+ would then use this to determine that the prgname should be set to the GtkApplication ID rather than argv[0]. I am not entirely sure this new flag would be a good idea, though, since it's not going to be much easier to use than simply setting the prgname manually.

[1] https://wiki.gnome.org/Projects/GnomeShell/ApplicationBased
Comment 31 Florian Müllner 2015-08-09 09:44:18 UTC
(In reply to Michael Catanzaro from comment #30)
> Our preferred solution is to simply revert this change. Then
> WM_CLASS/xdg_surface.set_application_id will be preferred always. D-Bus
> activatable applications will need to set their prgname to the full name of
> the desktop file, e.g. g_set_prgname("org.gnome.gedit"). 

The current heuristics used in gnome-shell check for _GTK_APPLICATION_ID first, and only then use WM_CLASS/xdg_surface.application_id. So unless you are suggesting to modify our heuristics, the g_set_prgname() call is not used for app matching in the case of DBus-activatable apps as long as they're based on GtkApplication.


> To make this simpler, it would be ideal for applications to not need to call
> g_set_prgname, but for it to be set properly automatically.

That should already work for the majority of applications on X11 - prgname defaults to the executable's basename, which usually matches the .desktop filename for non-DBus-activatable apps.
And if xdg_surface.application_id is changed to match the prgname again, it would just work for most apps on wayland again.
Comment 32 Jasper St. Pierre (not reading bugmail) 2015-08-09 16:14:13 UTC
(In reply to Michael Catanzaro from comment #30)
> To make this simpler, it would be ideal for applications to not need to call
> g_set_prgname, but for it to be set properly automatically. This requires
> GTK+ to know whether an application is D-Bus activatable. Is there already a
> solution for that? If not, we discussed one possibility: adding a new enum
> value to GApplicationFlags, say G_APPLICATION_IS_DBUS_ACTIVATABLE. GTK+
> would then use this to determine that the prgname should be set to the
> GtkApplication ID rather than argv[0]. I am not entirely sure this new flag
> would be a good idea, though, since it's not going to be much easier to use
> than simply setting the prgname manually.

As I said in the comment above you, we have two sources to match on: the prgname and the application ID. We already match on both. So you shouldn't need to change anything.
Comment 33 Michael Catanzaro 2015-08-10 09:42:11 UTC
> And if xdg_surface.application_id is changed to match the prgname again, it
> would just work for most apps on wayland again.

Hm, your logic looks good to me, but it can't be right, because if you are correct, then simply reverting Jonas's commit would fix everything, but his commit fixed an actual issue with D-Bus activatable apps.

(In reply to Jasper St. Pierre from comment #32)
> As I said in the comment above you, we have two sources to match on: the
> prgname and the application ID. We already match on both. So you shouldn't
> need to change anything.

mutter never gets the prgname currently. Step one: let's change that. We all agree that xdg_surface.application_id needs to be equivalent to WM_CLASS....
Comment 34 Jonas Ådahl 2015-08-10 09:50:12 UTC
(In reply to Michael Catanzaro from comment #33)
> > And if xdg_surface.application_id is changed to match the prgname again, it
> > would just work for most apps on wayland again.
> 
> Hm, your logic looks good to me, but it can't be right, because if you are
> correct, then simply reverting Jonas's commit would fix everything, but his
> commit fixed an actual issue with D-Bus activatable apps.
> 
> (In reply to Jasper St. Pierre from comment #32)
> > As I said in the comment above you, we have two sources to match on: the
> > prgname and the application ID. We already match on both. So you shouldn't
> > need to change anything.
> 
> mutter never gets the prgname currently. Step one: let's change that. We all
> agree that xdg_surface.application_id needs to be equivalent to WM_CLASS....

That could possibly work now anyway if mutter and gnome-shell uses the GtkApplication ID passed via a gnome private protocol. Reverting would most likely work for us, but we'd still not implement the protocol. Now, I don't know if other compositors use the xdg_shell application Id for finding .desktop files, or if they will, but we will make it impossible to do so without doing the g_set_prgname thing (or adding a flag to GtkApplication)
Comment 35 Jasper St. Pierre (not reading bugmail) 2015-08-10 16:02:51 UTC
As I mentioned before, no matter which value you pick, it could be wrong. For some apps, the desktop file is named after the prgname ("epiphany.desktop"), so set_application_id should use that. For others, they use the full app ID ("org.gnome.gedit.desktop") so we should use that. GTK+ currently has *no* API to tell it which is the proper file, so no matter which value we pick, it will just be a guess.
Comment 36 Jonas Ådahl 2015-08-18 01:36:30 UTC
A patch <https://bugzilla.gnome.org/show_bug.cgi?id=746435> was pushed that changed the behaviour to use the prgname. I suppose we should keep this bug open until the problem is solved properly, whether it is updating various applications, or adding a GtkApplication API.
Comment 37 Matthias Clasen 2018-02-10 05:06:57 UTC
We're moving to gitlab! As part of this move, we are moving bugs to NEEDINFO if they haven't seen activity in more than a year. If this issue is still important to you and still relevant with GTK+ 3.22 or master, please reopen it and we will migrate it to gitlab.
Comment 38 Matthias Clasen 2018-04-15 00:28:27 UTC
As announced a while ago, we are migrating to gitlab, and bugs that haven't seen activity in the last year or so will be not be migrated, but closed out in bugzilla.

If this bug is still relevant to you, you can open a new issue describing the symptoms and how to reproduce it with gtk 3.22.x or master in gitlab:

https://gitlab.gnome.org/GNOME/gtk/issues/new