WIP feat(notifications): Replace deprecated snorenotify with knotifications

[sphaerophoria]

Proof of concept swap of snorenotify with knotifications.

• Implementation seems to work pretty well on my machine (Archlinux, KNotofications 5.78, Gnome 3.38)
  • Unfortunately user icon is no longer shown on gnome for me, I believe this is due to a problem on gnome's side with priority of icon/image-data
  • 
• Local appimage build works correctly
• Local flatpak build works but notifications are not replacing themselves
• Seems to work fine on Ubuntu 16.04
  
• Windows build is almost certainly broken.
• Currently working on enabling in OSX
  • Install script currently broken on OSX, but after manually copying qTox.notifyrc to the right spot
    

• Commits follow our git commit guidelines

---

This change is 

[nurupo]

Looks like the Windows build is missing https://commits.kde.org/extra-cmake-modules.

Also, you might notice that KNotifications can optionally use SnoreToast on Windows - don't even bother trying to make it use that. KDE devs have said that SnoreToast can only be built using MSVC, it can't be built with MingW-W64 as it's missing some WinRT headers, so we won't be able to build it.

I can help with getting KNotifications build on Windows if you want, most of that Windows build script madness is written by me :P If you want to do that on your own then it's fine by me, just make sure you build it as a shared dll and handle the dll dependency check correctly.

Btw, your editor seem to have modified the inline libvpx patch, I don't think you have meant to touch it.

[sphaerophoria]

Thanks @nurupo! If you're offering to help out with the build script stuff I'll gladly accept :P. I was planning on handling the windows build last so if you handle that it's unlikely that I'll do any conflicting work with you.

Thanks for the heads up on the patches, I'll clean those up when I next look at this (maybe tomorrow or the next day)

[nurupo]

Ok, working on Windows build script then. You can undo the vpx patch modifications, if it would cause conflicts they would be minimal anyway.

[nurupo]

I'm very annoyed with this whole KDE Frameworks shebang. To build KNotifications you need to build:

• CMake Extra Modules
• KF5WindowSystem
• KF5Config
• KF5CoreAddons
• maybe something more, I'm just at the KF5Config building part now...

All of this just to use KNotifications as a QSystemTrayIcon abstraction layer, which could be called directly without having to build all of this... It doesn't look like using KNotifications on Windows makes much sense. At least not until the SnoreToast backend is usable.

I'm going to keep working on it, just noting that the amount of deps it pulls is ridiculous.

[sphaerophoria]

Oof, I can take a look if it's irritating you.

All of this just to use KNotifications as a QSystemTrayIcon abstraction layer

I think this is a bit of an unfair assessment of what it does. On linux it handles dispatching freedesktop notifications over dbus. On mac it dispatches to the OSX notification through an objective C wrapper. On windows it looks like it should be using SnoreToast to show notifications with text content.

Also, you might notice that KNotifications can optionally use SnoreToast on Windows - don't even bother trying to make it use that. KDE devs have said that SnoreToast can only be built using MSVC, it can't be built with MingW-W64 as it's missing some WinRT headers, so we won't be able to build it.

Didn't we have snoretoast already with snorenotify? Without it I think KNotifications will fall back on a worse notification system which would end up with a worse user experience on windows. I can look into it a little more closely though since I haven't even started to cover the windows side yet.

[nurupo]

Didn't we have snoretoast already with snorenotify?

See #6238

[sphaerophoria]

It doesn't look like using KNotifications on Windows makes much sense. At least not until the SnoreToast backend is usable.

After re-reading this and looking at the KNotifications implementation... I agree with you. I wouldn't bother doing all this just for the QApplication::alert() on windows unless we're going to pull in SnoreToast as well. If we're not going to do that I might as well just build without DESKTOP_NOTIFICATIONS on windows for now and use the fallback qtox behavior.

[nurupo]

Well, I'm going to finish building KNotifications and we will see how it works on Windows and if we want to keep it. I just have KF5CoreAddons left to build, hopefully it doesn't pull any deps on me.

Also, qTox has a fallback notifications behavior? What does it do as the fallback?

[sphaerophoria]

Also, qTox has a fallback notifications behavior? What does it do as the fallback?

It just calls the normal QApplication::alert() function. IIIRC makes the taskbar and tray icon blink on windows

#if DESKTOP_NOTIFICATIONS
                if (!settings.getDesktopNotify()) {
                    QApplication::alert(currentWindow);
                }
