Skip to content

notification: Add platform-data (with activation_token) to ActionInvoked #791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3v1n0 wants to merge 3 commits into from
+294 −25
Closed

notification: Add platform-data (with activation_token) to ActionInvoked #791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6b5536
notification: Add ActionInvoked2 with platform_data variant
3v1n0 May 5, 2022
37ae801
meson: Generate auto-cleanup functions for dbus objects
3v1n0 Jul 12, 2022
80e5c1f
tests: Check that notifications using ActionInvoked v1 are handled
3v1n0 May 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions .github/workflows/check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,22 @@ jobs:
libglib2.0-dev \
libjson-glib-dev \
libpipewire-0.3-dev \
libportal-dev \
libsystemd-dev \
libtool \
llvm \
make \
python3-gi \
shared-mime-info

- name: Install libportal
run: |
git clone https://github.com/3v1n0/libportal.git -b activation-token
$RUN_CMD apt-get install -y --no-install-recommends meson
$RUN_CMD env -u CFLAGS meson libportal libportal/_build --prefix=/usr --libdir=lib \
$meson_options -Dbackends=[] -Dintrospection=false -Dvapi=false -Ddocs=false
$RUN_CMD ninja -C libportal/_build install
$RUN_CMD rm -rf libportal

- name: Check out xdg-desktop-portal
uses: actions/checkout@v2

Expand Down Expand Up @@ -181,7 +189,6 @@ jobs:
libglib2.0-dev \
libjson-glib-dev \
libpipewire-0.3-dev \
libportal-dev \
libsystemd-dev \
libtool \
llvm \
Expand All @@ -193,6 +200,15 @@ jobs:
- name: Check out xdg-desktop-portal
uses: actions/checkout@v2

- name: Install libportal
run: |
$RUN_CMD apt-get install -y --no-install-recommends meson git
$RUN_CMD git clone https://github.com/3v1n0/libportal.git -b activation-token
$RUN_CMD env -u CFLAGS meson libportal libportal/_build --prefix=/usr --libdir=lib \
$meson_options -Dbackends=[] -Dintrospection=false -Dvapi=false -Ddocs=false
$RUN_CMD ninja -C libportal/_build install
$RUN_CMD rm -rf libportal

- name: Setup test user
run: |
$RUN_CMD adduser --disabled-password --gecos "" tester
Expand Down Expand Up @@ -287,3 +303,4 @@ jobs:

- name: Build xdg-desktop-portal
run: make -j $(getconf _NPROCESSORS_ONLN)

38 changes: 38 additions & 0 deletions data/org.freedesktop.impl.portal.Notification.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,43 @@
<arg type="s" name="action"/>
<arg type="av" name="parameter"/>
</signal>

<!--
ActionInvoked2:
@app_id: App id of the application
@id: the application-provided ID for the notification
@action: the name of the action
@platform_data: array which will contain the platform data
such as desktop-startup-id.
@parameter: array which will contain the target parameter
for the action, if one was specified

Send to the application when a non-exported action is
activated.

Supported keys in the @platform_data vardict include:
<variablelist>
<varlistentry>
<term>activation_token s</term>
<listitem>
<para>
A token that can be used to activate windows or applications
when action is invoked.
</para>
<para>
This can be either an X11-style startup ID or a Wayland
xdg-activation token.
</para>
</listitem>
</varlistentry>
</variablelist>
-->
<signal name="ActionInvoked2">
<arg type="s" name="app_id"/>
<arg type="s" name="id"/>
<arg type="s" name="action"/>
<arg type="a{sv}" name="platform_data"/>
Copy link
Collaborator

@jadahl jadahl Sep 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps name this options? It's what the equivalent thing is called in other portals.

<arg type="av" name="parameter"/>
</signal>
</interface>
</node>
36 changes: 36 additions & 0 deletions data/org.freedesktop.portal.Notification.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,42 @@
<arg type="s" name="action"/>
<arg type="av" name="parameter"/>
</signal>

