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

fix: Load systray items that are registered without a path #1230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kage-Yami
Copy link

Description

In some environments (e.g. Budgie Desktop on Ultramarine Linux 40, or when using home-manager's status-notifier-watcher on Nix), StatusNotifierItems don't have their path as part of their registration, resulting in these items always failing to load in the systray widget.

This fixes those "misbehaving" registrations, via DBus introspection; the node tree from the top is crawled until any node implementing org.kde.StatusNotifierItem is found, and the path to that node is used as the resulting systray item path.

It also removes string slicing for the same code path that assumed :-prefixed addresses without paths must always be a total of 6 characters, when they can be shorter. (This unrelated issue was "hiding" the real issue in some circumstances.)

Additional Notes

Because DBus introspection (including via zbus) spits out XML, quick-xml was added as a dependency (and serde promoted from a transient dependency to an explicit dependency) to notifier_host.

The "crawl" mentioned earlier is implemented by way of a recursive function; given my (albeit naive) understanding of DBus, the trees will never be very deep, so I don't foresee this recursion being problematic.

Checklist

Please make sure you can check all the boxes that apply to this PR.

  • All widgets I've added are correctly documented.
  • I added my changes to CHANGELOG.md, if appropriate.
  • The documentation in the docs/content/main directory has been adjusted to reflect my changes.
    • (Errm, the docs/content directory doesn't seem to exist?)
  • I used cargo fmt to automatically format all code before committing

@Kage-Yami Kage-Yami mentioned this pull request Nov 18, 2024
3 tasks
@Kage-Yami
Copy link
Author

Kage-Yami commented Nov 24, 2024

Seems this needs a little more work (or at least, may need a "phase 2" to fix more edge-cases); just installed Discord (from the Terra repository), and its icon is not showing in the systray when using this branch, with the following in the logs:

WARN notifier_host::host > Could not create StatusNotifierItem from address ":1.990": Failure("no StatusNotifierItem found for :1.990")

I'll investigate this one further...

@Kage-Yami Kage-Yami marked this pull request as draft November 24, 2024 05:05
@Kage-Yami Kage-Yami force-pushed the fix/pathless-systray-items-2 branch 3 times, most recently from fdfc6cc to 5712a45 Compare November 24, 2024 06:05
@Kage-Yami Kage-Yami force-pushed the fix/pathless-systray-items-2 branch from 5712a45 to aa768ce Compare November 24, 2024 06:16
@Kage-Yami
Copy link
Author

Ok, what I've got now seems to do the trick. Turns out, for some reason, DBus didn't think any node for Discord implemented the interface (i.e. introspection was empty for every single node, except for listing children).

Interestingly, the previous fallback (that I had removed in favour of the introspection method) of just assuming that it was at /StatusNotifierItem would've worked. Here though, I've tweaked that assumption slightly such that if any node in the tree is named exactly StatusNotifierItem, then that's probably the one we're after (instead of assuming that it be directly under the root).

@Kage-Yami Kage-Yami marked this pull request as ready for review November 24, 2024 06:25
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.

1 participant