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 664444 - Support additional application actions in .desktop files
Support additional application actions in .desktop files
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 669603
 
 
Reported: 2011-11-21 00:09 UTC by Giovanni Campagna
Modified: 2013-08-04 22:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gio: introduce desktop shorcuts (13.13 KB, patch)
2011-11-21 00:10 UTC, Giovanni Campagna
none Details | Review
gio: introduce desktop shorcuts (16.51 KB, patch)
2011-11-21 14:46 UTC, Giovanni Campagna
none Details | Review
gio: introduce desktop shorcuts (16.51 KB, patch)
2011-11-21 14:56 UTC, Giovanni Campagna
none Details | Review
gio: introduce desktop actions (16.68 KB, patch)
2012-02-06 16:26 UTC, Giovanni Campagna
none Details | Review
Implement the Desktop Action specification (13.96 KB, patch)
2013-06-08 20:40 UTC, Allison Karlitskaya (desrt)
none Details | Review
Implement the Desktop Action specification (13.98 KB, patch)
2013-06-08 22:14 UTC, Allison Karlitskaya (desrt)
none Details | Review
Implement the Desktop Action specification (24.64 KB, patch)
2013-06-10 02:38 UTC, Allison Karlitskaya (desrt)
reviewed Details | Review
Implement the Desktop Action specification (24.66 KB, patch)
2013-07-02 19:25 UTC, Allison Karlitskaya (desrt)
needs-work Details | Review
Implement the Desktop Action specification (17.78 KB, patch)
2013-07-11 16:54 UTC, Allison Karlitskaya (desrt)
accepted-commit_now Details | Review

Description Giovanni Campagna 2011-11-21 00:09:44 UTC
https://live.gnome.org/ThreePointThree/Features/Jumplists calls for the ability of applications to specify a set of static actions, to be shown in gnome-shell alongside New window and other dynamic actions.
This bug tracks the GDesktopAppInfo side of this, starting from a format and a design that's compatible with Unity.
Comment 1 Giovanni Campagna 2011-11-21 00:10:03 UTC
Created attachment 201780 [details] [review]
gio: introduce desktop shorcuts

This patch introduces support in GDesktopAppInfo for so called "desktop
shortcuts". Those are extra names/exec pairs that can be associated
with an application, to create static launcher actions.
The format is the same as Unity, which first implemented this.
Comment 2 Giovanni Campagna 2011-11-21 14:46:35 UTC
Created attachment 201819 [details] [review]
gio: introduce desktop shorcuts

This patch introduces support in GDesktopAppInfo for so called "desktop
shortcuts". Those are extra names/exec pairs that can be associated
with an application, to create static launcher actions.
The format is the same as Unity, which first implemented this.

Completed with docs and annotations.
Comment 3 Giovanni Campagna 2011-11-21 14:56:27 UTC
Created attachment 201820 [details] [review]
gio: introduce desktop shorcuts

