Remove checks for notification service and fallback to KPassivePopup
ClosedPublic

Authored by nicolasfella on Apr 20 2020, 7:20 PM.

Details

Summary

Alternative patch to D26605 and D26604

KNotifications has proper support for all major platforms (freedesktop, windows, macOS, Android). The KPassivePopup fallback is only relevant for Linux when no FDO notification daemon is running. IMO the case of the user having no notification daemon at all is not worth supporting since the users intention is very clearly to not have notifications.

Previously the KPassivePopup would sometimes appear when the server has crashed. Instead we now always make a call to the notification service, which then get's DBus-activated if it isn't running. In the Plasma case there is the plasma_waitforname helper that makes sure all notifications are captured until Plasma has restarted and forwards them to Plasma. This way no notifications are lost.

This allows to drop a good amount of code. It reduces the dependency on Widgets so we can eventually get rid of it which is nice for Android. Furthermore the reduced complexity will make it easier to implement our plans for KF6

Test Plan

Tested with new notification tester app.

Killed Plasma, sent notification, restarted Plasma, notification showed up

Diff Detail

Repository
R289 KNotifications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nicolasfella created this revision.Apr 20 2020, 7:20 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 20 2020, 7:20 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
nicolasfella requested review of this revision.Apr 20 2020, 7:20 PM
nicolasfella edited the summary of this revision. (Show Details)Apr 20 2020, 7:28 PM
nicolasfella edited the test plan for this revision. (Show Details)
broulik accepted this revision.Apr 29 2020, 2:13 PM

Verified that close signal works and that quitting plasma, sending a notification, then has the notification show up when plasma is started.
Please wait until after Frameworks tagging before pushing this

This revision is now accepted and ready to land.Apr 29 2020, 2:13 PM
This revision was automatically updated to reflect the committed changes.