<!--
ActionInvoked2:
@id: the application-provided ID for the notification
@action: the name of the action
@platform_data: array which will contain the platform data
such as desktop-startup-id.
@parameter: array which will contain the target parameter
for the action, if one was specified

Send to the application when a non-exported action is
activated.

Supported keys in the @platform_data vardict include:
<variablelist>
<varlistentry>
<term>activation_token s</term>
<listitem>
<para>
A token that can be used to activate windows or applications
when action is invoked.
</para>
<para>
This can be either an X11-style startup ID or a Wayland
xdg-activation token.
</para>
</listitem>
</varlistentry>
</variablelist>
-->
<signal name="ActionInvoked2">
<arg type="s" name="id"/>
<arg type="s" name="action"/>
<arg type="a{sv}" name="platform_data"/>
Copy link
Collaborator

@jadahl jadahl Sep 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same about calling it options for consistency.

<arg type="av" name="parameter"/>
</signal>
<property name="version" type="u" access="read"/>
Copy link
Collaborator

@ebassi ebassi Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not bump the version, here? If version == 2, then ActionInvoked has more parameters, otherwise it doesn't.

Unless we assume that all portal implementations aren't checking the version.

I really don't want to have methods called Foo2; that's a great way to cause legibility issues, as well as the potential for typos. Might as well call this signal with another name entirely.

Copy link
Collaborator

@jadahl jadahl Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not how it works; the apps don't "bind" a version it's compatible with, so we cannot change the type signature.

How do you imagine "app1" that is using the current version of the API to continue to work with a new xdg-desktop-portal if the type signature changes?

I think we just have to live with the fact that there will be a "Foo2" here and there. It's also why it's good to more or less always have a vararg so one can add more properties like this without changing the type signature.

</interface>
</node>
6 changes: 3 additions & 3 deletions src/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,23 @@ portal_built_sources = gnome.gdbus_codegen(
interface_prefix: 'org.freedesktop.portal',
namespace: 'XdpDbus',
docbook: 'portal',
autocleanup: 'none',
autocleanup: 'objects',
)
impl_built_sources = gnome.gdbus_codegen(
'xdp-impl-dbus',
sources: portal_impl_sources,
interface_prefix: 'org.freedesktop.impl.portal',
namespace: 'XdpDbusImpl',
docbook: 'portal',
autocleanup: 'none',
autocleanup: 'objects',
)
if have_geoclue
geoclue_built_sources = gnome.gdbus_codegen(
'geoclue-dbus',
sources: 'org.freedesktop.GeoClue2.Client.xml',
interface_prefix: 'org.freedesktop.GeoClue2',
namespace: 'Geoclue',
autocleanup: 'none',
autocleanup: 'objects',
)
else
geoclue_built_sources = []
Expand Down
59 changes: 47 additions & 12 deletions src/notification.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,24 +483,49 @@ action_invoked (GDBusConnection *connection,
Pair p;
const char *action;
GVariant *param;
GVariant *platform_data = NULL;
const char *sender;

g_variant_get (parameters, "(&s&s&s@av)", &p.app_id, &p.id, &action, &param);
if (g_str_equal (signal_name, "ActionInvoked2"))
{
g_variant_get (parameters, "(&s&s&s@a{sv}@av)", &p.app_id, &p.id, &action,
&platform_data, &param);
}
else
{
g_variant_get (parameters, "(&s&s&s@av)", &p.app_id, &p.id, &action,
&param);
}

sender = g_hash_table_lookup (active, &p);
if (sender == NULL)
return;

g_dbus_connection_emit_signal (connection,
sender,
"/org/freedesktop/portal/desktop",
"org.freedesktop.portal.Notification",
"ActionInvoked",
g_variant_new ("(ss@av)",
p.id, action,
param),
NULL);

if (g_str_equal (signal_name, "ActionInvoked2"))
{
g_dbus_connection_emit_signal (connection,
sender,
"/org/freedesktop/portal/desktop",
"org.freedesktop.portal.Notification",
"ActionInvoked2",
g_variant_new ("(ss@a{sv}@av)",
p.id, action,
platform_data,
param),
NULL);
}
else
{
g_dbus_connection_emit_signal (connection,
sender,
"/org/freedesktop/portal/desktop",
"org.freedesktop.portal.Notification",
"ActionInvoked",
g_variant_new ("(ss@av)",
p.id, action,
param),
NULL);
}
}

