Merge lp:~nobuto/notify-osd/fix_dnd_is_screensaver_inhibited into lp:notify-osd

Proposed by Nobuto Murata
Status: Merged
Approved by: Iain Lane
Approved revision: 489
Merged at revision: 494
Proposed branch: lp:~nobuto/notify-osd/fix_dnd_is_screensaver_inhibited
Merge into: lp:notify-osd
Diff against target: 178 lines (+62/-44)
4 files modified
debian/changelog (+7/-0)
src/dnd.c (+52/-41)
src/dnd.h (+1/-1)
tests/test-dnd.c (+2/-2)
To merge this branch: bzr merge lp:~nobuto/notify-osd/fix_dnd_is_screensaver_inhibited
Reviewer Review Type Date Requested Status
Iain Lane Approve
Review via email: mp+255538@code.launchpad.net

Description of the change

This merge proposal fixes LP: #1440825.

However the dnd_is_screensaver_inhibited is broken at least for a few years now, users of caffeine[1] or indicator-session-idle-inhibit[2] might feel the "fix" is a regression, i.e. notification suddenly stops working by notify-osd update.

[1] https://launchpad.net/caffeine
[2] https://github.com/oyvindstegard/indicator-session-idle-inhibit

I'm not sure which is the right way. A cleaner way to inhibit notifications(e.g. example org.freedesktop.Notifications.X-Inhibit dbus method) might be needed though.

To post a comment you must log in.
Revision history for this message
Lars Karlitski (larsu) wrote :

I'm not sure. Inhibiting the session from going idle doesn't necessarily mean "don't show notifications" to me...

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> I'm not sure. Inhibiting the session from going idle doesn't necessarily mean
> "don't show notifications" to me...

Yeah, I agree. But still this is what was supposed to happen before (so not sure we should resume it).
However, it is even true that 99% of apps that inhibit the screensaver are media-players which might use this strategy.
Do we have a list of apps using this API?

Having an inhibit "notifications" (API is extendible) would be nicer, though.
Also, I'm not sure whether unity does support this currently (in case we've to fix it).

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> Also, I'm not sure whether unity does support this currently (in case we've to
> fix it).

