[WIP] status-area: Handle menuless items properly

cosmic-applet-status-area/src/components/app.rs

@@ -7,7 +7,7 @@ use cosmic::{
     iced::{
         self,
         platform_specific::shell::commands::popup::{destroy_popup, get_popup},
-        window, Limits, Padding, Subscription,
+        window, Subscription,
     },
     surface,
     widget::{container, mouse_area},
@@ -26,6 +26,15 @@ pub enum Msg {
     TogglePopup(usize),
     Hovered(usize),
     Surface(surface::Action),
+    /// Open the item's context menu.
+    ///
+    /// Currently, this always calls `StatusNotifierItem.ContextMenu`.
+    ///
+    /// TODO: Decide if behavior should be different in cases like `ContextMenu` method missing,
+    /// but a menu being available.
+    OpenContextMenu {
+        id: usize,
+    },
 }
 
 #[derive(Default)]
@@ -188,6 +197,10 @@ impl cosmic::Application for App {
                 }
                 Task::none()
             }
+            Msg::OpenContextMenu { id } => {
+                self.menus[&id].ctx_menu_activate();
+                Task::none()
+            }
             Msg::Hovered(id) => {
                 let mut cmds = Vec::new();
                 if let Some(old_id) = self.open_menu.take() {
@@ -261,6 +274,7 @@ impl cosmic::Application for App {
                 .on_press_down(Msg::TogglePopup(*id)),
             )
             .on_enter(Msg::Hovered(*id))
+            .on_right_press(Msg::OpenContextMenu { id: *id })
             .into()
         });
         self.core

cosmic-applet-status-area/src/components/status_menu.rs

@@ -3,7 +3,7 @@
 
 use cosmic::{
     applet::menu_button,
-    iced::{self, Padding},
+    iced::{self},
     widget::icon,
 };
 
@@ -71,8 +71,11 @@ impl State {
 
                 iced::Task::none()
             }
