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 792777 - g_notification_set_urgent() unconditionally sets G_NOTIFICATION_PRIORITY_URGENT, ignores parameter
g_notification_set_urgent() unconditionally sets G_NOTIFICATION_PRIORITY_URGE...
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.54.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2018-01-22 11:16 UTC by Matthew W. S. Bell
Modified: 2018-01-23 14:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GNotification: Don't ignore set_urgent()'s argument (1021 bytes, patch)
2018-01-23 10:53 UTC, Bastien Nocera
none Details | Review
GNotification: Don't ignore set_urgent()'s argument (985 bytes, patch)
2018-01-23 11:05 UTC, Bastien Nocera
committed Details | Review

Description Matthew W. S. Bell 2018-01-22 11:16:09 UTC
After g_notification_set_urgent(…, FALSE), the expectation is not that the notification is urgent.
Comment 1 Philip Withnall 2018-01-22 11:22:25 UTC
g_notification_set_urgent() has been deprecated in favour of g_notification_set_priority() since GLib 2.41.2. Use that instead.

I’ll push a commit to make the deprecation notice more obvious.
Comment 2 Matthew W. S. Bell 2018-01-23 05:22:32 UTC
Fully aware of the deprecated notice. Rather than than rushing to absolve oneself of all responsibility, please would you document the failing and resolve as WONTFIX.
Comment 3 Bastien Nocera 2018-01-23 10:52:16 UTC
(In reply to Matthew W. S. Bell from comment #2)
> Fully aware of the deprecated notice. Rather than than rushing to absolve
> oneself of all responsibility, please would you document the failing and
> resolve as WONTFIX.

commit 01098e34c188b4ec93944e14dbece6818d786aec
Author: Lars Uebernickel <lars.uebernickel@canonical.com>
Date:   Sun Jun 15 15:42:31 2014 +0200

Not sure where you're seeing a rush there. Been broken in the same way for 3 1/2 years.
Comment 4 Bastien Nocera 2018-01-23 10:53:58 UTC
Created attachment 367292 [details] [review]
GNotification: Don't ignore set_urgent()'s argument

set_urgent() would behave is if @urgent was always true. The regression
was introduced in commit 01098e34c188b4ec93944e14dbece6818d786aec
Comment 5 Philip Withnall 2018-01-23 10:57:23 UTC
Review of attachment 367292 [details] [review]:

::: gio/gnotification.c
@@ +323,3 @@
+    notification->priority |= G_NOTIFICATION_PRIORITY_URGENT;
+  else
+    notification->priority &= ~G_NOTIFICATION_PRIORITY_URGENT;

priority is an enum, not a bit mask. If (!urgent), the best we can do is set it to G_NOTIFICATION_PRIORITY_NORMAL.
Comment 6 Philip Withnall 2018-01-23 10:59:35 UTC
(In reply to Matthew W. S. Bell from comment #2)
> Fully aware of the deprecated notice. Rather than than rushing to absolve
> oneself of all responsibility, please would you document the failing and
> resolve as WONTFIX.

The deprecation is documented, and the particular way the bug is resolved does not really matter. I rush because 10 bugs come in per day and this one is really not important.
Comment 7 Bastien Nocera 2018-01-23 11:05:32 UTC
Created attachment 367293 [details] [review]
GNotification: Don't ignore set_urgent()'s argument

set_urgent() would behave is if @urgent was always true. The regression
was introduced in commit 01098e34c188b4ec93944e14dbece6818d786aec
Comment 8 Philip Withnall 2018-01-23 11:44:27 UTC
Review of attachment 367293 [details] [review]:

Sure, thanks.
Comment 9 Philip Withnall 2018-01-23 14:24:49 UTC
Pushed to master. Thanks Bastien!

Attachment 367293 [details] pushed as 801accf - GNotification: Don't ignore set_urgent()'s argument