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 781326 - org.gnome.SettingsDaemon.MediaKeys is unowned
org.gnome.SettingsDaemon.MediaKeys is unowned
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: media-keys
3.24.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2017-04-14 19:44 UTC by Patrick Griffis (tingping)
Modified: 2017-04-26 10:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
media-keys: Fix mmkeys D-Bus API to match API docs (2.49 KB, patch)
2017-04-21 13:40 UTC, Bastien Nocera
none Details | Review
media-keys: Fix mmkeys D-Bus API to match API docs (3.30 KB, patch)
2017-04-24 13:56 UTC, Bastien Nocera
none Details | Review
media-keys: Fix mmkeys D-Bus API to match API docs (3.29 KB, patch)
2017-04-24 14:07 UTC, Bastien Nocera
committed Details | Review

Description Patrick Griffis (tingping) 2017-04-14 19:44:35 UTC
This is a regression of 3.24 where gsd-media-keys does not own this bus name so applications error looking for a service file.

As a slight extension to this dozens of applications, including GNOME ones like Totem and Evince, try to use the org.gnome.SettingsDaemon bus name which is now no longer owned. While technically incorrect it would be valuable to keep that working IMO.
Comment 1 Patrick Griffis (tingping) 2017-04-16 02:05:33 UTC
Also it looks like this bus name was not owned in earlier releases too, so now applications must support both.
Comment 2 Jason Gray 2017-04-19 00:49:11 UTC
It seems that mediakeys are going though mpris now? So you remove a known, highly used interface and replace it with mpris? Massive regression, now not only can an app not depend on focus detection to grab and release the mediakeys as to play nice with other open players, but it can't disable mediakeys without also disabling another COMPLETELY UNRELATED service. Good job out a you guys...
Comment 3 Bastien Nocera 2017-04-19 02:12:17 UTC
(In reply to Jason Gray from comment #2)
> It seems that mediakeys are going though mpris now? So you remove a known,
> highly used interface and replace it with mpris? Massive regression, now not
> only can an app not depend on focus detection to grab and release the
> mediakeys as to play nice with other open players, but it can't disable
> mediakeys without also disabling another COMPLETELY UNRELATED service. Good
> job out a you guys...

Aside from you having trouble reading bugs or code properly, attributing malice to a code bug that you failed to test for the months that it was present, and making me reel about how I would reply to this insult to my work during my days off, what exactly did you hope to achieve with this comment?

I'll re-add you to the bug when you've sent me apologies via e-mail.
Comment 4 Bastien Nocera 2017-04-21 13:40:12 UTC
Created attachment 350203 [details] [review]
media-keys: Fix mmkeys D-Bus API to match API docs

Bizarrely, since 2011, gnome-settings-daemon was documented as using
org.gnome.SettingsDaemon.MediaKeys D-Bus name, but everybody ended up
using the org.gnome.SettingsDaemon owned by the daemon instead, and
never reported the discrepancy.

This fixes the code to match the 6-year old API as documented by owning
the org.gnome.SettingsDaemon.MediaKeys D-Bus name.

This patch will need to be backported as far as reasonably possible by
distributions, and all users of the API changed. This would obviously
have been easier if the problem was reported when detected,
committer of this fix included...
Comment 5 Rui Matos 2017-04-24 13:42:20 UTC
Review of attachment 350203 [details] [review]:

with that fixed, seem fine

::: plugins/media-keys/gsd-media-keys-manager.c
@@ +2880,3 @@
         g_debug ("Starting mpris controller");
         manager->priv->mpris_controller = mpris_controller_new ();
+        manager->priv->mmkeys_name_id = g_bus_own_name (G_BUS_TYPE_SESSION,

Since we register the API in on_bus_gotten() we should also claim the name there to avoid the case where we "lose" the race and have a window when we own the name but haven't exported the API yet.
Comment 6 Bastien Nocera 2017-04-24 13:56:40 UTC
Created attachment 350298 [details] [review]
media-keys: Fix mmkeys D-Bus API to match API docs

Bizarrely, since 2011, gnome-settings-daemon was documented as using
org.gnome.SettingsDaemon.MediaKeys D-Bus name, but everybody ended up
using the org.gnome.SettingsDaemon owned by the daemon instead, and
never reported the discrepancy.

This fixes the code to match the 6-year old API as documented by owning
the org.gnome.SettingsDaemon.MediaKeys as well as the
org.gnome.SettingsDaemon D-Bus name to give 3rd-party users time to
adapt their code.

The portion of this patch adding the org.gnome.SettingsDaemon.MediaKeys
name owning will need to be backported as far as reasonably possible by
distributions, and all users of the API changed before GNOME 3.26. This
would obviously have been easier if the problem was reported when
detected, committer of this fix included.
Comment 7 Rui Matos 2017-04-24 14:00:21 UTC
Review of attachment 350298 [details] [review]:

lgtm
Comment 8 Bastien Nocera 2017-04-24 14:07:34 UTC
Created attachment 350299 [details] [review]
media-keys: Fix mmkeys D-Bus API to match API docs

Bizarrely, since 2011, gnome-settings-daemon was documented as using
org.gnome.SettingsDaemon.MediaKeys D-Bus name, but everybody ended up
using the org.gnome.SettingsDaemon owned by the daemon instead, and
never reported the discrepancy.

This fixes the code to match the 6-year old API as documented by owning
the org.gnome.SettingsDaemon.MediaKeys as well as the
org.gnome.SettingsDaemon D-Bus name to give 3rd-party users time to
adapt their code.

The portion of this patch adding the org.gnome.SettingsDaemon.MediaKeys
name owning will need to be backported as far as reasonably possible by
distributions, and all users of the API changed before GNOME 3.26. This
would obviously have been easier if the problem was reported when
detected, committer of this fix included.
Comment 9 Bastien Nocera 2017-04-24 14:10:08 UTC
Attachment 350299 [details] pushed as 42f75ed - media-keys: Fix mmkeys D-Bus API to match API docs

Pushed to gnome-3-24. Similar patches will also be committed for gnome-3-22 and master, without
the "org.gnome.SettingsDaemon" owning though.
Comment 10 Bastien Nocera 2017-04-24 14:37:26 UTC
E-mails have been sent to distributor-list@ and desktop-devel-list@, please pass it on.