-            Msg::Click(id, is_submenu) => {
-                let menu_proxy = self.item.menu_proxy().clone();
+            Msg::Click(id, is_submenu) => 'block: {
+                let Some(menu_proxy) = self.item.menu_proxy().cloned() else {
+                    tracing::error!("Msg::click on item without menu_proxy");
+                    break 'block iced::Task::none();
+                };
                 tokio::spawn(async move {
                     let _ = menu_proxy.event(id, "clicked", &0.into(), 0).await;
                 });
@@ -112,27 +115,72 @@ impl State {
 
     pub fn subscription(&self) -> iced::Subscription<Msg> {
         iced::Subscription::batch([
-            self.item.layout_subscription().map(Msg::Layout),
+            self.item.menu_layout_subscription().map(Msg::Layout),
             self.item.icon_subscription().map(Msg::Icon),
         ])
     }
 
     pub fn opened(&self) {
-        let menu_proxy = self.item.menu_proxy().clone();
+        let Some(menu_proxy) = self.item.menu_proxy().cloned() else {
+            let item_proxy = self.item.item_proxy().clone();
+            tokio::spawn(async move {
+                let [x, y] = item_screen_pos_stub();
+                if let Err(e) = item_proxy.activate(x, y).await {
+                    tracing::error!("Error on Activate: {e}");
+                }
+            });
+            return;
+        };
+        let item_proxy = self.item.item_proxy().clone();
+        tokio::spawn(async move {
+            let is_menu = match item_proxy.item_is_menu().await {
+                Ok(is_menu) => is_menu,
+                Err(e) => {
+                    tracing::error!("Error on ItemIsMenu: {e}");
+                    false
+                }
+            };
+            if is_menu {
+                let _ = menu_proxy.event(0, "opened", &0i32.into(), 0).await;
+                let _ = menu_proxy.about_to_show(0).await;
+            } else {
+                let [x, y] = item_screen_pos_stub();
+                if let Err(e) = item_proxy.activate(x, y).await {
+                    tracing::error!("Error on Activate: {e}");
+                }
+            }
+        });
+    }
+
+    pub fn ctx_menu_activate(&self) {
+        let item_proxy = self.item.item_proxy().clone();
         tokio::spawn(async move {
-            let _ = menu_proxy.event(0, "opened", &0i32.into(), 0).await;
-            let _ = menu_proxy.about_to_show(0).await;
+            let [x, y] = item_screen_pos_stub();
+            if let Err(e) = item_proxy.context_menu(x, y).await {
+                tracing::error!("Error on ContextMenu: {e}");
+            }
         });
     }
 
     pub fn closed(&self) {
-        let menu_proxy = self.item.menu_proxy().clone();
+        let Some(menu_proxy) = self.item.menu_proxy().cloned() else {
+            return;
+        };
         tokio::spawn(async move {
             let _ = menu_proxy.event(0, "closed", &0i32.into(), 0).await;
         });
     }
 }
 
+/// Stub: Get the screen coordinates of an item
+///
+/// Used for passing coordinates for SNI `Activate` and `ContextMenu` methods.
+///
+/// TODO: Figure out how to implement
+fn item_screen_pos_stub() -> [i32; 2] {
+    [0, 0]
+}
+
 fn layout_view(layout: &Layout, expanded: Option<i32>) -> cosmic::Element<Msg> {
     iced::widget::column(layout.children().iter().filter_map(|i| {
         if !i.visible() {

cosmic-applet-status-area/src/subscriptions/status_notifier_item.rs

@@ -1,18 +1,15 @@
 // Copyright 2023 System76 <[email protected]>
 // SPDX-License-Identifier: GPL-3.0-only
 
-use cosmic::{
-    iced::{self, Subscription},
-    widget::icon,
-};
+use cosmic::iced::{self, Subscription};
 use futures::{FutureExt, StreamExt};
 use zbus::zvariant::{self, OwnedValue};
 
 #[derive(Clone, Debug)]
 pub struct StatusNotifierItem {
     name: String,
     item_proxy: StatusNotifierItemProxy<'static>,
-    menu_proxy: DBusMenuProxy<'static>,
+    menu_proxy: Option<DBusMenuProxy<'static>>,
 }
 
 #[derive(Clone, Debug, zvariant::Value)]
@@ -44,12 +41,22 @@ impl StatusNotifierItem {
             .build()
             .await?;
 
-        let menu_path = item_proxy.menu().await?;
-        let menu_proxy = DBusMenuProxy::builder(connection)
-            .destination(dest.to_string())?
-            .path(menu_path)?
-            .build()
-            .await?;
+        let menu_proxy = match item_proxy.menu().await {
+            Ok(menu_path) => Some(
+                DBusMenuProxy::builder(connection)
+                    .destination(dest.to_string())?
+                    .path(menu_path)?
+                    .build()
+                    .await?,
+            ),
+            Err(e) => {
+                let destination = item_proxy.inner().destination();
+                tracing::error!(
+                    "Error getting Menu property for {destination}: {e}. Treating as menuless item.",
+                );
+                None
+            }
+        };
 
         Ok(Self {
             name,
@@ -63,8 +70,10 @@ impl StatusNotifierItem {
     }
 
     // TODO: Only fetch changed part of layout, if that's any faster
-    pub fn layout_subscription(&self) -> iced::Subscription<Result<Layout, String>> {
-        let menu_proxy = self.menu_proxy.clone();
+    pub fn menu_layout_subscription(&self) -> iced::Subscription<Result<Layout, String>> {
+        let Some(menu_proxy) = self.menu_proxy.clone() else {
+            return iced::Subscription::none();
+        };
         Subscription::run_with_id(
             format!("status-notifier-item-layout-{}", &self.name),
             async move {
@@ -109,8 +118,12 @@ impl StatusNotifierItem {
         )
     }
 
-    pub fn menu_proxy(&self) -> &DBusMenuProxy<'static> {
-        &self.menu_proxy
+    pub fn menu_proxy(&self) -> Option<&DBusMenuProxy<'static>> {
+        self.menu_proxy.as_ref()
+    }
+
+    pub fn item_proxy(&self) -> &StatusNotifierItemProxy<'static> {
+        &self.item_proxy
     }
 }
 
@@ -135,6 +148,10 @@ trait StatusNotifierItem {
 
     #[zbus(signal)]
     fn new_icon(&self) -> zbus::Result<()>;
+
+    fn activate(&self, x: i32, y: i32) -> zbus::Result<()>;
+    fn context_menu(&self, x: i32, y: i32) -> zbus::Result<()>;
+    fn item_is_menu(&self) -> zbus::Result<bool>;
 }
 
 #[derive(Clone, Debug)]