#else
                QApplication::alert(currentWindow);
#endif

[nurupo]

KNotifications build failing because it can't find Qt5DBus, which we obviously don't build since it's a Windows Qt build... Am I misunderstanding something and Qt5DBus actually works on Windows?

https://github.com/KDE/knotifications/blob/703171e26e9b804402f3cca50b60b375a0311be3/CMakeLists.txt#L93-L94

[nurupo]

It just calls the normal QApplication::alert() function. IIIRC makes the taskbar and tray icon blink on windows

Weird that it doesn't use QSystemTrayIcon.

[nurupo]

DBus on Windows is used by KDE, Gnome and commercial applications.

Huh. I don't really get what is DBus needed for. It's used for IPC, but we don't want to IPC our notifications to any other program, in fact it would be bad if some other program reads your private chat messages.

I will try patching DBus out of CMakeLists.txt and see if it builds without.

[nurupo]

Disabling DBus checks from CMakeLists.txt didn't work out, the build kept failing. I could patch the cpp code to remove the DBus backend usage that was failing the build, but I decided not to fight it and build Qt5DBus.

After Qt5DBus, it asked for either Canberra or Phonot4Qt5. I have added Phonon4Qt5 build and then tried to build Phonon DirectShow backend for it, but the Phonon DirectShow kept failing because it's too old to be used with the latest Phonon4Qt5 release and I couldn't find any other Windows backends nor downgrade Phonon4Qt5 (older versions require Qt4...), so I have left Phonon4Qt5 without any backends for now.

With that, KNotifications got successfully built.

However, qTox was failing because it couldn't find headers for KNotifications:

[ 79%] Building CXX object CMakeFiles/qtox_static.dir/src/platform/desktop_notifications/desktopnotify.cpp.obj
/build/qtox/src/platform/desktop_notifications/desktopnotify.cpp:26:10: fatal error: KF5/knotifications_version.h: No such file or directory
 #include <KF5/knotifications_version.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [CMakeFiles/qtox_static.dir/build.make:2154: CMakeFiles/qtox_static.dir/src/platform/desktop_notifications/desktopnotify.cpp.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:404: CMakeFiles/qtox_static.dir/all] Error 2
make: *** [Makefile:141: all] Error 2

It looks like that should be #include <knotifications_version.h>, without the "KF5" part, as KF5NotificationsTargets.cmake already includes it:

INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/KF5/KNotifications;${_IMPORT_PREFIX}/include/KF5"

So I have changed it to #include <knotifications_version.h> and qTox now builds successfully too, at least the x86_64 release one, haven't tested other configurations yet. Should be good enough to test KNotifications, if it's worth the 7 extra deps on Windows: Qt5DBus, CMake Extra Modules, KF5WindowSystem, KF5Config, KF5CoreAddons, Phonon, (Phonon DirectShow), and KNotifications itself.

Note that if you have been playing with the Windows build locally, you would need to re-build libqt (i.e. rm -rf it from the dep-cache directory) as it needs to build Qt5DBus.

Here are the changes: nurupo@68c67e8

[Monsterovich]

sudo apt install libkf5notifications5 
Reading package lists... Done
Building dependency tree       
Reading state information... Done
The following additional packages will be installed:
  kwayland-data kwayland-integration libdbusmenu-qt5-2 libfam0 libkf5config-bin libkf5config-data libkf5configcore5 libkf5coreaddons-data libkf5coreaddons5
  libkf5guiaddons5 libkf5idletime5 libkf5notifications-data libkf5waylandclient5 libkf5windowsystem-data libkf5windowsystem5 libqt5texttospeech5 libqt5waylandclient5
  libqt5waylandcompositor5 qtwayland5
Suggested packages:
  fam
The following NEW packages will be installed:
  kwayland-data kwayland-integration libdbusmenu-qt5-2 libfam0 libkf5config-bin libkf5config-data libkf5configcore5 libkf5coreaddons-data libkf5coreaddons5
  libkf5guiaddons5 libkf5idletime5 libkf5notifications-data libkf5notifications5 libkf5waylandclient5 libkf5windowsystem-data libkf5windowsystem5 libqt5texttospeech5
  libqt5waylandclient5 libqt5waylandcompositor5 qtwayland5
0 upgraded, 20 newly installed, 0 to remove and 0 not upgraded.
Need to get 1950 kB of archives.
After this operation, 11.0 MB of additional disk space will be used.

