diff --git a/.gitignore b/.gitignore index db734f415..5a31b1c50 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,6 @@ docs/book/ # generated from CHANGELOG.md by website/scripts/sync-changelog.mjs website/src/pages/changelog.md + +# IDE +.vscode/ diff --git a/docs/src/reference/dbus-interfaces.md b/docs/src/reference/dbus-interfaces.md index 40a455bcf..841ad7d42 100644 --- a/docs/src/reference/dbus-interfaces.md +++ b/docs/src/reference/dbus-interfaces.md @@ -36,8 +36,9 @@ ashell connects to several D-Bus services. This reference lists all interfaces u | MPRIS | `org.mpris.MediaPlayer2.Player` | `services/mpris/dbus.rs` | Playback control | MPRIS-compatible player | | StatusNotifier | `org.kde.StatusNotifierWatcher` (served by ashell) | `services/tray/dbus.rs` | System tray icon registration | — | | StatusNotifier | `org.kde.StatusNotifierItem` | `services/tray/dbus.rs` | Individual tray icons | — | -| DBusMenu | `com.canonical.dbusmenu` | `services/tray/dbus.rs` | Tray item menus | — | +| DBusMenu | `com.canonical.dbusmenu` | `services/tray/dbus.rs` | Tray menu layout (SNI context menus) | — | | Notifications | `org.freedesktop.Notifications` (served by ashell) | `services/notifications/dbus.rs` | Notification daemon | — | +| Portal | `org.freedesktop.portal.Desktop` | `services/privacy.rs` | Privacy indicators (mic/camera) | `pipewire` | ## Checking D-Bus Availability @@ -48,7 +49,7 @@ You can verify that D-Bus services are running: busctl --system list | grep -E "bluez|NetworkManager|UPower|login1|connman" # Session bus -busctl --user list | grep -E "mpris|StatusNotifier|Notifications" +busctl --user list | grep -E "mpris|StatusNotifier|Notifications|portal" ``` If a module is not working (e.g., battery info is missing), check that the corresponding service is active: diff --git a/src/modules/tray.rs b/src/modules/tray.rs index f6ef61a40..e4e5efe73 100644 --- a/src/modules/tray.rs +++ b/src/modules/tray.rs @@ -10,7 +10,7 @@ use crate::{ services::{ ReadOnlyService, Service, ServiceEvent, tray::{ - TrayCommand, TrayEvent, TrayIcon, TrayService, + ItemStatus, StatusNotifierItem, TrayCommand, TrayEvent, TrayIcon, TrayService, dbus::{Layout, LayoutProps}, }, }, @@ -92,6 +92,16 @@ impl TrayModule { self.blocklist.iter().any(|pattern| pattern.is_match(name)) } + /// Whether a tray item should be visible. Passive SNI items are hidden by + /// the tray (per the SNI spec: `Passive` items are not shown until they + /// become `Active`); the blocklist still takes precedence. + fn is_visible(&self, item: &StatusNotifierItem) -> bool { + if self.is_blocklisted(&item.name) { + return false; + } + item.status != ItemStatus::Passive + } + pub fn update(&mut self, message: Message) -> Action { match message { Message::Event(event) => match *event { @@ -276,18 +286,28 @@ impl TrayModule { self.service .as_ref() - .filter(|s| s.data.iter().any(|item| !self.is_blocklisted(&item.name))) + .filter(|s| s.data.iter().any(|item| self.is_visible(item))) .map(|service| { Into::>::into( Row::with_children( service .data .iter() - .filter(|item| !self.is_blocklisted(&item.name)) + .filter(|item| self.is_visible(item)) .map(|item| { let name = item.name.to_owned(); let button_style = button_style.clone(); - let icon_content: Element<'_, Message> = match &item.icon { + // When the item is in `NeedsAttention` state, prefer + // the attention icon (and overlay if present). + let active_icon = if item.status == ItemStatus::NeedsAttention { + item.attention_icon + .as_ref() + .or(item.overlay_icon.as_ref()) + .or(item.icon.as_ref()) + } else { + item.icon.as_ref() + }; + let icon_content: Element<'_, Message> = match active_icon { Some(TrayIcon::Image(handle)) => Image::new(handle.clone()) .height(Length::Fixed(font_size.md - 2.0)) .into(), @@ -347,3 +367,154 @@ impl TrayModule { TrayService::subscribe().map(|e| Message::Event(Box::new(e))) } } + +#[cfg(test)] +mod tests { + use super::{TrayModule, is_separator, is_visible, renderable_children}; + use crate::config::{RegexCfg, TrayModuleConfig}; + use crate::services::tray::dbus::{Layout, LayoutProps}; + + fn make_layout(id: i32, type_: Option<&str>, visible: Option) -> Layout { + Layout( + id, + LayoutProps { + children_display: None, + label: None, + type_: type_.map(|t| t.to_string()), + toggle_type: None, + toggle_state: None, + visible, + }, + Vec::new(), + ) + } + + // --- is_separator --- + + #[test] + fn is_separator_detects_separator_type() { + let layout = make_layout(1, Some("separator"), Some(true)); + assert!(is_separator(&layout)); + } + + #[test] + fn is_separator_rejects_normal_item() { + let layout = make_layout(1, None, Some(true)); + assert!(!is_separator(&layout)); + } + + // --- is_visible (menu) --- + + #[test] + fn is_visible_treats_none_as_visible() { + let layout = make_layout(1, None, None); + assert!(is_visible(&layout)); + } + + #[test] + fn is_visible_respects_explicit_false() { + let layout = make_layout(1, None, Some(false)); + assert!(!is_visible(&layout)); + } + + #[test] + fn is_visible_respects_explicit_true() { + let layout = make_layout(1, None, Some(true)); + assert!(is_visible(&layout)); + } + + // --- renderable_children --- + + #[test] + fn renderable_children_drops_trailing_separator_and_invisible() { + // A trailing separator and a trailing invisible child must not + // appear in the rendered output. + let children = vec![ + make_layout(1, Some("separator"), Some(true)), + make_layout(2, None, Some(true)), + make_layout(3, Some("separator"), Some(true)), // trailing sep + ]; + let rendered: Vec<_> = renderable_children(&children).collect(); + // Only the non-separator visible item (id 2) should survive. + assert_eq!(rendered.len(), 1); + assert_eq!(rendered[0].0, 2); + } + + #[test] + fn renderable_children_collapses_consecutive_separators() { + // Consecutive separators must collapse to a single separator. + let children = vec![ + make_layout(1, None, Some(true)), + make_layout(2, Some("separator"), Some(true)), + make_layout(3, Some("separator"), Some(true)), + make_layout(4, None, Some(true)), + ]; + let rendered: Vec<_> = renderable_children(&children).collect(); + // Items 1, 4 (normal) + one separator (id 2, the first of the pair). + assert_eq!(rendered.len(), 3); + assert_eq!(rendered[0].0, 1); + assert_eq!(rendered[1].0, 2); + assert_eq!(rendered[2].0, 4); + } + + #[test] + fn renderable_children_drops_invisible_items() { + let children = vec![ + make_layout(1, None, Some(true)), + make_layout(2, None, Some(false)), // invisible + make_layout(3, None, Some(true)), + ]; + let rendered: Vec<_> = renderable_children(&children).collect(); + assert_eq!(rendered.len(), 2); + assert_eq!(rendered[0].0, 1); + assert_eq!(rendered[1].0, 3); + } + + // --- TrayModule::is_blocklisted --- + // The blocklist is the part of `is_visible` that is independent of a + // live D-Bus connection: it only inspects the item *name* against the + // configured regexes. The passive-status half of `is_visible` is + // exercised by the unit tests in `services::tray` and by the live + // integration (icons resolving after reboot). + + fn module_with_blocklist(patterns: &[&str]) -> TrayModule { + let config = TrayModuleConfig { + blocklist: patterns + .iter() + .map(|p| RegexCfg(regex::Regex::new(p).unwrap())) + .collect(), + right_click: None, + }; + TrayModule::new(config) + } + + #[test] + fn is_blocklisted_matches_regex_pattern() { + let module = module_with_blocklist(&["^telegram"]); + assert!(module.is_blocklisted("telegram")); + assert!(!module.is_blocklisted("nextcloud")); + } + + #[test] + fn is_blocklisted_matches_partial_pattern() { + // A pattern without anchors matches a substring of the name. + let module = module_with_blocklist(&["blueman"]); + assert!(module.is_blocklisted("org.blueman.sni")); + assert!(!module.is_blocklisted("telegram")); + } + + #[test] + fn is_blocklisted_empty_blocklist_matches_nothing() { + let module = module_with_blocklist(&[]); + assert!(!module.is_blocklisted("anything")); + } + + #[test] + fn is_blocklisted_multiple_patterns_any_match() { + // `is_blocklisted` returns true if *any* pattern matches. + let module = module_with_blocklist(&["telegram", "nextcloud"]); + assert!(module.is_blocklisted("nextcloud")); + assert!(module.is_blocklisted("telegram")); + assert!(!module.is_blocklisted("blueman")); + } +} diff --git a/src/services/tray/dbus.rs b/src/services/tray/dbus.rs index 5e7ae9dcf..f005cdfe9 100644 --- a/src/services/tray/dbus.rs +++ b/src/services/tray/dbus.rs @@ -16,9 +16,23 @@ const NAME: WellKnownName = WellKnownName::from_static_str_unchecked("org.kde.StatusNotifierWatcher"); const OBJECT_PATH: &str = "/StatusNotifierWatcher"; +/// How many times the `NameOwnerChanged` fast path re-probes a +/// `StatusNotifierItem`-suffixed name whose item object is not ready yet. +/// Clients grab their well-known name *before* exporting `/StatusNotifierItem` +/// (startup race), so the first probe can fire while the object does not +/// exist yet. Re-probing briefly closes that window; without it the sender +/// would be registered into `items` before its properties are readable and +/// the item would be lost forever (every later probe is deduped on sender). +const READY_PROBE_ATTEMPTS: usize = 6; +const READY_PROBE_INTERVAL: Duration = Duration::from_secs(1); + #[derive(Debug, Default)] pub struct StatusNotifierWatcher { items: Vec<(UniqueName<'static>, String)>, + /// Name of the host service that registered itself via + /// `RegisterStatusNotifierHost`. Tracked so we can emit + /// `StatusNotifierHostRegistered` exactly once (not in a loop). + host_name: Option>, } impl StatusNotifierWatcher { @@ -38,7 +52,6 @@ impl StatusNotifierWatcher { if dbus_proxy.request_name(NAME, flags).await? == RequestNameReply::InQueue { warn!("Bus name '{NAME}' already owned"); } - let emitter = SignalEmitter::new(&connection, OBJECT_PATH)?; Self::status_notifier_host_registered(&emitter).await?; @@ -49,6 +62,13 @@ impl StatusNotifierWatcher { let unique_name = internal_connection.unique_name().map(|x| x.as_ref()); let mut name_owner_changed_stream = name_owner_changed_stream.fuse(); + // Discovery tick: a slow self-healing fallback. Late-starting + // clients are caught by the `NameOwnerChanged` fast path below + // (probe a name when it appears, with a short readiness retry), + // and the full scan at startup covers names that already existed. + // The periodic scan only catches the remaining case — an item + // whose bus name predates this watcher but whose item object + // appeared later — so it can stay slow. let mut interval = tokio::time::interval(Duration::from_secs(30)); loop { @@ -66,33 +86,56 @@ impl StatusNotifierWatcher { info!("Lost bus name: {NAME}"); have_bus_name = false; } - } else if let BusName::Unique(name) = &args.name { - let mut interface = internal_interface.get_mut().await; - if let Some(idx) = interface - .items - .iter() - .position(|(unique_name, _)| unique_name == name) - { - let emitter = match - SignalEmitter::new(&internal_connection, OBJECT_PATH) { - Ok(e) => e, - Err(e) => { - warn!("Failed to create signal emitter: {e}"); - continue; - } - }; - let service = interface.items.remove(idx).1; - if let Err(e) = StatusNotifierWatcher::status_notifier_item_unregistered( - &emitter, &service, - ) - .await { - warn!("Failed to emit item_unregistered signal: {e}"); + } else if args.new_owner.as_ref().is_none() { + // Name left the bus: if it owned a tracked SNI + // item, emit `ItemUnregistered` and drop it. + if let BusName::Unique(name) = &args.name { + let mut interface = internal_interface.get_mut().await; + if let Some(idx) = interface + .items + .iter() + .position(|(unique_name, _)| unique_name == name) + { + let emitter = match + SignalEmitter::new(&internal_connection, OBJECT_PATH) + { + Ok(e) => e, + Err(e) => { + warn!("Failed to create signal emitter: {e}"); + continue; + } + }; + let service = interface.items.remove(idx).1; + if let Err(e) = StatusNotifierWatcher::status_notifier_item_unregistered( + &emitter, &service, + ) + .await { + warn!("Failed to emit item_unregistered signal: {e}"); + } } } + } else { + // A name appeared: probe it (with a short + // readiness retry) instead of waiting for the + // periodic discovery tick, so a late-starting + // client shows up immediately. Run outside the + // select loop so a slow/unresponsive owner cannot + // stall signal handling; the sender-based dedup + // in `register_status_notifier_item_manual` makes + // racing with the periodic scan harmless. + let conn = internal_connection.clone(); + let interface = internal_interface.clone(); + let name = args.name.to_string(); + tokio::spawn(async move { + Self::probe_and_register_name(&conn, &interface, &name).await; + }); } } _ = interval.tick() => { - if let Err(e) = Self::discover_items(&internal_connection, &internal_interface).await { + if let Err(e) = + Self::discover_items(&internal_connection, &internal_interface) + .await + { info!("Failed to discover tray items: {e}"); } } @@ -108,36 +151,154 @@ impl StatusNotifierWatcher { Ok(connection) } + /// Re-sync the watcher's tracked items from the watcher's own + /// `RegisteredItems` property. This is the key fix for late-start: + /// a client that registered via the `RegisterStatusNotifierItem` + /// method *before* this watcher existed (or before it re-armed) + /// emits `StatusNotifierItemRegistered` only once, so a + /// signal-only consumer misses it. Re-reading `RegisteredItems` + /// on each discovery tick makes the watcher authoritative and + /// self-healing, mirroring what a restart does — without requiring + /// the user to restart the application. + /// + /// Each service in `RegisteredItems` is of the form + /// `/` (for method-registered items) or + /// 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( + conn: &Connection, + interface: &zbus::object_server::InterfaceRef, + ) { + let mut watcher = interface.get_mut().await; + let registered = watcher.registered_status_notifier_items(); + + let emitter = match SignalEmitter::new(conn, OBJECT_PATH) { + Ok(e) => e, + Err(e) => { + warn!("Failed to create signal emitter: {e}"); + return; + } + }; + + for service in ®istered { + let (sender, _path) = split_service_name(service); + // Skip items already tracked by `self.items` (registered via + // the method or the well-known-name probe path) so we don't + // re-emit `StatusNotifierItemRegistered` for the same item. + if watcher.items.iter().any(|(s, _)| s.as_ref() == sender) { + continue; + } + let _ = StatusNotifierWatcher::status_notifier_item_registered(&emitter, service).await; + // The sender is a unique name like `:1.131`; build a + // `UniqueName<'static>` from the borrowed string. + let sender = match UniqueName::try_from(sender.to_owned()) { + Ok(s) => s, + Err(_) => continue, + }; + watcher.items.push((sender, service.clone())); + } + } + async fn discover_items( conn: &Connection, interface: &zbus::object_server::InterfaceRef, ) -> anyhow::Result<()> { + // Re-sync from the watcher's `RegisteredItems` first: catches + // clients that registered via the `RegisterStatusNotifierItem` + // method but whose signal we missed (e.g. they registered + // before this watcher existed). Without this, such items only + // appear after a restart. + Self::sync_registered_items(conn, interface).await; + let dbus_proxy = DBusProxy::new(conn).await?; let names = dbus_proxy.list_names().await?; - for name in names { - let name_str = name.as_str(); - if name_str.starts_with(':') - || name_str == NAME.as_str() - || name_str == "org.freedesktop.DBus" - { - continue; - } - - if name_str.contains("StatusNotifierItem") { - let sender = match dbus_proxy.get_name_owner(BusName::from(name.clone())).await { - Ok(owner) => owner, - Err(_) => continue, - }; + // Probe every name (well-known AND unique) concurrently so a slow + // or unresponsive name does not block the watcher's event loop. + // Probing unique names is essential: Qt/GTK/Chromium SNI clients + // (telegram, nextcloud, ...) register via the + // `RegisterStatusNotifierItem` method and live under a unique + // name; their item may not be reachable from the well-known + // name, and a missed registration signal means the item only + // appeared after a restart. Concurrent probes keep the tick + // bounded regardless of how many names are on the bus. + let futures = names + .into_iter() + .filter(|name| { + let s = name.as_str(); + s != NAME.as_str() && s != "org.freedesktop.DBus" + }) + .map(|name| { + let conn = conn.clone(); + let interface = interface.clone(); + async move { + Self::probe_and_register_name(&conn, &interface, name.as_str()).await; + } + }); + iced::futures::future::join_all(futures).await; + Ok(()) + } - let mut watcher = interface.get_mut().await; + /// Probe a single bus name for an `org.kde.StatusNotifierItem` interface + /// and register the item if found and not already tracked. Shared by the + /// periodic discovery tick and the `NameOwnerChanged` fast path. + async fn probe_and_register_name( + conn: &Connection, + interface: &zbus::object_server::InterfaceRef, + name: &str, + ) { + let dbus_proxy = match DBusProxy::new(conn).await { + Ok(p) => p, + Err(_) => return, + }; + let bus_name = match BusName::try_from(name) { + Ok(n) => n, + Err(_) => return, + }; + let sender = match dbus_proxy.get_name_owner(bus_name).await { + Ok(owner) => owner, + Err(_) => return, + }; + // Skip if already tracked (by the method handler, the + // `RegisteredItems` sync, or a previous probe) to avoid + // re-emitting `StatusNotifierItemRegistered` for the same item. + // Checked *before* the readiness probe so the periodic scan does + // not re-probe items that are already registered. + let tracked: std::collections::HashSet = interface + .get() + .await + .items + .iter() + .map(|(s, _)| s.as_str().to_owned()) + .collect(); + if tracked.contains(sender.as_str()) { + return; + } + // A name containing the `StatusNotifierItem` suffix is almost + // certainly an SNI item, but its owner may not have exported the + // `/StatusNotifierItem` object yet when the name appeared (clients + // grab their well-known name first, set up the object later). Re-probe + // briefly so we do not register — and burn the item in `items` — at a + // moment when the UI still cannot read its properties. Names that do + // not look like SNI (e.g. unique names probed by the periodic scan) + // are checked once, to avoid a re-probe storm whenever a process + // appears on the bus. + let attempts = if name.contains("StatusNotifierItem") { + READY_PROBE_ATTEMPTS + } else { + 1 + }; + for attempt in 0..attempts { + if Self::is_status_notifier_item(conn, name).await { let emitter = match SignalEmitter::new(conn, OBJECT_PATH) { Ok(e) => e, Err(e) => { warn!("Failed to create signal emitter: {e}"); - continue; + return; } }; + let mut watcher = interface.get_mut().await; watcher .register_status_notifier_item_manual( "/StatusNotifierItem", @@ -145,9 +306,51 @@ impl StatusNotifierWatcher { &emitter, ) .await; + return; + } + if attempt + 1 < attempts { + tokio::time::sleep(READY_PROBE_INTERVAL).await; } } - Ok(()) + } + + /// Probe whether a name exposes an `org.kde.StatusNotifierItem` interface. + /// Some SNI clients (Qt `QSystemTrayIcon`, GTK `GtkStatusIcon`, + /// Chromium/CEF) register the item under a well-known name that does *not* + /// contain the `StatusNotifierItem` suffix, so a suffix-only match misses + /// them. We probe the standard object path `/StatusNotifierItem` with a + /// short timeout (500ms) so discovery stays fast for late-start. + /// + /// The object is always verified rather than trusting the name suffix: + /// a client grabs its well-known name *before* exporting the item object, + /// so a suffix-only match would let us register — and burn — an item + /// whose properties the UI still cannot read. + async fn is_status_notifier_item(conn: &Connection, name: &str) -> bool { + // Probe the standard object path for the SNI interface. + // A single property read (`IconName`) confirms the item exists. + // Disable property caching so zbus does not call `GetAll` on + // `/StatusNotifierItem` at `build()` time — that emits a WARN + // ("Object does not exist at path /StatusNotifierItem") on every + // discovery probe for names that do not yet (or no longer) + // expose the object. The probe only needs a single method + // call (`icon_name`), which works without a cached property. + let builder = match StatusNotifierItemProxy::builder(conn) + .destination(name.to_owned()) + .and_then(|b| b.path("/StatusNotifierItem")) + { + Ok(b) => b, + Err(_) => return false, + }; + // `.cache_properties` is infallible (returns `Builder`, not `Result`); + // call it before `.build()` so the `GetAll` side-effect never fires. + let builder = builder.cache_properties(zbus::proxy::CacheProperties::No); + match builder.build().await { + Ok(proxy) => tokio::time::timeout(Duration::from_millis(500), proxy.icon_name()) + .await + .map(|r| r.is_ok()) + .unwrap_or(false), + Err(_) => false, + } } } @@ -205,8 +408,24 @@ impl StatusNotifierWatcher { async fn register_status_notifier_host( &mut self, _service: &str, + #[zbus(header)] header: Header<'_>, #[zbus(signal_emitter)] emitter: SignalEmitter<'_>, ) { + // A host registers itself with the watcher. Save its name and emit + // the `StatusNotifierHostRegistered` signal exactly once. Emitting it + // on every call would loop (clients re-register), so guard on + // `host_name`. + let sender = match header.sender() { + Some(s) => s.to_owned(), + None => { + warn!("D-Bus message has no sender"); + return; + } + }; + if self.host_name.replace(sender).is_some() { + // already registered a host; ignore subsequent registrations + return; + } let _ = Self::status_notifier_host_registered(&emitter).await; } @@ -222,7 +441,9 @@ impl StatusNotifierWatcher { #[zbus(property)] fn protocol_version(&self) -> i32 { - 0 + // `1` is the protocol version clients check for (`> 0`). Returning + // `0` may be interpreted as "unsupported" by some SNI clients. + 1 } #[zbus(signal)] @@ -251,6 +472,32 @@ pub struct Icon { pub bytes: Vec, } +impl Icon { + /// Pixel count (width * height). Used to pick the largest pixmap. + fn pixel_count(&self) -> u32 { + (self.width.max(0) as u32) + .checked_mul(self.height.max(0) as u32) + .unwrap_or(0) + } +} + +/// Pick the largest icon (by pixel count) from a set of pixmaps. SNI clients +/// expose `IconPixmap` as an array of pixmaps at multiple resolutions; the +/// largest is preferred for rendering. +pub fn best_icon_pixmap(icons: &[Icon]) -> Option<&Icon> { + icons.iter().max_by_key(|i| i.pixel_count()) +} + +/// Split a registered service name into its unique sender and object path. +/// A method-registered item is `/`; a +/// well-known-name registration is a bare ``. +pub(crate) fn split_service_name(name: &str) -> (&str, &str) { + match name.find('/') { + Some(idx) => (&name[..idx], &name[idx..]), + None => (name, "/StatusNotifierItem"), + } +} + #[proxy(interface = "org.kde.StatusNotifierItem")] pub trait StatusNotifierItem { #[zbus(property)] @@ -262,8 +509,36 @@ pub trait StatusNotifierItem { #[zbus(property)] fn menu(&self) -> zbus::Result; + /// SNI `Status`: `Passive` / `Active` / `NeedsAttention`. + #[zbus(property)] + fn status(&self) -> zbus::Result; + + /// SNI `IconThemePath`: extra icon theme search path. + #[zbus(property)] + fn icon_theme_path(&self) -> zbus::Result; + + /// SNI `AttentionIconName`. + #[zbus(property)] + fn attention_icon_name(&self) -> zbus::Result; + + /// SNI `AttentionIconPixmap`. + #[zbus(property)] + fn attention_icon_pixmap(&self) -> zbus::Result>; + + /// SNI `OverlayIconName`. + #[zbus(property)] + fn overlay_icon_name(&self) -> zbus::Result; + + /// SNI `OverlayIconPixmap`. + #[zbus(property)] + fn overlay_icon_pixmap(&self) -> zbus::Result>; + + #[zbus(signal)] + async fn new_icon(&self) -> zbus::Result<()>; + + /// SNI `NewStatus` signal (status changed). #[zbus(signal)] - fn new_icon(&self) -> zbus::Result<()>; + async fn new_status(&self, status: &str) -> zbus::Result<()>; fn activate(&self, x: i32, y: i32) -> zbus::Result<()>; } @@ -311,6 +586,126 @@ pub trait DBusMenu { fn about_to_show(&self, id: i32) -> zbus::Result; + /// `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( + &self, + item_ids: &[i32], + property_names: &[&str], + ) -> zbus::Result; + + /// `GetProperty` — single property read for one item id. + fn get_property(&self, item_id: i32, name: &str) -> zbus::Result; + #[zbus(signal)] fn layout_updated(&self, revision: u32, parent: i32) -> zbus::Result<()>; + + /// `ItemsRemoved` is a dbusmenu signal (not an SNI signal). It indicates + /// the menu layout changed (items removed), so we refetch the menu. + #[zbus(signal)] + fn items_removed( + &self, + revision: u32, + path: String, + parent: i32, + item_ids: Vec, + ) -> zbus::Result<()>; +} + +#[cfg(test)] +mod tests { + use super::{Icon, StatusNotifierWatcher, best_icon_pixmap, split_service_name}; + + fn icon(width: i32, height: i32) -> Icon { + Icon { + width, + height, + bytes: vec![0u8; (width.max(0) as usize) * (height.max(0) as usize) * 4], + } + } + + #[test] + fn protocol_version_is_one() { + let watcher = StatusNotifierWatcher::default(); + assert_eq!(watcher.protocol_version(), 1); + } + + #[test] + fn host_is_reported_registered() { + let watcher = StatusNotifierWatcher::default(); + assert!(watcher.is_status_notifier_host_registered()); + } + + #[test] + fn best_icon_pixmap_picks_largest_by_pixel_count() { + let small = icon(16, 16); + let large = icon(48, 48); + let medium = icon(24, 24); + + let icons = vec![small, large.clone(), medium]; + let best = best_icon_pixmap(&icons).expect("expected a best icon"); + + assert_eq!(best.width, 48); + assert_eq!(best.height, 48); + assert_eq!(best.pixel_count(), 48 * 48); + } + + #[test] + fn best_icon_pixmap_ignores_zero_dimension_entries() { + let zero = icon(0, 32); + let valid = icon(16, 16); + + let icons = vec![zero, valid]; + let best = best_icon_pixmap(&icons).expect("expected a best icon"); + + assert_eq!(best.width, 16); + assert_eq!(best.height, 16); + // The zero-dimension entry has pixel count 0 and must not win. + assert_eq!(best.pixel_count(), 16 * 16); + } + + #[test] + fn best_icon_pixmap_returns_none_for_empty_set() { + assert!(best_icon_pixmap(&[]).is_none()); + } + + #[test] + fn pixel_count_saturates_on_overflow() { + let huge = Icon { + width: i32::MAX, + height: i32::MAX, + bytes: Vec::new(), + }; + assert_eq!(huge.pixel_count(), 0); + } + + #[test] + fn split_service_name_splits_unique_sender_from_path() { + let (sender, path) = split_service_name(":1.131/StatusNotifierItem"); + assert_eq!(sender, ":1.131"); + assert_eq!(path, "/StatusNotifierItem"); + } + + #[test] + fn split_service_name_splits_at_first_slash() { + let (sender, path) = split_service_name(":1.283/org/blueman/sni"); + assert_eq!(sender, ":1.283"); + assert_eq!(path, "/org/blueman/sni"); + } + + #[test] + fn split_service_name_defaults_path_for_bare_well_known_name() { + let (sender, path) = split_service_name("org.kde.StatusNotifierItem-123"); + assert_eq!(sender, "org.kde.StatusNotifierItem-123"); + assert_eq!(path, "/StatusNotifierItem"); + } + + #[test] + fn split_service_name_handles_bare_path() { + // A bare path (leading `/`) splits at index 0: the sender is the + // empty string and the path is the bare path itself. + let (sender, path) = split_service_name("/StatusNotifierItem"); + assert_eq!(sender, ""); + assert_eq!(path, "/StatusNotifierItem"); + } } diff --git a/src/services/tray/mod.rs b/src/services/tray/mod.rs index d46ab4a9b..ac88adc78 100644 --- a/src/services/tray/mod.rs +++ b/src/services/tray/mod.rs @@ -72,48 +72,87 @@ fn pixmap_to_icon(icons: Vec) -> Option { } async fn current_icon_from_proxy(item_proxy: &StatusNotifierItemProxy<'_>) -> Option { - match item_proxy.icon_pixmap().await.ok().and_then(pixmap_to_icon) { - Some(icon) => Some(icon), - None => item_proxy - .icon_name() - .await - .ok() - .as_deref() - .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) + && let Some(loaded) = pixmap_to_icon(vec![icon.clone()]) + { + return Some(loaded); } -} - -fn split_service_name(name: &str) -> (&str, &str) { - match name.find('/') { - Some(idx) => (&name[..idx], &name[idx..]), - None => (name, "/StatusNotifierItem"), + // 2. `IconName` via the XDG icon theme. + if let Some(name) = item_proxy.icon_name().await.ok() + && !name.is_empty() + && let Some(icon) = xdg_icons::get_icon_from_name(&name) + { + return Some(icon); } + None } + #[derive(Debug, Clone)] pub enum TrayEvent { - Registered(StatusNotifierItem), + Registered(Box), IconChanged(String, TrayIcon), MenuLayoutChanged(String, Layout), + /// `NewStatus` signal from the SNI client. `status` is + /// `Passive` / `Active` / `NeedsAttention`. + StatusChanged(String, String), Unregistered(String), None, } +/// SNI status (subset of the spec values). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum ItemStatus { + /// `Passive` — not active (hidden from the tray until an `active` + /// registrant exists). + #[default] + Passive, + /// `Active` — active. + Active, + /// `NeedsAttention` — needs attention (use attention/overlay icons). + NeedsAttention, +} + +impl From<&str> for ItemStatus { + fn from(s: &str) -> Self { + match s { + "Active" => Self::Active, + "NeedsAttention" => Self::NeedsAttention, + _ => Self::Passive, + } + } +} + #[derive(Debug, Clone)] pub struct StatusNotifierItem { pub name: String, pub icon: Option, pub menu: Layout, + /// SNI `Status` (`Passive` / `Active` / `NeedsAttention`). + pub status: ItemStatus, + /// Attention icon (used when `NeedsAttention`). + pub attention_icon: Option, + /// Overlay icon. + pub overlay_icon: Option, item_proxy: StatusNotifierItemProxy<'static>, menu_proxy: DBusMenuProxy<'static>, } impl StatusNotifierItem { pub async fn new(conn: &zbus::Connection, name: String) -> anyhow::Result { - let (dest, path) = split_service_name(&name); - + let (dest, path) = dbus::split_service_name(&name); + + // Disable property caching: zbus would otherwise call `GetAll` + // on `/StatusNotifierItem` at `build()` time, which emits a + // WARN ("Object does not exist at path /StatusNotifierItem") + // when the client has not yet exported the object. The + // subsequent explicit property reads (below) work the same + // without a cached baseline. let item_proxy = StatusNotifierItemProxy::builder(conn) .destination(dest.to_owned())? .path(path.to_owned())? + .cache_properties(zbus::proxy::CacheProperties::No) .build() .await?; @@ -121,6 +160,46 @@ impl StatusNotifierItem { let icon = current_icon_from_proxy(&item_proxy).await; + // Full SNI properties. Each read is guarded — some clients (or + // clients that expose only a subset) may not implement a property; + // fall back to a default rather than failing. + // A client that does not implement `Status` (the read fails) is not + // necessarily a passive item: defaulting to `Passive` would hide it + // from the tray entirely. Only an explicit `Passive` reply should + // hide an item, so fall back to `Active`. + let status_str = item_proxy + .status() + .await + .unwrap_or_else(|_| "Active".to_owned()); + let status = ItemStatus::from(status_str.as_str()); + + // Attention/overlay icons: pixmap first (preferred), then icon name. + let attention_pixmap = item_proxy + .attention_icon_pixmap() + .await + .ok() + .and_then(pixmap_to_icon); + let attention_name = item_proxy + .attention_icon_name() + .await + .ok() + .as_deref() + .and_then(xdg_icons::get_icon_from_name); + let attention_icon = attention_pixmap.or(attention_name); + + let overlay_pixmap = item_proxy + .overlay_icon_pixmap() + .await + .ok() + .and_then(pixmap_to_icon); + let overlay_name = item_proxy + .overlay_icon_name() + .await + .ok() + .as_deref() + .and_then(xdg_icons::get_icon_from_name); + let overlay_icon = overlay_pixmap.or(overlay_name); + let menu_path = item_proxy.menu().await?; let menu_proxy = dbus::DBusMenuProxy::builder(conn) .destination(dest.to_owned())? @@ -134,6 +213,9 @@ impl StatusNotifierItem { name, icon, menu, + status, + attention_icon, + overlay_icon, item_proxy, menu_proxy, }) @@ -205,7 +287,7 @@ impl TrayService { let item = StatusNotifierItem::new(&conn, args.service.to_string()).await; - item.map(TrayEvent::Registered).ok() + item.map(|item| TrayEvent::Registered(Box::new(item))).ok() } _ => None, } @@ -231,6 +313,7 @@ impl TrayService { let mut icon_name_change = Vec::with_capacity(items.len()); let mut new_icon_change = Vec::with_capacity(items.len()); let mut menu_layout_change = Vec::with_capacity(items.len()); + let mut status_change = Vec::with_capacity(items.len()); for name in items { let item = StatusNotifierItem::new(conn, name.to_string()).await?; @@ -275,9 +358,32 @@ impl TrayService { .boxed(), ); + let new_status = item.item_proxy.receive_new_status().await; + if let Ok(new_status) = new_status { + // The `NewStatus` signal has no matching property-changed signal, + // so we re-read the `Status` property (uncached) rather than + // relying on the signal payload struct field name. + status_change.push( + new_status + .filter_map({ + let name = name.clone(); + let proxy = item.item_proxy.clone(); + move |_| { + let name = name.clone(); + let proxy = proxy.clone(); + async move { + let status = proxy.status().await.ok()?; + Some(TrayEvent::StatusChanged(name.to_owned(), status)) + } + } + }) + .boxed(), + ); + } + let new_icon = item.item_proxy.receive_new_icon().await; if let Ok(new_icon) = new_icon { - let (dest, path) = split_service_name(&name); + let (dest, path) = dbus::split_service_name(&name); // NewIcon has no matching PropertiesChanged, so a cached read would be stale; let uncached_proxy = StatusNotifierItemProxy::builder(conn) .destination(dest.to_owned())? @@ -331,6 +437,33 @@ impl TrayService { .boxed(), ); } + + // `ItemsRemoved` is a dbusmenu signal (not an SNI signal). When the + // menu layout changes (items removed), refetch the menu. + let items_removed = item.menu_proxy.receive_items_removed().await; + if let Ok(items_removed) = items_removed { + menu_layout_change.push( + items_removed + .filter_map({ + let name = name.clone(); + let menu_proxy = item.menu_proxy.clone(); + move |_| { + debug!("items removed event name {}", name); + + let name = name.clone(); + let menu_proxy = menu_proxy.clone(); + async move { + menu_proxy.get_layout(0, -1, &[]).await.ok().map( + |(_, layout)| { + TrayEvent::MenuLayoutChanged(name.to_owned(), layout) + }, + ) + } + } + }) + .boxed(), + ); + } } Ok(stream_select!( @@ -339,7 +472,8 @@ impl TrayService { select_all(icon_pixel_change), select_all(icon_name_change), select_all(new_icon_change), - select_all(menu_layout_change) + select_all(menu_layout_change), + select_all(status_change) ) .boxed()) } @@ -384,6 +518,12 @@ impl TrayService { while let Some(event) = events.next().await { debug!("tray data {event:?}"); + // Per-item signal subscriptions (icon/menu/status + // changes) are only created in `events()` for the + // items present at call time. A newly registered + // item arrives via the `Registered` signal without + // any subscriptions, so restart the event loop to + // rebuild the stream including the new item. let reload_events = matches!(event, TrayEvent::Registered(_)); let _ = output.send(ServiceEvent::Update(event)).await; @@ -437,6 +577,10 @@ impl ReadOnlyService for TrayService { fn update(&mut self, event: Self::UpdateEvent) { match event { TrayEvent::Registered(new_item) => { + let new_item = *new_item; + // The watcher registers each sender (unique bus name) only + // once, so a duplicate `Registered` cannot arrive; replace + // by name as a defensive fallback, otherwise append. match self .data .0 @@ -462,6 +606,11 @@ impl ReadOnlyService for TrayService { item.menu = layout; } } + TrayEvent::StatusChanged(name, status) => { + if let Some(item) = self.data.0.iter_mut().find(|item| item.name == name) { + item.status = ItemStatus::from(status.as_str()); + } + } TrayEvent::Unregistered(name) => { self.data.0.retain(|item| item.name != name); } @@ -538,3 +687,97 @@ impl Service for TrayService { } } } + +#[cfg(test)] +mod tests { + use super::{ItemStatus, pixmap_to_icon}; + use crate::services::tray::dbus::Icon; + + fn valid_pixmap(width: i32, height: i32) -> Icon { + Icon { + width, + height, + // `width * height * 4` ARGB bytes — the byte contract + // `pixmap_to_icon` requires to accept a pixmap. + bytes: vec![0u8; (width as usize) * (height as usize) * 4], + } + } + + #[test] + fn item_status_parses_active() { + assert_eq!(ItemStatus::from("Active"), ItemStatus::Active); + } + + #[test] + fn item_status_parses_needs_attention() { + assert_eq!( + ItemStatus::from("NeedsAttention"), + ItemStatus::NeedsAttention + ); + } + + #[test] + fn item_status_defaults_passive_for_unknown_and_empty() { + assert_eq!(ItemStatus::from("Passive"), ItemStatus::Passive); + assert_eq!(ItemStatus::from(""), ItemStatus::Passive); + // Unknown values fall back to `Passive` (the default). + assert_eq!(ItemStatus::from("garbage"), ItemStatus::Passive); + } + + #[test] + fn item_status_default_is_passive() { + assert_eq!(ItemStatus::default(), ItemStatus::Passive); + } + + #[test] + fn pixmap_to_icon_accepts_valid_pixmap() { + let icon = valid_pixmap(24, 24); + let result = pixmap_to_icon(vec![icon]); + // A valid pixmap (correct byte length, positive dimensions) + // must resolve to a `TrayIcon::Image`. + assert!(result.is_some()); + } + + #[test] + fn pixmap_to_icon_rejects_zero_dimension_pixmap() { + // A zero-width or zero-height pixmap is dropped up front. + let icon = Icon { + width: 0, + height: 24, + bytes: vec![], + }; + assert!(pixmap_to_icon(vec![icon]).is_none()); + } + + #[test] + fn pixmap_to_icon_rejects_byte_mismatch() { + // Dimensions say 24x24 but the payload is too short: the + // expected byte count (`24 * 24 * 4`) does not match, so the + // pixmap is dropped rather than fed to the atlas uploader. + let icon = Icon { + width: 24, + height: 24, + bytes: vec![0u8; 100], + }; + assert!(pixmap_to_icon(vec![icon]).is_none()); + } + + #[test] + fn pixmap_to_icon_picks_largest_pixmap() { + // Two pixmaps of different sizes: the largest (by pixel count) + // must be the one returned. The selection itself is locked down + // by the `best_icon_pixmap` tests in `dbus.rs`; here we confirm + // that a mixed set still yields an `Image` icon. + let small = valid_pixmap(16, 16); + let large = valid_pixmap(48, 48); + + let result = pixmap_to_icon(vec![small, large]).expect("expected an icon"); + + assert!(matches!(result, crate::services::tray::TrayIcon::Image(_))); + } + + #[test] + fn pixmap_to_icon_returns_none_for_empty_vec() { + assert!(pixmap_to_icon(Vec::new()).is_none()); + } +} diff --git a/src/services/xdg_icons.rs b/src/services/xdg_icons.rs index 77babb92c..74d677eb7 100644 --- a/src/services/xdg_icons.rs +++ b/src/services/xdg_icons.rs @@ -444,7 +444,9 @@ fn icon_directories() -> Vec { #[cfg(test)] mod tests { - use super::{get_icon_from_name, is_safe_icon_name}; + use super::{ + get_icon_from_name, is_safe_icon_name, normalize_icon_name, strip_icon_separators, + }; #[test] fn accepts_ordinary_icon_names() { @@ -488,4 +490,70 @@ mod tests { assert!(get_icon_from_name("/etc/passwd").is_none()); assert!(get_icon_from_name("").is_none()); } + + #[test] + fn normalize_icon_name_keeps_lowercase_alphanumeric_unchanged() { + // A name already in lowercase-alphanumeric form is returned as-is + // (borrowed, no allocation). + assert_eq!(normalize_icon_name("telegram"), "telegram"); + assert_eq!(normalize_icon_name("blueman"), "blueman"); + } + + #[test] + fn normalize_icon_name_strips_separators_in_mixed_names() { + // A name containing separators (`.`) is not all-lowercase-alphanumeric, + // so it goes through the owned path and the separators are stripped. + assert_eq!(normalize_icon_name("org.blueman"), "orgblueman"); + assert_eq!( + normalize_icon_name("org.telegram.desktop"), + "orgtelegramdesktop" + ); + } + + #[test] + fn normalize_icon_name_lowercases_and_strips_non_alphanumeric() { + // Uppercase is lowercased; separators and non-alphanumeric + // characters are stripped. + assert_eq!(normalize_icon_name("Telegram"), "telegram"); + assert_eq!( + normalize_icon_name("org.telegram.desktop"), + "orgtelegramdesktop" + ); + assert_eq!(normalize_icon_name("BlueMan!"), "blueman"); + assert_eq!(normalize_icon_name("123"), "123"); + } + + #[test] + fn normalize_icon_name_strips_unicode_non_ascii() { + // Non-ASCII characters are filtered out (only ASCII alphanumerics + // survive), lowercased. + assert_eq!(normalize_icon_name("naïve"), "nave"); + assert_eq!(normalize_icon_name("app-1.0"), "app10"); + } + + #[test] + fn strip_icon_separators_removes_dashes_and_underscores() { + assert_eq!(strip_icon_separators("org.telegram"), "org.telegram"); + assert_eq!( + strip_icon_separators("org-telegram-desktop"), + "orgtelegramdesktop" + ); + assert_eq!(strip_icon_separators("foo_bar-baz"), "foobarbaz"); + } + + #[test] + fn strip_icon_separators_no_separators_is_borrowed() { + // When there are no separators, the input is returned as-is. + assert_eq!(strip_icon_separators("telegram"), "telegram"); + assert_eq!(strip_icon_separators("org.telegram"), "org.telegram"); + } + + #[test] + fn normalize_then_strip_is_idempotent_for_clean_names() { + // Normalizing an already-clean name then stripping separators + // yields the same value — the pipeline is stable. + let normalized = normalize_icon_name("telegram"); + let stripped = strip_icon_separators(normalized.as_ref()); + assert_eq!(normalized, stripped); + } } diff --git a/website/docs/configuration/modules/tray.md b/website/docs/configuration/modules/tray.md index 693558f13..91ce9b96b 100644 --- a/website/docs/configuration/modules/tray.md +++ b/website/docs/configuration/modules/tray.md @@ -8,6 +8,21 @@ This module provides a system tray for displaying icons of running applications. 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 +(`org.kde.StatusNotifierItem` + `org.kde.StatusNotifierWatcher`), so it +works with Qt (`QSystemTrayIcon`), GTK (`GtkStatusIcon`), Chromium/CEF, +Gecko (`ksni`) and other SNI clients. + +## Status and attention icons + +SNI items expose a `Status` property: `Passive`, `Active` or +`NeedsAttention`. + +- **Passive** items are **hidden** from the tray until they become `Active`. +- **Active** items are shown normally. +- **NeedsAttention** items are shown using the client's **attention icon** + (and overlay icon when available) so the user notices them. + ## Blocklist You can filter which tray icons are displayed using the `blocklist` option. If a tray item's name matches any regex pattern in the blocklist, it won't be rendered.