GNOME Bugzilla – Bug 620464
.desktop files from subdirectories not detected by application tracking
Last modified: 2011-03-20 20:43:45 UTC
I noticed using Okular that it's not detected. Its .desktop file is in /usr/share/applications/kde4/okular, the WM_CLASS is "okular", "Okular". This is likely to apply to all KDE/Qt apps (I could check with RKward too). Maybe we need checking in subdirectories too. For example, if checking in the main dir fails, then try matching the name in dubdirs.
Created attachment 163168 [details] [review] [ShellAppSystem] dynamically generate known_vendor_prefixes It create prefix based on desktop file's id and desktop file's path. KDE4 apps has prefix 'kde4-' for id of desktop file (they stored in $XDG_DATA_DIRS/applications/kde4).
Created attachment 171282 [details] [review] [ShellAppSystem] dynamically generate known_vendor_prefixes It generate prefixes for: 1. sub dir in app dir 2. LegacyDir with prefix But it still need vendor_prefixes list, because It is impossible generate some prefixes without errors. e. g. fedora-juicer.desktop sound-juicer.desktop juicer.desktop
patch is still actual! Gnome-shell will display wm_class for each kde4 app, that was started from runDialog(i.e. without startup notification)
(In reply to comment #2) > Created an attachment (id=171282) [details] [review] > [ShellAppSystem] dynamically generate known_vendor_prefixes > > It generate prefixes for: > 1. sub dir in app dir > 2. LegacyDir with prefix > > But it still need vendor_prefixes list, because > It is impossible generate some prefixes without errors. > > e. g. > fedora-juicer.desktop > sound-juicer.desktop > juicer.desktop What's the problem with sound-juicer? Or in general, can I get some concrete examples for problematic applications, where their .desktop files are, etc?
(In reply to comment #4) > What's the problem with sound-juicer? In general we can't assume that abc- is prefix. > Or in general, can I get some concrete examples for problematic applications, where their .desktop files are, etc? okular (kde4/okular.desktop) kdiff3 (kde/kdiff3.desktop)
(In reply to comment #5) > (In reply to comment #4) > > What's the problem with sound-juicer? > In general we can't assume that abc- is prefix. Right; that was the point of the known_vendor_prefixes variable; what was wrong with it? > > Or in general, can I get some concrete examples for problematic applications, where their .desktop files are, etc? > okular (kde4/okular.desktop) > kdiff3 (kde/kdiff3.desktop) Okay, but for this, can't we just use the last path component?
(In reply to comment #6) > Okay, but for this, can't we just use the last path component? my patch do this
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > What's the problem with sound-juicer? > > In general we can't assume that abc- is prefix. > > Right; that was the point of the known_vendor_prefixes variable; what was wrong > with it? > > > > Or in general, can I get some concrete examples for problematic applications, where their .desktop files are, etc? > > okular (kde4/okular.desktop) > > kdiff3 (kde/kdiff3.desktop) > > Okay, but for this, can't we just use the last path component? GMenu, following the menu specification, concatenates subdirectories in the id returned by gmenu_tree_entry_get_desktop_file_id, replacing '/' with '-'. (http://git.gnome.org/browse/gnome-menus/tree/libmenu/entry-directories.c#n851) This means that ShellAppSystem has to remove the prefix, but only those that are actually prefixes added by libgmenu, not just anything. From review, the patch is wrong with the handling of desktop files in LegacyDirs: libgmenu doesn't add directory prefixes in addition to the specified prefix, so you shouldn't g_strconcat (t_id, prefix, NULL). Btw, prefix at that point should be NULL in most cases (it would fail only in corner cases like e.g. /etc/applnk/gnome/gnome-shell.desktop, with a <LegacyDir prefix="legacy", the id is "legacy-gnome-shell.desktop", but get_prefix would return "legacy-gnome-", because "gnome" is indeed found in the id). Also, it should be t_id[id_len - name_len] = '\0', not t_id[id_len - name_len - 1], or it will eat the '-'.
Created attachment 183681 [details] [review] ShellAppSystem: dynamically generate known_vendor_prefixes > the patch is wrong with the handling of desktop files in LegacyDirs fixed
Review of attachment 183681 [details] [review]: Generally looks good, even if conceptually it's a bit backward - thinking out loudly we could strip the prefix in cache_by_id and add the stripped name in the hashtable, instead of trying to add each prefix when looking up, but perhaps there's some value in having real IDs in ShellApps. Also, I heard they wanted to revise all the code around .desktop caching and application matching to be less dependent on menus, but let's get this in for now.