mikaelbr / node-notifier

A Node.js module for sending notifications on native Mac, Windows and Linux (or Growl as fallback)
MIT License
5.73k stars 322 forks source link

Bump snoretoast to fix activation response with appID #375

Open DuBistKomisch opened 3 years ago

DuBistKomisch commented 3 years ago

I've rebuilt the snoretoast binaries to include this change I've submitted upstream: https://github.com/KDE/snoretoast/pull/15

This fixes https://github.com/mikaelbr/node-notifier/issues/332 https://github.com/mikaelbr/node-notifier/issues/326 and possibly others.

TL;DR: The pipe never got written to when the notification is activated (i.e. clicked the body or clicked an action button), so a blank response and metadata would be received in the notifier callback. This only happens when passing a custom appID, because the default snoretoast behaviour is to install a shortcut to itself and register a background callback in the Windows registry, which uses a different mechanism which did actually write to the pipe.

Action buttons on Windows are really the only advantage this library has over the built-in Electron notifications, so this is a pretty important fix for us Electron users.

Araxeus commented 3 years ago

This is a really great bugfix 🎉 We have tested this over at youtube-music and can confirm it fixes all bugs with the appID + action buttons interaction

Please merge and publish as soon as possible ❤️

tomboolean commented 3 years ago

Thank you so much @DuBistKomisch!

mikaelbr commented 3 years ago

Thanks for this! To just be on the safe side, I think I'll wait for KDE/snoretoast#15 and use binaries from that/build myself.

mikaelbr commented 2 years ago

Just an update, still waiting for https://github.com/KDE/snoretoast/pull/15. There's an ongoing discussion there.

Araxeus commented 2 years ago

Starting to lose hope that this will ever get merged... 😢

Araxeus commented 1 year ago

@mikaelbr changes were made to snoretoast which might fix the issues, see:

https://github.com/KDE/snoretoast/commit/8502ef06167b3741c072cf65ba74d4f501600408 https://invent.kde.org/libraries/snoretoast/-/commit/8502ef06167b3741c072cf65ba74d4f501600408

̶(̶t̶h̶i̶s̶ ̶i̶s̶ ̶p̶r̶e̶t̶t̶y̶ ̶m̶u̶c̶h̶ ̶w̶h̶a̶t̶ ̶t̶h̶e̶ ̶p̶a̶t̶c̶h̶ ̶b̶y̶ ̶@DuBistKomisch ̶d̶o̶e̶s̶)̶

https://binary-factory.kde.org/job/SnoreToast_Release_win64/ https://binary-factory.kde.org/job/SnoreToast_Nightly_win64/

̶c̶o̶u̶l̶d̶ ̶y̶o̶u̶ ̶m̶a̶y̶b̶e̶ ̶j̶u̶s̶t̶ ̶u̶p̶d̶a̶t̶e̶ ̶t̶h̶e̶ ̶s̶n̶o̶r̶e̶t̶o̶a̶s̶t̶ ̶b̶i̶n̶a̶r̶i̶e̶s̶?̶ ̶ ̶(̶I̶ ̶h̶a̶v̶e̶ ̶t̶o̶ ̶n̶o̶t̶e̶ ̶t̶h̶a̶t̶ ̶I̶ ̶h̶a̶v̶e̶n̶'̶t̶ ̶p̶e̶r̶s̶o̶n̶a̶l̶l̶y̶ ̶t̶e̶s̶t̶e̶d̶ ̶t̶h̶a̶t̶ ̶i̶t̶ ̶d̶o̶e̶s̶ ̶f̶i̶x̶ ̶t̶h̶e̶ ̶i̶s̶s̶u̶e̶,̶ ̶t̶h̶o̶ ̶j̶u̶s̶t̶ ̶b̶y̶ ̶l̶o̶o̶k̶i̶n̶g̶ ̶a̶t̶ ̶t̶h̶e̶ ̶c̶o̶d̶e̶ ̶i̶t̶ ̶s̶h̶o̶u̶l̶d̶)̶

Araxeus commented 1 year ago

1.5 Years later I realized that there's actually a pretty easy way to fix this, we just didn't implement SnoreToast well enough:

Here's a guide: https://github.com/mikaelbr/node-notifier/issues/424

this PR can be closed (as well as https://github.com/KDE/snoretoast/pull/15 @DuBistKomisch)