This patch introduces support in GDesktopAppInfo for so called "desktop
shortcuts". Those are extra names/exec pairs that can be associated
with an application, to create static launcher actions.
The format is the same as Unity, which first implemented this.
Comment 4 Matthias Clasen 2011-11-23 00:19:28 UTC
Before even considering this, I'd like the desktop entry syntax proposed and discussed.
Comment 5 Giovanni Campagna 2011-11-23 00:32:40 UTC
(In reply to comment #4)
> Before even considering this, I'd like the desktop entry syntax proposed and
> discussed.

This uses Unity . If there are no particular concerns, I think we should stick to it, so that app writers and distributors don't need to deal with different, incompatible syntaxes.
Still, I was thinking of proposing this to Freedesktop if approved for glib, and then deprecating the X-Ayatana prefix with a proper extension to the Desktop Entry Specification.

The spec itself is pretty simple: one X-Ayatana-Desktop-Shortcuts key with a list of strings, each identifies a group of the form [<key> Shortcut Group. Each group has Name, Exec, TargetEnvironment.
The set of properties for a group could be extended (I was thinking of Icon, and then maybe MimeType, for example for Print actions), but I would say they're fine for the first iteration.
Comment 6 Matthias Clasen 2011-11-23 03:50:18 UTC
It really needs to be discussed on the list first, I think there's quite a few questionable choices here that I'd like to see some explanation for. The 'Unity uses this' argument is not very convincing to me by itself.
Comment 7 Matthias Clasen 2011-11-24 04:00:31 UTC
One of the reasons why this needs to go into the spec first is this:

desktop-file-validate /builddir/build/BUILDROOT/gnome-screenshot-3.3.1-1.fc17.x86_64/usr/share/applications/gnome-screenshot.desktop
/builddir/build/BUILDROOT/gnome-screenshot-3.3.1-1.fc17.x86_64/usr/share/applications/gnome-screenshot.desktop: error: file contains group "Screen Shortcut Group", but groups extending the format should start with "X-"
Comment 8 Sergey "Shnatsel" Davidoff 2011-12-10 20:45:39 UTC
elementary project is also interested in this format. We have the same use case as Unity and GNOME Shell. 
We also might use it for specifying Contractor actions, which will require specifying a list of supported mime types for each action.
Comment 9 Giovanni Campagna 2012-02-06 16:26:04 UTC
Created attachment 206912 [details] [review]
gio: introduce desktop actions

This patch introduces support in GDesktopAppInfo for so called "desktop
actions". Those are extra names/exec pairs that can be associated
with an application, to create static launcher actions.
The format is included in the latest version of the Desktop Entry
Specification.
Comment 10 Giovanni Campagna 2012-02-06 16:26:46 UTC
Rebased, and updated for the specification that was approved at freedesktop.
Comment 11 Matthias Clasen 2012-02-10 01:19:32 UTC
I must say that I don't really like the idea of exposing these actions as independent objects that can outlive their owners. I'd rather go with a much more minimal api, a la

const gchar **g_desktop_info_get_actions (GDesktopInfo *info,
                                          const gchar  *desktop_env)

gboolean g_desktop_info_action_launch_uris (GDesktopInfo *info,
                                            const gchar  *action,
                                            ...)

I'd also like to investigate if we should have similar support on os x or win32,
in which case the api should be

g_app_info_get_actions / g_app_info_action_launch_uris
Comment 12 Giovanni Campagna 2012-02-14 00:00:12 UTC
I went with GDesktopAction because we may want to support more keys from action groups (for example Icon, which is already part of the spec), and because it allows to have the *ShowIn check explicit rather than done in get_actions(). It also made a nice parallel with GDesktopAppInfo.

GDesktopActions can't really outlive they're parent. In C the expectation is no memory management at all (hence (transfer none)) and rely on the parent GAppInfo to keep everything alive. Obviously, JS and Python can do all kinds of nasty things with objects, but they're already discouraged from doing so (it breaks object identity, for example).

As for other systems, Windows 7 interface is at http://msdn.microsoft.com/en-us/library/dd378402(v=vs.85).aspx, and it is the other way round: the application is invoked in a special mode and registers its actions with the desktop shell. There is no way for third parties to query jumplists (and I don't expect one would use gio to implement a replacement windows shell).
I'm not aware of standard jumplists for Mac OS.
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-06-07 17:01:29 UTC
Ping on this?
Comment 14 Allison Karlitskaya (desrt) 2013-06-05 17:04:18 UTC
I agree with Matthias about the API minimalism and I also don't think we should support launching URIs on desktop actions.  I'll look into this.
Comment 15 Allison Karlitskaya (desrt) 2013-06-08 20:40:51 UTC
Created attachment 246332 [details] [review]
Implement the Desktop Action specification

This is a much smaller API vs. the previous patch and it includes
support for DBusActivatable apps.
Comment 16 Allison Karlitskaya (desrt) 2013-06-08 22:14:56 UTC
Created attachment 246337 [details] [review]
Implement the Desktop Action specification

Found a small bug in the sending of the D-Bus message.  Fixed in this revision.
Comment 17 Giovanni Campagna 2013-06-08 23:38:39 UTC
One thing that should be pointed out: this patch ignores OnlyShowIn, so eg. actions that only make sense in Unity will show up in GNOME Shell.
Comment 18 Allison Karlitskaya (desrt) 2013-06-09 06:02:27 UTC
It's also probably (unfortunately) going to also need GAppLaunchContext* on the activate_action() function... :/
Comment 19 Jasper St. Pierre (not reading bugmail) 2013-06-09 06:09:19 UTC
1) what's wrong with GAppLaunchContext?
2) it's not needed for OnlyShowIn -- g_desktop_app_info_set_desktop_env sets a global.
Comment 20 Jasper St. Pierre (not reading bugmail) 2013-06-09 06:10:23 UTC
Oh, and I should probably point out, since nobody else has, that the spec is now upstream. I'm not sure if this implementation is compliant:

