Bug 407881 - new notifications system does not work with Snap package of Discord
Summary: new notifications system does not work with Snap package of Discord
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Notifications (show other bugs)
Version: 5.15.90
Platform: Arch Linux Linux
: NOR normal
Target Milestone: 1.0
Assignee: Kai Uwe Broulik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-23 20:28 UTC by Patrick Silva
Modified: 2019-05-29 11:49 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.16.0
Sentry Crash Report:


Attachments
output of dbus-monitor (76.92 KB, text/plain)
2019-05-23 21:08 UTC, Patrick Silva
Details
full output of dbus-monitor (2.93 MB, text/plain)
2019-05-28 17:56 UTC, Patrick Silva
Details

Note You need to log in before you can comment on or make changes to this bug.
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