Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linux): Emit D-Bus signal directly for dock notification badges #686

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Covkie
Copy link
Collaborator

@Covkie Covkie commented Jun 21, 2024

Closes #298, #424, #189

remove (#) title prefix when Notification Badge option is toggled

explode scuffed optional libunity dependency (kabloomed)

Closes Vencord#298, Vencord#424, Vencord#189

remove (#) title prefix when `Notification Badge` option is toggled

explode scuffed option libunity dependency (kabloomed)
src/main/appBadge.ts Outdated Show resolved Hide resolved
@Covkie
Copy link
Collaborator Author

Covkie commented Jun 21, 2024

I need some design input regarding the unread badge option.

Do I provide an unread badge at all and if I do do i make it display a "-" or "0" (also should I strip out the like with the (#)
image
image
This is simply a limitation of the Launcher API unfortunately.

Applied reviewer suggestions.

Unread Badge displays a `0`
@ToxicMushroom
Copy link

Do I provide an unread badge at all and if I do do i make it display a "-" or "0" (also should I strip out the like with the (#)

I think it makes most sense to not display an unread pings badge if the count is 0.
So it makes it more noticeable when you do have one.

Idm either way for the •

@Covkie
Copy link
Collaborator Author

Covkie commented Jun 23, 2024

@ToxicMushroom The unread count isnt actually "0" in this case. Theres two distinct badge modes; unreads and mentions. Mentions are what have the number count and only shows up for pings and DMs, etc.

Unreads are when there is a message in a channel that you do not have muted. Users can disable the unread badge but still have the mention badge.

The reason why the unread badge shows a "0" is due to a limitation with the Launcher API it can only show 0-9 with no option for blank badge or a dot.

src/main/appBadge.ts Outdated Show resolved Hide resolved
src/main/appBadge.ts Show resolved Hide resolved
src/main/appBadge.ts Outdated Show resolved Hide resolved
src/main/mainWindow.ts Outdated Show resolved Hide resolved
src/renderer/appBadge.ts Show resolved Hide resolved
@flying-sheep
Copy link

Why gdbus? It doesn’t necessarily exist on platforms supporting this DBus API (e.g. KDE Plasma)

Can you switch to a DBus node.js library?

@Covkie
Copy link
Collaborator Author

Covkie commented Aug 13, 2024

I think we went with cli to not bring in another dep. If the issue is gdbus not existing everywhere we could switch to dbus-send

@flying-sheep
Copy link

flying-sheep commented Aug 13, 2024

dbus-send is probably the better call: I checked Arch Linux, Ubuntu, Debian, and Fedora where it’s provided by the dbus package, which is installed on every system with a DE, and depended on by many many things.

There are two DBus implementations: dbus-daemon (included with dbus) and dbus-broker. There are a few systems that only use dbus-broker, in which case dbus-send is not necessarily present. That currently includes only immutable distros like Fedora Silverblue. Debian has a split-off dbus-bin package that contains dbus-send so maybe going forward immutable distros also come with it.

So all in all I’d say if you go with one tool, dbus-send is the safer bet, multiple tools would be better, and a node.js library would be the perfect, always-works alternative.

src/main/appBadge.ts Outdated Show resolved Hide resolved
@CarJem
Copy link

CarJem commented Oct 5, 2024

Curious where this PR stands as I'd like to have this fix already so I can ditch the Official Discord Client on GNOME Wayland.

@Covkie
Copy link
Collaborator Author

Covkie commented Oct 6, 2024

@CarJem wait for vee to get around to merging or build and run vesktop yourself locally

@RealSweetPanda
Copy link

Important info: I compiled this pr and used it for some time, this makes programs crash after some time. turns out it opens dbus interfaces like crazy. I'm almost sure it happens even if you shouldn't have a badge. (as in, you don't get any ping or message).
if it's relevant I didn't get any badge even when I should with this pr (probably more related to waybar/hyprland)

@Covkie
Copy link
Collaborator Author

Covkie commented Oct 11, 2024

if it's relevant I didn't get any badge even when I should with this pr

if the com.canonical.Unity.LauncherEntry interface doesnt exist this pr will do nothing for you. (and I guess is causing the odd behaviour with the crashing.)

@Covkie
Copy link
Collaborator Author

Covkie commented Oct 11, 2024

@RealSweetPanda I cannot replicate the dbus spam or crashing.

opens dbus interfaces like crazy

com.canonical.Unity.LauncherEntry can't even have connections left open. it just fires into the void. Hyprland/waybar issue I assume.

@prplz
Copy link

prplz commented Dec 5, 2024

Hi, thanks for working on this.
To get it working with Dash To Panel I had to change the type signature of count from i (int32) to x (int64).
See: https://wiki.ubuntu.com/Unity/LauncherAPI#Low_level_DBus_API:_com.canonical.Unity.LauncherEntry

Edit: After the vesktop update that came out about an hour ago, unread badge count is now working for me without this PR

@Covkie
Copy link
Collaborator Author

Covkie commented Dec 6, 2024

Edit: After the vesktop update that came out about an hour ago, unread badge count is now working for me without this PR

Yeah electron fixed (removed) their desktop env checking for the libunity dep. I still think this PR is relevant tho as libunity is old and many distros don't package it anymore. It should be dropped in favour of dbus communication.

There's also some things that this PR does like clean the title of the web specific prefixes (#) & and include message requests in the count.
This PR also used 0 as an indicator for unreads when there was no mention count. Electron's setBadgeCount's API doesn't allow this and treats 0 as an indicator to hide the badge.

@Michael-B8
Copy link

Hey, I've also built with this since my notification badge doesn't work without this PR. I've also ran into @RealSweetPanda's problem (and I'm on Plasma), in which one of my dbus-broker processes reaches its max file descriptor limit (1024) then it crashes and so does everything on my pc. It seems every single call to dbus.sessionBus() opens a connection that's never closed.

I think I managed to fix it on my machine (didn't test for too long, but seems to work and not leave too many connections hanging) by making sessionBus a singleton. I don't know much about js though so I don't know if that's a good solution lol.

apparently causes crashes but i cant repro :p

Vencord#517 should have this same issue so will need this same fix, which is why its in util :3
@Covkie
Copy link
Collaborator Author

Covkie commented Dec 25, 2024

@Michael-B8 can you lmk if that fixed it.

@Michael-B8
Copy link

@Michael-B8 can you lmk if that fixed it.

Yep, doesn't seem to create any extra connections anymore. Thanks :)

If you're curious how I went about checking, I got the pid of the main vesktop process pgrep vestkop -a (or electron), then checked the pid of every dbus-broker pgrep dbus-broker for vesktop opened files sudo lsof -p <pid_dbus> | grep <pid_vesktop>. And when I found the one that has opened files I message my discord account to trigger a notification badge update and check if the number of vesktop opened files stayed the same. Before, it added a connection for every badge update and eventually it reached the limit and crashed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unread Notification Badge not working on KDE
9 participants