POC: Make kstatusnotifieritem available without dbus
ClosedPublic

Authored by vonreth on Feb 12 2020, 9:26 PM.

Details

Summary

Well yes that extremely ugly and some real refactoring would be needed....

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.
vonreth created this revision.Feb 12 2020, 9:26 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 12 2020, 9:26 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
vonreth requested review of this revision.Feb 12 2020, 9:26 PM
vonreth retitled this revision from Make kstatusnotifieritem available without dbus to POC: Make kstatusnotifieritem available without dbus.Feb 12 2020, 9:28 PM
vonreth edited the summary of this revision. (Show Details)
vonreth added reviewers: bcooksley, jjazeix.

While preparing this I discovered that we should also review the locations where if MAC is used, as thats not the only platform that should use the systray icon

bcooksley added inline comments.Feb 13 2020, 6:45 AM
src/kstatusnotifieritem.cpp
48

New header?

617–618

Won't this mean that showMessage() is a no-op on Windows systems without D-Bus?
I would have thought that systemTrayIcon->showMessage() should be used here as well?

888

From my reading of the setLegacySystemTrayEnabled() won't setting it to false explicitly disable any form of system tray icon?

jjazeix added a comment.EditedFeb 13 2020, 7:37 AM

thank you!
I did the same but way worse.
I think we can at least push this fix to remove all failing jobs then clean it up? This should fix half the failing jobs.

CMakeLists.txt
94

this is not the aim of this diff, but I looked at the code, I found this line was duplicated from line 43

Have you had a chance to look into this @broulik?

Can we push it as this to fix the Windows builds? It also breaks a few ones in https://binary-factory.kde.org/view/Windows%2064-bit/

This revision was not accepted when it landed; it landed in state Needs Review.Fri, Mar 6, 9:42 AM
This revision was automatically updated to reflect the committed changes.