Bug 486911 - Some notifications have an extra empty button since 24.02.0
Summary: Some notifications have an extra empty button since 24.02.0
Status: RESOLVED FIXED
Alias: None
Product: kdeconnect
Classification: Applications
Component: windows-application (other bugs)
Version First Reported In: 24.02.0
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: brute4s99
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-05-12 03:54 UTC by yan12125
Modified: 2024-05-23 12:22 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
An example notification with an extra empty button (42.22 KB, image/png)
2024-05-12 03:54 UTC, yan12125
Details

Note You need to log in before you can comment on or make changes to this bug.
Description yan12125 2024-05-12 03:54:16 UTC
Created attachment 169391 [details]
An example notification with an extra empty button

SUMMARY
In KDE Connect 24.02.0, some notifications have an extra empty button (see the screenshot below). I remember notifications are normal in 23.08.1. Notifications are normal as well for 24.02.2 on Arch Linux.

STEPS TO REPRODUCE
1. Connect Android with Windows via KDE Connect and enable synchronization of notifications
2. Install an Android application that sends notifications with buttons (ex: RxDroid as in my screenshot)
3. Let the Android application send a notification

OBSERVED RESULT
Synchronized notifications on Windows have an extra empty button

EXPECTED RESULT
Synchronized notifications on Windows have only buttons specified by the Android application

SOFTWARE/OS VERSIONS
Windows: 10
KDE Plasma Version: 24.02.0
KDE Frameworks Version: 6.0.0
Qt Version: 6.6.2

ADDITIONAL INFORMATION

When I run kdeconnectd.exe with logging, I noticed it passes something strange to SnoreToast:

$env:QT_LOGGING_RULES="*.debug=true; qt.*.debug=false"
& 'C:\Program Files\KDE Connect\bin\kdeconnectd.exe'
(...some other logs...)
2024-05-12T11:31:20 kf.notifications: SnoreToast process starting: QList("-id", "2", "-t", "RxDroid", "-m", "Doses: 1 missed", "-appID", "kdeconnect.daemon", "-pid", "2328", "-pipename", "\\\\.\\pipe\\9415449995397050792", "-p", "C:/Users/user/AppData/Local/Temp/kdeconnect.daemon-WsCDWr/2", "-b", ";Take all")

The last argument ";Take all" may be why I get two buttons - one empty and another normal one.
Comment 1 yan12125 2024-05-12 04:15:14 UTC
The bug seems caused by https://invent.kde.org/frameworks/knotifications/-/commit/0b0cb05ce751ca5524f774224632668a0ea48c30#e522d5d47789104af70f49d867223dfc2d02bf0d_192_195. After that commit, actionsString always starts with a semicolon. A possible fix might be:

diff --git a/src/notifybysnore.cpp b/src/notifybysnore.cpp
index a82d0a4..3a7aa2d 100644
--- a/src/notifybysnore.cpp
+++ b/src/notifybysnore.cpp
@@ -192,7 +192,10 @@ void NotifyBySnore::notifyDeferred(KNotification *notification)
 
         for (KNotificationAction *action : actions) {
             action->setId(action->label());
-            actionsString += QLatin1Char(';') + action->label();
+            if (!actionsString.isEmpty()) {
+                actionsString += QLatin1Char(';');
+            }
+            actionsString += action->label();
         }
 
         snoretoastArgsList << QStringLiteral("-b") << actionsString;

I didn't test it, as https://community.kde.org/KDEConnect/Build_Windows suggests Visual Studio, and I had bad experience with that huge IDE.
Comment 2 Bug Janitor Service 2024-05-13 12:01:58 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/knotifications/-/merge_requests/145
Comment 3 yan12125 2024-05-23 12:22:43 UTC
Git commit ebd4452b83dff0b67313714215bce18e9c20041a by Chih-Hsuan Yen.
Committed on 23/05/2024 at 12:12.
Pushed by vonreth into branch 'master'.

notifybysnore: correct action buttons

Don't add a semicolon before the first action string to avoid an empty
button

M  +3    -3    src/notifybysnore.cpp

https://invent.kde.org/frameworks/knotifications/-/commit/ebd4452b83dff0b67313714215bce18e9c20041a