Bug 407881

Summary:	new notifications system does not work with Snap package of Discord
Product:	[Plasma] plasmashell	Reporter:	Patrick Silva <bugseforuns>
Component:	Notifications	Assignee:	Kai Uwe Broulik <kde>
Status:	RESOLVED FIXED
Severity:	normal	CC:	nate, plasma-bugs
Priority:	NOR
Version:	5.15.90
Target Milestone:	1.0
Platform:	Arch Linux
OS:	Linux
Latest Commit:	https://commits.kde.org/plasma-workspace/17f1834e98ae061226b10b572c4aeebcaca042bc	Version Fixed In:	5.16.0
Sentry Crash Report:
Attachments:	output of dbus-monitor full output of dbus-monitor

Description Patrick Silva 2019-05-23 20:28:44 UTC

SUMMARY
Notifications from Discord installed via Snap package stopped working
after I install Plasma 5.16 beta on Arch Linux. I downgraded to Plasma 5.15.5
and now Plasma shows the notifications from Discord again.

STEPS TO REPRODUCE
1. install Snap package of Discord
https://snapcraft.io/discord
2. connect to Discord, chat to a friend and wait for a notification
3. 

OBSERVED RESULT
Discord plays a notification sound but Plasma shows no notification

EXPECTED RESULT
Plasma shows notifications from Discord

SOFTWARE/OS VERSIONS
Operating System: Arch Linux
KDE Plasma Version: 5.15.5
KDE Frameworks Version: 5.58.0
Qt Version: 5.12.3

Additional information
Plasma does also not show notifications from Snap Discord on neon dev unstable

Comment 1 Kai Uwe Broulik 2019-05-23 20:43:22 UTC

Can you monitor notification dbus traffic and see what it does?

dbus-monitor interface=org.freedesktop.Notifications

Also, run plasma from console, enabling notification logging 

QT_LOGGING_RULES=org.kde.plasma.notifications=true plasmashell

Comment 2 Patrick Silva 2019-05-23 21:08:53 UTC

Created attachment 120277 [details]
output of dbus-monitor

Comment 3 Kai Uwe Broulik 2019-05-24 06:39:44 UTC

The file is cut off at the top, you might want to redirect the output of dbus-monitor directly into a file.

Comment 4 Nate Graham 2019-05-26 23:57:01 UTC

(In reply to Kai Uwe Broulik from comment #3)
> The file is cut off at the top, you might want to redirect the output of
> dbus-monitor directly into a file.
Here's how to do that:

dbus-monitor interface=org.freedesktop.Notifications >> ~/log.txt

Comment 5 Patrick Silva 2019-05-28 17:56:47 UTC

Created attachment 120360 [details]
full output of dbus-monitor

Comment 6 Kai Uwe Broulik 2019-05-28 21:01:56 UTC

Thank you very much! From the output I can tell that it asks us to replace an existing notification with ID 1767 with a new one. Testing suggests that when replacing a non-existing notification we end up not displaying anything.

The notification spec isn't clear about how to deal with that situation but given it describes it as "An optional ID of an existing notification that this notification is intended to replace." and the fact that it previously worked, I'll adjust our system to just add a new notification in this case. This is still an application bug imho as I have never seen any application do that.

Comment 7 Kai Uwe Broulik 2019-05-29 11:49:21 UTC

Git commit 17f1834e98ae061226b10b572c4aeebcaca042bc by Kai Uwe Broulik.
Committed on 29/05/2019 at 11:47.
Pushed by broulik into branch 'Plasma/5.16'.

[Notifications] Spawn new notification if one to replace doesn't exist

Since the Notification Server knows nothing about what notifications were sent, it tells us a notification was replaced and
uses the ID supplied by the application. Only in the model we then realize it didn't exist.
This means we will potentially reuse the notification ID sent by the application as we just increment a counter.
However, in practice I haven't encountered this issue before (maybe we just never noticed?) but I think this is an
acceptable workaround.
FIXED-IN: 5.16.0

Differential Revision: https://phabricator.kde.org/D21474

M  +2    -0    libnotificationmanager/notificationsmodel.cpp

https://commits.kde.org/plasma-workspace/17f1834e98ae061226b10b572c4aeebcaca042bc