Native notifications for Windows, Linux and OSX #3389

pr8x · 2019-12-23T16:59:08Z

What does the pull request do?

Implementing native notification support for Windows (10), Linux (DBUS/FreeDesktop) and OSX.

Discussion

In Windows it is required to register an application shortcut in the StartMenu in order to send toast notifications (See https://docs.microsoft.com/en-us/windows/win32/shell/enable-desktop-toast-with-appusermodelid). I am not exactlly sure how we want to handle this. I could add a RegisterApplication function in the Win32Platform to do this registration and also create a unique AppUserModelID based on the Application.Name. It's probably a good idea to uninstall the shortcut once the application exits.
This PR provides a basic implementation that works across the three major desktop platforms. Advanced features (such as replacing notifications, custom buttons, images, etc) can be rolled in future updates.

What is the current behavior?

We only have managed notifications hosted inside avalonia windows.

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Windows 10: Using the UWP ToastNotification API
Linux: Using the FreeDesktop DBUS org.freedesktop.Notifications service interface. (Thanks to [WIP] Native notifications #2884)
OSX: Creating a COM interface for interacting with the native Cocoa API (NSUserNotificationCenter)

Fixed issues

#2155

kekekeks · 2019-12-23T20:39:16Z

src/Windows/Avalonia.Win32/Avalonia.Win32.csproj

@@ -4,6 +4,9 @@
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <PackageId>Avalonia.Win32</PackageId>
  </PropertyGroup>
+  <ItemGroup>
+    <PackageReference Include="Microsoft.Windows.SDK.Contracts" Version="10.0.18362.2005" />


Does that work with Win 8.1 or is it Win10+ only?

This is Win10+ only from what I can tell.

The Windows 10 WinRT API Pack enables you to add the latest Windows Runtime APIs support to your .NET Framework 4.5+ and .NET Core 3.0+ libraries and apps.

https://www.nuget.org/packages/Microsoft.Windows.SDK.Contracts/10.0.17763.1000-preview

But AFAIK the toast notifications only work in Windows 10 anyway.

Windows 8.1 also has them

Yeah Windows 8 and 8.1 both have support for the ToastNotification API. Not sure if we really care about Windows 8 though 😄 .

We probably want a managed fallback that displays notifications in separate windows manually. Like IM clients of the past did. That's not in the scope of this PR though.

Well, it's more of "do people who use Avalonia for their software care about Windows 8 and 8.1", not "if we care about it".

!T FIxed warning

!F Close and action callbacks !F Handling notification center events in seperate delegate !T Renaming some files to be more consistent

pr8x · 2020-01-04T11:46:38Z

@danwalmsley So how do we solve the problem with registering the application shortcut in the startmenu on Windows? Should I just register a temporary and uninstall it when the application terminates?

MonkAlex · 2020-01-04T11:53:54Z

its not required, but will be used, if registered (not check, only google)
social.msdn.microsoft.com

pr8x · 2020-01-04T13:36:23Z

In my experience deleting the shortcut (even when the AppUserModelID has been assigned to the current process via SetCurrentProcessExplicitAppUserModelID) will cause the toast nofications to be ignored by Windows. Most official Microsoft sources confirm this.

A shortcut to your app, with a System.AppUserModel.ID, must be installed to the Start screen. Note, however, that it does not need to be pinned to the Start screen

https://docs.microsoft.com/en-us/previous-versions/windows/desktop/legacy/hh802768(v=vs.85)?redirectedfrom=MSDN

Also it seems like once the shortcut was installed they use the name and the icon of it to display the application in the taskbar. Without a shortcut the taskbar icon will default to the main window icon AFAICT. So it might actually be a good idea to have some sort of RegisterApplication function on the Win32Platform that takes care of registering your AppUserModelID and assigning name and icon of your application via the shortcut.

# Conflicts: # src/Avalonia.Native/AvaloniaNativePlatform.cs

grokys · 2020-01-21T13:31:19Z

@danwalmsley this is ready for review now, when you have a moment.

samples/ControlCatalog/MainWindow.xaml

samples/ControlCatalog/Pages/NotificationsPage.xaml

…om/pr8x/Avalonia into native_notifications_win_linux_osx

src/Avalonia.FreeDesktop/Notifications/FreeDesktopNotificationManager.cs

kekekeks

In addition to the comments: please, avoid doing formatting changes for the sake of doing formatted changes. It confuses git blame and makes it way harder to track the origin of a particular change. Same goes for renaming files. You'll probably have to squash your changes now.

src/Avalonia.FreeDesktop/Avalonia.FreeDesktop.csproj

kekekeks · 2020-02-01T15:19:36Z

src/Avalonia.FreeDesktop/Notifications/FreeDesktopNotificationManager.cs

+
+        private async ValueTask Connect()
+        {
+            _isConnected = await


Note that the app might be started without a session bus or without org.freedesktop.Notifications service on the session bus. That's the case for VNC-powered sessions, WSL, embedded devices and other scenarios without a "full" desktop environment. We should gracefully fall back to some managed implementation in that case.

Use DBusHelper.Connection which will be just null if no session DBus is available like we are doing in our dbusmenu implementation. Then try to create a proxy and see if that throws. That check should be done before registering FreeDesktopNotificationManager in the service locator, so the rest of the code base won't be confused by dysfunctional notification manager. It would also be a good place to call GetCapabilitiesAsync.

It would also be a good place to call GetCapabilitiesAsync.

Which capabilties would you check for? At first glance the features we rely on should be working in all server implementations.

We should gracefully fall back to some managed implementation in that case.

You mean the WindowNotificationManager? Which Window should it be installed on though?

Currently we're only invoking DBusHelper.TryInitialize() when UseDBusMenu is enabled. We should probably be doing this unconditionally now.

Which Window should it be installed on though

The initial idea was that user code is supposed to get the notification manager from the window itself rather than using service locator. A single app might be using toplevels represented by different windowing systems. I. e. it's planned to have a "remote" mode for devtools when they are using our previewer remote widget infrastructure to render themselves to a remote machine (i. e. you will be able to connect to devtools of an application running on your android phone). So the general policy is to remove global variables when possible.

So there shouldn't be a globally accessible instance of the notification manager, only one accessed via ITopLevelImpl in some manner.

So there shouldn't be a globally accessible instance of the notification manager, only one accessed via ITopLevelImpl in some manner.

Allright, but that seems vastly out of scope for this PR. For now the service will just return null when it fails to initialize. I guess the users can account for that in their code and fire up the WindowNotificationManager instead.

Wait... So instead of registering the notification manager (on all platforms) through AvaloniaLocator I should just make it a property on the TopLevel?

Yes, probably an attached one. The general idea is that services should be resolved via toplevels (somewhat like in Android where every damn thing needs a Context)

Also, note how it works with global menu - toplevel impl implements an extra interface where it announces that it could potentially provide some service.

toplevel impl implements an extra interface

I can't seem to find the interface you're referring to. What is it called?

Avalonia/src/Avalonia.Controls/Platform/ITopLevelNativeMenuExporter.cs

Line 14 in e80ddf5

public interface ITopLevelImplWithNativeMenuExporter : ITopLevelImpl

kekekeks · 2020-02-01T15:33:18Z

src/Avalonia.Native/NotificationManager.cs

+            _notifications = new Dictionary<int, INotification>();
+            _notificationCloseCallback = NotificationOnClose;
+            _notificationActionCallback = NotificationOnAction;
+            _native.CloseCallback = Marshal.GetFunctionPointerForDelegate(_notificationCloseCallback);


Don't use function pointers with Avalonia.Native. Create a COM interface and implement it on the C# side instead. We are automatically generating native-to-managed call proxies for interfaces with Events suffix.

The preferred way would be to not use an int identifier field in AvnNotification but introduce

AVNCOM(IAvnNotificationEvents, 21) : IUnknown { virtual void OnAction() = 0; virtual void OnClose() = 0; };

and create a new instance of a class implementing that interface for each ShowNotification call and pass it along with other notification info, then keep it in ComPtr<IAvnNotificationEvents> on the native side. That way the callback life time would be managed automatically and we won't need to keep the GC-rooted dictionary with all of the notifications.

The preferred way would be to not use an int identifier field in AvnNotification

I guess once we implement replacing of notifications it still makes sense to have some id field to identify the OSX notification we want to replace.

then keep it in ComPtr on the native side

That means I have to store a vector of all the ComPtr<IAvnNotificationEvents> and release them when the notification closes? AFAIU you want the interface of IAvnNotificationManager to be ShowNotification(AvnNotification* notification, IAvnNotificationCallbacks* callbacks), correct?

Why do you need a vector?

The user can show multiple notifications. I need to store their corresponding callbacks somewhere in order to call them from the global NotificationCenterDelegate. I could plug them into the NSUserNotification.userInfo somehow, I suppose. userInfo expects a map of id, though (which is essentially a pointer to objc_object). So I guess I need to find some ugly workaround of putting a ComPtr in there...

userInfo expects a map of id

it seems that OSX expects the contents of userInfo to be serializable. I'm afraid that might need to change the API design to accomodate OSX notifications properly.

What API design are you thinking of? For now, we could also keep it with the Dictionary on C# and the function pointers. And revisit it in the future once we figured out a good/better way of designing this API.

For now, we could also keep it with the Dictionary on C# and the function pointers.

That can and will lead to memory leaks.

src/Windows/Avalonia.Win32/Win32Platform.cs

kekekeks · 2020-02-01T15:44:08Z

native/Avalonia.Native/src/OSX/app.mm

+- (NSString *)__bundleIdentifier;
+{
+    if (self == [NSBundle mainBundle]) {
+        return @"com.avalonia.native.osx";


We should add an option to configure that with AvaloniaNativeMacOptions and fall back to "com.avaloniaui.apps."+Assembly.GetEntryAssembly().Name

This probably should also somehow interact with bundle identifier from Info.plist if the app is running from an actual app bundle.

I guess we should check the [[NSBundle mainBundle] bundleIdentifier] and only install the fake bundle identifier when there is no mainBundle identifier.

kekekeks · 2020-02-01T15:59:55Z

@pr8x it seems that OSX notifications kinda "live forever" and one is expected to track them using a string identifier and up to 1KB of data in userInfo. It seems that the current API is simply not suitable for dealing with OSX notifications and might need to be redesigned completely. Not sure if that's in the scope of this PR though.

With the current API we might want to disallow using arbitrary huge expiration times, so we could assume that notification will always expire in a reasonable time.

pr8x · 2020-02-01T17:25:54Z

@kekekeks What do you mean by "live forever"? They just stay in the notification center until the user manually removes them? I guess we can default to some reasonable expiration time when the user doens't specify it. But what is the problem with them just staying in the notfication center?

…ialize

kekekeks · 2020-02-01T18:33:15Z

Do they stay in the notification center after application is closed?

pr8x · 2020-02-16T18:39:38Z

@kekekeks Yes, the notifications indeed stay when the application exits. I should probably listen to some application shutdown event and call removeAllDeliveredNotifications()

kekekeks · 2020-02-16T19:09:49Z

We should also call removeAllDeliveredNotifications on startup, I think.

kekekeks · 2020-02-16T19:11:32Z

BTW, same happens on KDE.

danwalmsley · 2020-07-05T21:23:01Z

closing due to inactivity... @pr8x if you would like to resolve the conflicts we can resurrect this PR.

pr8x added 5 commits December 22, 2019 23:15

!F Porting FreeDesktop NotificationManager from PR

22d8d98

!T Cleaning up the notification page

4e58a51

!B Fixing About and Exit commands

39ea180

!F Adding support for Win10NotificationManager

964d28e

!F OSX Notification COM implementation

fee7617

kekekeks reviewed Dec 23, 2019

View reviewed changes

pr8x added 3 commits December 24, 2019 14:43

!F Adding CreateNotificationManager hook to IAvaloniaNativeFactory

ba3f4b6

!T FIxed warning

!F Implementing NativeNotificationManager

9efd362

!F Close and action callbacks !F Handling notification center events in seperate delegate !T Renaming some files to be more consistent

!T Reverting to .NET Core 2

51b53a2

Merge branch 'master' into native_notifications_win_linux_osx

291a274

# Conflicts: # src/Avalonia.Native/AvaloniaNativePlatform.cs

pr8x marked this pull request as ready for review January 13, 2020 13:16

pr8x changed the title ~~[WIP] Native notifications for Windows, Linux and OSX~~ Native notifications for Windows, Linux and OSX Jan 21, 2020

kekekeks assigned kekekeks and danwalmsley Jan 22, 2020

Merge branch 'master' into native_notifications_win_linux_osx

d7463f1

danwalmsley requested changes Jan 22, 2020

View reviewed changes

samples/ControlCatalog/MainWindow.xaml Outdated Show resolved Hide resolved

samples/ControlCatalog/Pages/NotificationsPage.xaml Outdated Show resolved Hide resolved

pr8x added 4 commits January 25, 2020 12:28

!T Reverting to ReactiveCommand

da4422c

Merge branch 'master' into native_notifications_win_linux_osx

a4e3230

!R Installing shortcut and appUserModlId in AfterSetup.

05ea283

Merge branch 'native_notifications_win_linux_osx' of https://github.c…

4c4e4ab

…om/pr8x/Avalonia into native_notifications_win_linux_osx

pr8x commented Jan 25, 2020

View reviewed changes

src/Avalonia.FreeDesktop/Notifications/FreeDesktopNotificationManager.cs Outdated Show resolved Hide resolved

pr8x added 3 commits February 1, 2020 13:24

!R Making INotificationManager.Show async

70978d7

Merge branch 'master' into native_notifications_win_linux_osx

e171730

!R Using ValueTask

8db0b4d

pr8x requested a review from danwalmsley February 1, 2020 13:19

!T Don't create notifier every time notification is requested

a937b2c

kekekeks requested changes Feb 1, 2020

View reviewed changes

kekekeks reviewed Feb 1, 2020

View reviewed changes

!T Removing duplicate package

7c76b26

!R Fallback to null when FreeDesktopNotificationManager fails to init…

c9913df

…ialize

!R Moving AfterSetup() into Win32Platform

625faf9

ajmcateer mentioned this pull request Feb 12, 2020

Alpha Desktop Client Available (unofficial) gotify/server#271

Closed

!T Removing service check

7323816

pr8x force-pushed the native_notifications_win_linux_osx branch from ad46e01 to 7323816 Compare February 16, 2020 13:12

Merge branch 'master' into native_notifications_win_linux_osx

62d3953

danwalmsley closed this Jul 5, 2020

pr8x mentioned this pull request Oct 4, 2020

Native notifications #4803

Closed

pr8x mentioned this pull request Feb 7, 2021

System notifications for Windows, MacOS and Linux #2155

Open

Native notifications for Windows, Linux and OSX #3389

Native notifications for Windows, Linux and OSX #3389

Conversation

pr8x commented Dec 23, 2019 • edited Loading

What does the pull request do?

Discussion

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Fixed issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pr8x commented Jan 4, 2020

MonkAlex commented Jan 4, 2020

pr8x commented Jan 4, 2020

grokys commented Jan 21, 2020

kekekeks left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pr8x Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

pr8x Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pr8x Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks Feb 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pr8x Feb 1, 2020 • edited Loading

Choose a reason for hiding this comment

kekekeks commented Feb 1, 2020

pr8x commented Feb 1, 2020 • edited Loading

kekekeks commented Feb 1, 2020

pr8x commented Feb 16, 2020

kekekeks commented Feb 16, 2020

kekekeks commented Feb 16, 2020

danwalmsley commented Jul 5, 2020

pr8x commented Dec 23, 2019 •

edited

Loading

kekekeks left a comment •

edited

Loading

pr8x Feb 1, 2020 •

edited

Loading

pr8x Feb 1, 2020 •

edited

Loading

pr8x Feb 1, 2020 •

edited

Loading

kekekeks Feb 1, 2020 •

edited

Loading

kekekeks Feb 16, 2020 •

edited

Loading

kekekeks Feb 1, 2020 •

edited

Loading

pr8x Feb 1, 2020 •

edited

Loading

pr8x commented Feb 1, 2020 •

edited

Loading