static void
Expand Down Expand Up @@ -546,7 +571,7 @@ notification_iface_init (XdpDbusNotificationIface *iface)
static void
notification_init (Notification *notification)
{
xdp_dbus_notification_set_version (XDP_DBUS_NOTIFICATION (notification), 1);
xdp_dbus_notification_set_version (XDP_DBUS_NOTIFICATION (notification), 2);
}

static void
Expand Down Expand Up @@ -586,6 +611,16 @@ notification_create (GDBusConnection *connection,
action_invoked,
NULL, NULL);

g_dbus_connection_signal_subscribe (connection,
dbus_name,
"org.freedesktop.impl.portal.Notification",
"ActionInvoked2",
DESKTOP_PORTAL_OBJECT_PATH,
NULL,
G_DBUS_SIGNAL_FLAGS_NONE,
action_invoked,
NULL, NULL);

g_dbus_connection_signal_subscribe (connection,
"org.freedesktop.DBus",
"org.freedesktop.DBus",
Expand Down
47 changes: 41 additions & 6 deletions tests/backend/notification.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ typedef struct {
char *app_id;
char *id;
char *action;
gint force_version;
GVariant *platform_data;
} ActionData;

static gboolean
Expand All @@ -23,16 +25,30 @@ invoke_action (gpointer data)

g_variant_builder_init (&builder, G_VARIANT_TYPE ("av"));

g_print ("emitting ActionInvoked\n");
xdp_dbus_impl_notification_emit_action_invoked (adata->impl,
adata->app_id,
adata->id,
adata->action,
g_variant_builder_end (&builder));
if (adata->force_version == 1)
{
g_message ("emitting ActionInvoked");
xdp_dbus_impl_notification_emit_action_invoked (adata->impl,
adata->app_id,
adata->id,
adata->action,
g_variant_builder_end (&builder));
}
else
{
g_message ("emitting ActionInvoked2");
xdp_dbus_impl_notification_emit_action_invoked2 (adata->impl,
adata->app_id,
adata->id,
adata->action,
adata->platform_data,
g_variant_builder_end (&builder));
}

g_free (adata->app_id);
g_free (adata->id);
g_free (adata->action);
g_variant_unref (adata->platform_data);
g_free (adata);

return G_SOURCE_REMOVE;
Expand Down Expand Up @@ -70,12 +86,31 @@ handle_add_notification (XdpDbusImplNotification *object,
delay = g_key_file_get_integer (keyfile, "backend", "delay", NULL);
if (delay != 0)
{
g_autofree char *platform_data_s = NULL;
g_autoptr(GVariant) platform_data = NULL;
ActionData *data;

platform_data_s = g_key_file_get_string (keyfile, "notification", "platform_data", NULL);
if (platform_data_s)
{
platform_data = g_variant_parse (G_VARIANT_TYPE_VARDICT, platform_data_s, NULL, NULL, &error);
g_assert_no_error (error);
}
else
{
GVariantBuilder builder;

g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
platform_data = g_variant_ref_sink (g_variant_builder_end (&builder));
}

data = g_new (ActionData, 1);
data->impl = object;
data->app_id = g_strdup (arg_app_id);
data->id = g_strdup (arg_id);
data->platform_data = g_steal_pointer (&platform_data);
data->action = g_key_file_get_string (keyfile, "notification", "action", NULL);
data->force_version = g_key_file_get_integer (keyfile, "notification", "force-version", NULL);

g_timeout_add (delay, invoke_action, data);
}
Expand Down
Loading