Well, I guess we should stick to snorenotify. knotifications looks bloat compared to snorenotify. Why replace snorenotify if it's working?

[nurupo]

See (1) in #6238.

[Monsterovich]

Is there any implementation of windows notifications that can be build under mingw? WinRT api which is used to implement notifications in SnoreToasts and many other tools and libraries seems to be available only with MSVC.

[sphaerophoria]

Disabling DBus checks from CMakeLists.txt didn't work out, the build kept failing. I could patch the cpp code to remove the DBus backend usage that was failing the build, but I decided not to fight it and build Qt5DBus.

After Qt5DBus, it asked for either Canberra or Phonot4Qt5. I have added Phonon4Qt5 build and then tried to build Phonon DirectShow backend for it, but the Phonon DirectShow kept failing because it's too old to be used with the latest Phonon4Qt5 release and I couldn't find any other Windows backends nor downgrade Phonon4Qt5 (older versions require Qt4...), so I have left Phonon4Qt5 without any backends for now.

With that, KNotifications got successfully built.

However, qTox was failing because it couldn't find headers for KNotifications:

[ 79%] Building CXX object CMakeFiles/qtox_static.dir/src/platform/desktop_notifications/desktopnotify.cpp.obj
/build/qtox/src/platform/desktop_notifications/desktopnotify.cpp:26:10: fatal error: KF5/knotifications_version.h: No such file or directory
 #include <KF5/knotifications_version.h>
          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[2]: *** [CMakeFiles/qtox_static.dir/build.make:2154: CMakeFiles/qtox_static.dir/src/platform/desktop_notifications/desktopnotify.cpp.obj] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [CMakeFiles/Makefile2:404: CMakeFiles/qtox_static.dir/all] Error 2
make: *** [Makefile:141: all] Error 2

It looks like that should be #include <knotifications_version.h>, without the "KF5" part, as KF5NotificationsTargets.cmake already includes it:

INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include/KF5/KNotifications;${_IMPORT_PREFIX}/include/KF5"

So I have changed it to #include <knotifications_version.h> and qTox now builds successfully too, at least the x86_64 release one, haven't tested other configurations yet. Should be good enough to test KNotifications, if it's worth the 7 extra deps on Windows: Qt5DBus, CMake Extra Modules, KF5WindowSystem, KF5Config, KF5CoreAddons, Phonon, (Phonon DirectShow), and KNotifications itself.

Note that if you have been playing with the Windows build locally, you would need to re-build libqt (i.e. rm -rf it from the dep-cache directory) as it needs to build Qt5DBus.

Here are the changes: nurupo@68c67e8

Awesome thanks @nurupo that's good stuff. I went through the dependency hell on OSX as well. No brew support so I had to install ECM/kconfig/kcoreaddons/kwindowsystem/phonon manually myself as well. What a PITA. In theory we might be able to leverage some of the new dependencies (phonon could replace some of our AV stuff) but that seems unlikely to be worth it... so it just becomes a question if the bloat is worth it over rolling our own notification implementation for each platform.

[sphaerophoria]

Hm, looks like for mac os we need to install the configuration files for knotifications into "/Library/Application Support". My gut says that this can't be done with a dmg but also I have no idea how mac os works. We could also install to ~/Library/Application Support on program start but that seems worse conceptually.

[nurupo]

Can someone test KNotifications on Windows? To see if it makes sense to keep it, if it's any better than if we used QSystemTrayIcon to make the temporary system tray notification popups to appear.

I have tried testing it on Linux under Wine, but that makes qTox tray icon integrate into my Plasma DE, so I'm not sure how representative it is of Windows behavior. The behavior I have observed is that whenever I get a new message, a notification sound plays and the qTox icon in the tray and the task bar keeps blinking with a different color, but there is no popup with the message text of any kind.

[sphaerophoria]

@nurupo I'll take a look after I wrap up the mac OS side of things. I have a windows install handy I can test on/tweak if it's broken.

[sphaerophoria]

Working around the osx installer issue by just installing our configuration at runtime. That's kind of gross but I think is passable for now. Next up is Windows testing/tweaking.

[a17r]

Any progress here?

Qt5DBus in KF5Notifcations is actually ifdef'd behind if (NOT ANDROID AND NOT WIN32)
https://invent.kde.org/frameworks/knotifications/-/blob/master/CMakeLists.txt#L59