Ok, it seems unity works fine when inhibited, but this is not checked when the screensaver is launched by logind or manually (but that's not idle).

Revision history for this message
Iain Lane (laney) wrote :

I tried it, it works good - the code seems fine. Let's get this in for wily.

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2015-01-26 14:16:51 +0000
+++ debian/changelog 2015-04-08 16:29:00 +0000
@@ -1,3 +1,10 @@
1notify-osd (0.9.35+15.04.20150126-0ubuntu2) UNRELEASED; urgency=medium
2
3 * dnd_is_screensaver_inhibited no longer works due to obsoleted dbus
4 method. replace it by an equivalent dbus call, LP: #1440825
5
6 -- Nobuto Murata <nobuto@ubuntu.com> Tue, 07 Apr 2015 17:47:18 +0900
7
1notify-osd (0.9.35+15.04.20150126-0ubuntu1) vivid; urgency=low8notify-osd (0.9.35+15.04.20150126-0ubuntu1) vivid; urgency=low
29
3 [ Alberts Muktupāvels ]10 [ Alberts Muktupāvels ]
411
=== modified file 'src/dnd.c'
--- src/dnd.c 2012-01-24 08:14:04 +0000
+++ src/dnd.c 2015-04-08 16:29:00 +0000
@@ -45,6 +45,7 @@
45#include "dbus.h"45#include "dbus.h"
4646
47static DBusGProxy *gsmgr = NULL;47static DBusGProxy *gsmgr = NULL;
48static DBusGProxy *gscrsvr = NULL;
4849
49gboolean50gboolean
50dnd_is_xscreensaver_active ()51dnd_is_xscreensaver_active ()
@@ -98,55 +99,65 @@
98}99}
99100
100static DBusGProxy*101static DBusGProxy*
101get_screensaver_proxy (void)102get_gnomesession_proxy (void)
102{103{
103 if (gsmgr == NULL)104 if (gsmgr == NULL)
104 {105 {
105 DBusGConnection *connection = dbus_get_connection ();106 DBusGConnection *connection = dbus_get_connection ();
106 gsmgr = dbus_g_proxy_new_for_name (connection,107 gsmgr = dbus_g_proxy_new_for_name (connection,
108 "org.gnome.SessionManager",
109 "/org/gnome/SessionManager",
110 "org.gnome.SessionManager");
111 }
112
113 return gsmgr;
114}
115
116gboolean
117dnd_is_idle_inhibited ()
118{
119 GError *error = NULL;
120 gboolean inhibited = FALSE;
121 guint idle = 8; // 8: Inhibit the session being marked as idle
122
123 if (! get_gnomesession_proxy ())
124 return FALSE;
125
126 dbus_g_proxy_call_with_timeout (
127 gsmgr, "IsInhibited", 2000, &error,
128 G_TYPE_UINT, idle,
129 G_TYPE_INVALID,
130 G_TYPE_BOOLEAN, &inhibited,
131 G_TYPE_INVALID);
132
133 if (error)
134 {
135 g_warning ("dnd_is_idle_inhibited(): "
136 "got error \"%s\"\n",
137 error->message);
138 g_error_free (error);
139 error = NULL;
140 }
141
142 if (inhibited)
143 g_debug ("Session idleness has been inhibited");
144
145 return inhibited;
146}
147
148static DBusGProxy*
149get_screensaver_proxy (void)
150{
151 if (gscrsvr == NULL)
152 {
153 DBusGConnection *connection = dbus_get_connection ();
154 gscrsvr = dbus_g_proxy_new_for_name (connection,
107 "org.gnome.ScreenSaver",155 "org.gnome.ScreenSaver",
108 "/org/gnome/ScreenSaver",156 "/org/gnome/ScreenSaver",
109 "org.gnome.ScreenSaver");157 "org.gnome.ScreenSaver");
110 }158 }
111159
112 return gsmgr;160 return gscrsvr;
113}
114
115gboolean
116dnd_is_screensaver_inhibited ()
117{
118 GError *error = NULL;
119 gboolean inhibited = FALSE;
120 char **list;
121
122 if (! get_screensaver_proxy ())
123 return FALSE;
124
125 if (dbus_g_proxy_call_with_timeout (
126 gsmgr, "GetInhibitors", 2000, &error,
127 G_TYPE_INVALID,
128 G_TYPE_STRV, &list,
129 G_TYPE_INVALID))
130 {
131 if (error)
132 {
133 g_warning ("dnd_is_screensaver_inhibited(): "
134 "got error \"%s\"\n",
135 error->message);
136 g_error_free (error);
137 error = NULL;
138 }
139
140 /* if the list is not empty, the screensaver is inhibited */
141 if (*list)
142 {
143 inhibited = TRUE;
144 g_debug ("Screensaver has been inhibited");
145 }
146 g_strfreev (list);
147 }
148
149 return inhibited;
150}161}
151162
152gboolean163gboolean
@@ -159,7 +170,7 @@
159 return FALSE;170 return FALSE;
160171
161 dbus_g_proxy_call_with_timeout (172 dbus_g_proxy_call_with_timeout (
162 gsmgr, "GetActive", 2000, &error,173 gscrsvr, "GetActive", 2000, &error,
163 G_TYPE_INVALID,174 G_TYPE_INVALID,
164 G_TYPE_BOOLEAN, &active,175 G_TYPE_BOOLEAN, &active,
165 G_TYPE_INVALID);176 G_TYPE_INVALID);
@@ -222,7 +233,7 @@
222 return (dnd_is_online_presence_dnd()233 return (dnd_is_online_presence_dnd()
223 || dnd_is_xscreensaver_active()234 || dnd_is_xscreensaver_active()
224 || dnd_is_screensaver_active()235 || dnd_is_screensaver_active()
225 || dnd_is_screensaver_inhibited()236 || dnd_is_idle_inhibited()
226 || dnd_has_one_fullscreen_window()237 || dnd_has_one_fullscreen_window()
227 );238 );
228}239}
229240
=== modified file 'src/dnd.h'
--- src/dnd.h 2009-06-02 16:28:26 +0000
+++ src/dnd.h 2015-04-08 16:29:00 +0000
@@ -39,7 +39,7 @@
39dnd_is_xscreensaver_active (void);39dnd_is_xscreensaver_active (void);
4040
41gboolean41gboolean
42dnd_is_screensaver_inhibited (void);42dnd_is_idle_inhibited (void);
4343
44gboolean44gboolean
45dnd_is_screensaver_active (void);45dnd_is_screensaver_active (void);
4646
=== modified file 'tests/test-dnd.c'
--- tests/test-dnd.c 2011-07-01 18:23:39 +0000
+++ tests/test-dnd.c 2015-04-08 16:29:00 +0000
@@ -41,10 +41,10 @@
41void41void
42test_dnd_screensaver (gpointer fixture, gconstpointer user_data)42test_dnd_screensaver (gpointer fixture, gconstpointer user_data)
43{43{
44 gboolean test = dnd_is_screensaver_inhibited();44 gboolean test = dnd_is_idle_inhibited();
4545
46 if (test)46 if (test)
47 g_debug ("screensaver is inhibited");47 g_debug ("idleness is inhibited");
4848
49 test = dnd_is_screensaver_active();49 test = dnd_is_screensaver_active();
5050

Subscribers

People subscribed via source and target branches