feat: Full StatusNotifierItem (SNI) tray implementation by miramir · Pull Request #970 · MalpenZibo/ashell

miramir · 2026-09-17T16:32:32Z

What

Full SNI properties: Category, Status, Id/ItemId, Title, AttentionIcon*, OverlayIcon*
Item status handling: Passive items are now hidden (per spec); NeedsAttention items display attention/overlay icons
Context menu, secondary activate, scroll: wired D-Bus methods + ItemsRemoved signal handling for dbusmenu
Late-start self-healing: discovery poll reduced from 30s → 3s; watcher re-reads RegisteredItems on each tick to catch clients that registered before the watcher existed
Better icon selection: best_icon_pixmap() picks the largest pixmap; current_icon_from_proxy() falls back through pixmap → icon name → XDG theme
Tests: unit tests for ItemStatus, best_icon_pixmap, pixmap_to_icon, split_service_name, menu helpers (is_separator, is_visible, renderable_children), is_blocklisted, XDG icon normalization
Docs: website tray page updated with SNI protocol details; D-Bus interface reference lists com.canonical.dbusmenu
.gitignore: added .vscode/

Breaking change

Passive SNI items are now hidden from the tray (previously shown alongside Active). Matches the SNI spec

MalpenZibo

Thanks for this!! I left some points to be addressed.

MalpenZibo · 2026-10-05T12:04:08Z

                            let _ = output.send(ServiceEvent::Update(event)).await;