http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s08.html#extra-actions-identifier
Comment 21 Allison Karlitskaya (desrt) 2013-06-09 15:03:51 UTC
(In reply to comment #20)
> Oh, and I should probably point out, since nobody else has, that the spec is
> now upstream. I'm not sure if this implementation is compliant:
> 
> http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s08.html#extra-actions-identifier

The fact that this is upstream is what caused me to want to support it, and I worked from the spec...
Comment 22 Allison Karlitskaya (desrt) 2013-06-10 02:38:46 UTC
Created attachment 246375 [details] [review]
Implement the Desktop Action specification

Now with 100% more OnlyShowIn/NotShowIn and GAppLaunchContext!
Comment 23 Jasper St. Pierre (not reading bugmail) 2013-06-10 18:29:46 UTC
Review of attachment 246375 [details] [review]:

::: gio/gdesktopappinfo.c
@@ +1720,3 @@
 {
+  /* We may have a spurious failure, so loop... */
+  while (g_atomic_pointer_get (&g_desktop_env) == NULL)

GOnce?

@@ +3781,3 @@
+
+/**
+ * g_desktop_app_info_should_show_action:

bad doc?

@@ +3922,3 @@
+  session_bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
+
+  if (session_bus && info->app_id)

I think it would be nice to explicitly be able to explicitly specify which path you go down, especially as you mention explicitly in the documentation that this can be used to activate applications that don't necessarily have exposed actions in the desktop file.

Being able to say "I want to ensure that we never pull up the desktop file and use the Exec= line for an action" is a better API.

::: gio/tests/Makefile.am
@@ +228,2 @@
 dist_test_data += \
+	actions.desktop				\

appinfo-test-actions.desktop ?

::: gio/tests/desktop-app-info.c
@@ +386,3 @@
+   * be OK.
+   *
+   * 600 * 100ms = 60 seconds.

I'd like Colin's OK on this.
Comment 24 Alexander Larsson 2013-06-11 08:11:05 UTC
Review of attachment 246375 [details] [review]:

::: gio/gdesktopappinfo.c
@@ +1719,3 @@
 g_desktop_app_info_set_desktop_env (const gchar *desktop_env)
 {
+  /* We may have a spurious failure, so loop... */

I don't see how we can get a spurious failure here? Surely our atomics are not weak, so if the compare_and_exchange succeeds it is now non-null, and if it failed its because it was not NULL to begin with.

I don't see how we need anything more than a single compare_and_exchange.

@@ +3731,3 @@
+ * action should actually be shown.
+ *
+ * Returns: a list of strings, always non-%NULL

(array zero-terminated=1) (element-type utf8) (transfer full)

@@ +3836,3 @@
+ *
+ * Returns: the locale-specific action name
+ *

(transfer full)

@@ +3869,3 @@
+ * @action_name: the name of the action as from
+ *   g_desktop_app_info_list_actions()
+ * @parameter: should be %NULL, but see below

(allow-none)

@@ +3929,3 @@
+      g_variant_builder_init (&params, G_VARIANT_TYPE ("av"));
+      if (parameter)
+        g_variant_builder_add (&params, "v", parameter);

Don't you ever want to pass in an array of parameters? (i.e. more than one?

::: gio/tests/desktop-app-info.c
@@ +383,3 @@
+
+  /* I hate time-based conditions in tests, but this will wait up to one
+   * whole minute for "touch file" to finish running.  I think it should

Couldn't you use startup notification for this? Then we get a test for that working too.
Comment 25 Matthias Clasen 2013-06-30 05:46:57 UTC
Review of attachment 246375 [details] [review]:

::: gio/tests/actions.desktop
@@ +1,3 @@
+[Desktop Entry]
+Type=Application
+Actions=frob;tweak;twiddle

there's a terminal semicolon missing
Comment 26 Allison Karlitskaya (desrt) 2013-07-02 19:25:35 UTC
Created attachment 248254 [details] [review]
Implement the Desktop Action specification

For some time, the desktop file specification has supported "additional
application actions".  This is intended to allow for additional methods
of starting an app, such as a mail client having a "Compose New Message"
action that brings up the compose window instead of the folder list.

This patch adds support for this with a relatively minimal API.

In the case that the application is a GApplication and DBusActivatable,
desktop actions are translated into GActions that have been added to the
application with g_action_map_add_action().  This more or less closes
the loop on being able to activate an application with an action
invocation (instead of 'activate').
Comment 27 Allison Karlitskaya (desrt) 2013-07-02 19:36:58 UTC
Changes vs last patch:

 - using g_once_init_enter() now instead of the loop
 - fix wrong doc string
 - rename desktop file in testcase, as suggsted
 - add annotations
 - add trailing ; to Actions= line

also: I agree with Jasper's concern that it's weird to deal with arbitrary D-Bus action invocation (with parameters) through the same API as might also cause an exec.  As such, I've restricted the API to only work with actions that are in the desktop file and I've added g_return_if_fail() on this and other APIs to ensure that they are only used with listed actions.

If and when we have actual usecases for invocation of arbitrary unknown actions via GDesktopAppInfo then we can talk about it....
Comment 28 Lars Karlitski 2013-07-02 20:09:12 UTC
Review of attachment 248254 [details] [review]:

::: gio/gdesktopappinfo.c
@@ +3816,3 @@
+  g_return_val_if_fail (G_IS_DESKTOP_APP_INFO (info), FALSE);
+  g_return_val_if_fail (action_name != NULL, FALSE);
+  g_return_if_fail (app_info_has_action (info, action_name));

Should be g_return_val_if_fail()

@@ +3855,3 @@
+  g_return_val_if_fail (G_IS_DESKTOP_APP_INFO (info), NULL);
+  g_return_val_if_fail (action_name != NULL, NULL);
+  g_return_if_fail (app_info_has_action (info, action_name));

Should be g_return_val_if_fail()

@@ +3933,3 @@
+      if (exec_line)
+        g_desktop_app_info_launch_uris_with_spawn (info, session_bus, exec_line, NULL, launch_context,
+                                                   _SPAWN_FLAGS_DEFAULT, NULL, NULL, NULL, NULL, NULL);

exec_line is leaked.
Comment 29 Allison Karlitskaya (desrt) 2013-07-02 20:49:24 UTC
Jasper doesn't like OnlyShowIn on actions so he's going to take his campaign to remove them from the spec to xdg-list.

Meanwhile, this patch is blocked.
Comment 30 Jasper St. Pierre (not reading bugmail) 2013-07-02 21:56:07 UTC
http://lists.freedesktop.org/archives/xdg/2013-July/012835.html

I don't really want to block the patch on this. If we can take it out and then put it in later, even if in a slightly ugly way, I'd be fine.
Comment 31 Jasper St. Pierre (not reading bugmail) 2013-07-02 21:56:54 UTC
Review of attachment 248254 [details] [review]:

::: gio/gdesktopappinfo.c
@@ -136,3 @@
 						g_desktop_app_info_iface_init))
 
-G_LOCK_DEFINE_STATIC (g_desktop_env);

Can you remove the lock in a separate commit?
Comment 32 Allison Karlitskaya (desrt) 2013-07-11 16:54:37 UTC
Created attachment 248946 [details] [review]
Implement the Desktop Action specification

For some time, the desktop file specification has supported "additional
application actions".  This is intended to allow for additional methods
of starting an app, such as a mail client having a "Compose New Message"
action that brings up the compose window instead of the folder list.

This patch adds support for this with a relatively minimal API.

In the case that the application is a GApplication and DBusActivatable,
desktop actions are translated into GActions that have been added to the
application with g_action_map_add_action().  This more or less closes
the loop on being able to activate an application with an action
invocation (instead of 'activate').
Comment 33 Jasper St. Pierre (not reading bugmail) 2013-07-11 16:58:41 UTC
Review of attachment 248946 [details] [review]:

This looks good to me. I'm not overly happy about sometimes spawning off a new process according to the Exec line if you can't connect to DBus (I think that should probably be a fail altogether), but it should be OK.
Comment 34 Jasper St. Pierre (not reading bugmail) 2013-07-11 16:59:19 UTC
Review of attachment 248946 [details] [review]:

(whoops, should be marked as ACN)
Comment 35 Matthias Clasen 2013-08-04 22:53:38 UTC
this was committed