Differential D21087

Fix name and set desktop entry for notifications
ClosedPublic
Actions

Authored by nicolasfella on May 8 2019, 8:21 PM.

Details

Reviewers

drosca

Group Reviewers

Falkon

Commits

R875:e3cca98e0dc4: Fix name and set desktop entry for notifications

Summary

With the Plasma notification rewrite the name passed with the notification is user-visible, so use "Falkon" instead of "falkon here". Also pass the desktop entry. This allows Plasma to embed the app icon in the notification

Test Plan

Before:

After:

Diff Detail

Repository

R875 Falkon

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.May 8 2019, 8:21 PM

Restricted Application added a project: Falkon. · View Herald TranscriptMay 8 2019, 8:21 PM

Restricted Application added a subscriber: falkon. · View Herald Transcript

nicolasfella requested review of this revision.May 8 2019, 8:21 PM

Harbormaster completed remote builds in B11674: Diff 57777.May 8 2019, 8:21 PM

nicolasfella edited the test plan for this revision. (Show Details)May 8 2019, 8:23 PM

nicolasfella edited the test plan for this revision. (Show Details)

The spec has always said about the app_name field:

This should be the application's formal name, rather than some sort of ID. An example would be "FredApp E-Mail Client," rather than "fredapp-email-client."

nicolasfella edited the test plan for this revision. (Show Details)May 8 2019, 8:25 PM

nicolasfella edited the test plan for this revision. (Show Details)

nicolasfella edited the test plan for this revision. (Show Details)May 8 2019, 8:27 PM

kossebau added a subscriber: kossebau.May 8 2019, 8:49 PM

kossebau added inline comments.

src/lib/notifications/desktopnotificationsfactory.cpp
94	Saw by accident: Beware that this might be with or without ".desktop" suffix in reality (cmp. recent discussion with quassel) ;) No idea what value is expected here, you might to chop or append as needed. > https://bugreports.qt.io/browse/QTBUG-75521

nicolasfella added inline comments.May 8 2019, 8:56 PM

src/lib/notifications/desktopnotificationsfactory.cpp
94	It is set without the suffix here https://cgit.kde.org/falkon.git/tree/src/lib/app/mainapplication.cpp#n119 So I can safely assume I get it without suffix, can I?

kossebau added inline comments.May 8 2019, 9:02 PM

src/lib/notifications/desktopnotificationsfactory.cpp
94	Oops, my bad, had missed this was for Falkon and not some lib, was confused after having scanned some things and just made a drive-by comment. But made a note to myself to check tomorrow/WE all the KF libs which might be used in non-KAboutData-driven apps, as lxr.kde.org showed me now some hits while I was made aware by myself of this challenge. Sorry for noise, code on :)