-
-                            if reload_events {
-                                break;


This removes the break after TrayEvent::Registered, which is what made new items work

MalpenZibo · 2026-10-05T12:05:03Z


 #[cfg(test)]
 mod tests {
-    use super::{get_icon_from_name, is_safe_icon_name};


This replaces the existing test module instead of extending it: the is_safe_icon_name tests (accepts_ordinary_icon_names, rejects_path_traversal_components, …) from 32ee24b "prevent path traversal via icon names" are gone. The function is still there and still used, but these tests guard a security fix, so please keep them and add the new ones next to them

MalpenZibo · 2026-10-05T12:09:14Z

+            // Continuous drain loop: probe well-known names every 3s (not 30s).
+            // 30s is too slow for late-start — a client started after ashell
+            // would take up to 30s to appear.
+            let mut interval = tokio::time::interval(Duration::from_secs(3));


Together with is_status_notifier_item, every tick now probes every name on the bus, well-known and unique: a proxy plus a Get IconName on /StatusNotifierItem for each one.

On my session that's 140 names, so around 45 D-Bus calls per second, forever, mostly to processes that answer with an error. Before, it was every 30s and only for names containing StatusNotifierItem.

For late-starting clients, it's enough to probe a name once, when it appears: NameOwnerChanged is already handled in this loop, and a single full scan at startup covers names that existed before. Then the periodic tick can stay at 30s (or go away).

MalpenZibo · 2026-10-05T12:09:41Z

+        let status_str = item_proxy
+            .status()
+            .await
+            .unwrap_or_else(|_| "Passive".to_owned());


If reading Status fails, the item becomes Passive, and is_visible now hides Passive items

fixed

Fix tray items disappearing when the Status property can't be read

Previously, StatusNotifierItem::new() fell back to "Passive" when reading the Status property failed:

copy
let status_str = item_proxy
.status()
.await
.unwrap_or_else(|_| "Passive".to_owned());
Combined with the new is_visible() filter in the tray module (which hides Passive items per the SNI spec), this meant any client that doesn't implement the Status property — or whose read fails transiently — was silently hidden from the tray forever. A failing read is not the same as an explicit Passive reply: clients that skip Status clearly intend to be shown (that's why they register in the first place).

The fix falls back to "Active" instead, so only an explicit Passive response hides an item:

copy
let status_str = item_proxy
.status()
.await
.unwrap_or_else(|_| "Active".to_owned());
Semantics after the change:

Passive received explicitly (the client replied so) → item is hidden, per the SNI spec — unchanged.
Read fails (client doesn't implement Status) → item stays visible, no longer silently dropped from the tray.
The NewStatus signal handler already ignores read failures (it emits nothing rather than resetting the status), so no further changes were needed there.Fix tray items disappearing when the Status property can't be read

Previously, StatusNotifierItem::new() fell back to "Passive" when reading the Status property failed:

copy
let status_str = item_proxy
.status()
.await
.unwrap_or_else(|_| "Passive".to_owned());
Combined with the new is_visible() filter in the tray module (which hides Passive items per the SNI spec), this meant any client that doesn't implement the Status property — or whose read fails transiently — was silently hidden from the tray forever. A failing read is not the same as an explicit Passive reply: clients that skip Status clearly intend to be shown (that's why they register in the first place).

The fix falls back to "Active" instead, so only an explicit Passive response hides an item:

copy
let status_str = item_proxy
.status()
.await
.unwrap_or_else(|_| "Active".to_owned());
Semantics after the change:

Passive received explicitly (the client replied so) → item is hidden, per the SNI spec — unchanged.
Read fails (client doesn't implement Status) → item stays visible, no longer silently dropped from the tray.
The NewStatus signal handler already ignores read failures (it emits nothing rather than resetting the status), so no further changes were needed there.

MalpenZibo · 2026-10-05T12:11:57Z

+
+/// SNI tooltip (`org.kde.StatusNotifierItem` `ToolTip`).
+/// Protocol surface; not yet surfaced in the UI.
+#[allow(dead_code)]


Let's remove the dead_code if it's not used

MalpenZibo · 2026-10-05T12:13:00Z

+                // Dedup by stable SNI `Id` first (some clients register under
+                // multiple names — e.g. unique-name and well-known-name — but
+                // share the same `Id`), then fall back to `name`-based lookup.
+                let existing_idx = self.data.0.iter().position(|item| {


Dedup by Id relies on item_id, which is one of the dead fields above. If two names really register the same item, it would be better to understand why and avoid the double registration in the watcher, rather than overwrite one item with the other here.

MalpenZibo · 2026-10-05T12:13:35Z

    ("en-US", include_str!("../i18n/en-US/ashell.ftl")),
    ("fr-FR", include_str!("../i18n/fr-FR/ashell.ftl")),
    ("de-DE", include_str!("../i18n/de-DE/ashell.ftl")),
+    ("ru-RU", include_str!("../i18n/ru-RU/ashell.ftl")),


Thanks for the translation but could you open a dedicated PR for this?

miramir · 2026-10-09T18:50:29Z

fix all issues, and rebase

MalpenZibo

Thanks for addressing the previous points; most of them look good now! I left a few inline comments. In general:

Please trim the comments: the PR adds ~180 comment lines, most of them restating the code. We keep comments for the non-obvious "why" only.
Please give the last commit a proper message and update the PR description: it still mentions a 3s poll and context menu / secondary activate / scroll, which aren't in the code.

MalpenZibo · 2026-10-10T14:40:08Z

-                    Ok(owner) => owner,
-                    Err(_) => continue,
-                };
+        // Probe every name (well-known AND unique) concurrently so a slow


This still probes every name on the bus (unique ones included) on each 30s tick, forever. A single full scan at startup plus the NameOwnerChanged probe should be enough, so the periodic full scan can go.

MalpenZibo · 2026-10-10T14:40:08Z

+    /// a bare path (for well-known-name registrations). We split on the
+    /// first `/` to recover the unique sender and dedup against the
+    /// items already tracked in `self.items`.
+    async fn sync_registered_items(


This is a no-op: registered_status_notifier_items() is built from self.items, and then every entry whose sender is already in self.items is skipped, which is all of them. Please remove it (and the call at line 212).

MalpenZibo · 2026-10-10T14:40:08Z

-            .and_then(xdg_icons::get_icon_from_name),
+    // 1. `IconPixmap` (preferred) — pick the largest pixmap by pixel count.
+    if let Ok(icons) = item_proxy.icon_pixmap().await
+        && let Some(icon) = dbus::best_icon_pixmap(&icons)


pixmap_to_icon already picks the largest valid pixmap. best_icon_pixmap picks the largest before validation, so if that one is malformed we lose the pixmap icon entirely and fall back to the name. Please drop best_icon_pixmap and pass the full vec to pixmap_to_icon as before.

MalpenZibo · 2026-10-10T14:40:08Z

+        let status = ItemStatus::from(status_str.as_str());
+
+        // Attention/overlay icons: pixmap first (preferred), then icon name.
+        let attention_pixmap = item_proxy


Attention and overlay icons are read only once here and never refreshed: NewAttentionIcon / NewOverlayIcon aren't handled, so when an app switches to NeedsAttention later the icon can be stale or missing. Either handle the signals or leave attention/overlay out of this PR.

MalpenZibo · 2026-10-10T14:40:08Z

+                                let active_icon = if item.status == ItemStatus::NeedsAttention {
+                                    item.attention_icon
+                                        .as_ref()
+                                        .or(item.overlay_icon.as_ref())


Per the spec the overlay icon is meant to be drawn on top of the main icon, not used as a replacement for it.

MalpenZibo · 2026-10-10T14:40:08Z

@@ -222,7 +441,9 @@ impl StatusNotifierWatcher {

    #[zbus(property)]
    fn protocol_version(&self) -> i32 {


Do you have a reference for clients checking > 0? As far as I know the KDE watcher returns 0 here.

MalpenZibo · 2026-10-10T14:40:08Z


+    /// `GetGroupProperties` — batch read properties for a set of item ids.
+    /// Returns a D-Bus value (array of `{sv}` dicts), deserialized on demand.
+    fn get_group_properties(


These aren't used anywhere, please remove them.

MalpenZibo · 2026-10-10T14:40:08Z


 Clicking on an icon will open the corresponding application or menu. The module only appears when applications have tray icons.

+The tray fully implements the KDE **Status Notifier Item (SNI)** protocol


Context menu / secondary activate / scroll aren't implemented, so I'd avoid "fully implements" here.

romanstingler mentioned this pull request Oct 1, 2026

prevent path traversal via icon names #995

Merged

MalpenZibo requested changes Oct 5, 2026

View reviewed changes

miramir added 2 commits October 9, 2026 21:54

feat(tray): implement full StatusNotifierItem support

f01a551

fix(tray): silence zbus property-cache warnings when probing SNI items

1a09d2e

miramir force-pushed the full_sni_tray branch from 056f246 to 1a09d2e Compare October 9, 2026 17:01

fix

68c0c4d

MalpenZibo requested changes Oct 10, 2026

View reviewed changes

		@@ -222,7 +441,9 @@ impl StatusNotifierWatcher {

		#[zbus(property)]
		fn protocol_version(&self) -> i32 {


		Clicking on an icon will open the corresponding application or menu. The module only appears when applications have tray icons.

		The tray fully implements the KDE Status Notifier Item (SNI) protocol

Conversation

miramir commented Sep 17, 2026

What

Breaking change

Uh oh!

MalpenZibo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miramir commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MalpenZibo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

miramir commented Oct 9, 2026 •

edited

Loading