quick commit

include/services/tray.hpp

@@ -52,7 +52,9 @@ class TrayService {
     };
     using MenuLayoutCallback = sigc::slot<void(std::optional<MenuNode>)>;
     void request_menu_layout(const std::string &id, MenuLayoutCallback callback);
-    bool activate_menu_item(const std::string &id, int itemId);
+    bool activate_menu_item(const std::string &id, int itemId, int32_t x = -1,
+                            int32_t y = -1, uint32_t button = 1,
+                            uint32_t timestampMs = 0);
 
     sigc::signal<void(const Item &)> &signal_item_added();
     sigc::signal<void(const std::string &)> &signal_item_removed();

include/widgets/tray.hpp

@@ -23,6 +23,7 @@
 class TrayIconWidget : public Button {
   public:
     TrayIconWidget(std::string id);
+    ~TrayIconWidget() override;
 
     void update(const TrayService::Item &item);
 
@@ -33,6 +34,7 @@ class TrayIconWidget : public Button {
     Gtk::Picture picture;
     Gtk::Image image;
     Glib::RefPtr<Gtk::GestureClick> primaryGesture;
+    Glib::RefPtr<Gtk::GestureClick> middleGesture;
     Glib::RefPtr<Gtk::GestureClick> secondaryGesture;
     Glib::RefPtr<Gtk::PopoverMenu> menuPopover;
     Glib::RefPtr<Gio::SimpleActionGroup> menuActions;
@@ -40,10 +42,12 @@ class TrayIconWidget : public Button {
     bool menuPopupPending = false;
     bool menuRequestInFlight = false;
     bool hasRemoteMenu = false;
+    std::shared_ptr<bool> aliveFlag;
     double pendingX       = 0.0;
     double pendingY       = 0.0;
 
     void on_primary_released(int n_press, double x, double y);
+    void on_middle_released(int n_press, double x, double y);
     void on_secondary_released(int n_press, double x, double y);
     void on_menu_layout_ready(std::optional<TrayService::MenuNode> layout);
     void
@@ -51,6 +55,7 @@ class TrayIconWidget : public Button {
                         const Glib::RefPtr<Gio::Menu> &menu,
                         const Glib::RefPtr<Gio::SimpleActionGroup> &actions);
     void on_menu_action(const Glib::VariantBase &parameter, int itemId);
+    bool try_get_pending_coords(int32_t &outX, int32_t &outY) const;
 };
 
 class TrayWidget : public Gtk::Box {

src/services/tray.cpp

@@ -561,7 +561,9 @@ void TrayService::request_menu_layout(const std::string &id,
                            &on_menu_layout_finished, data);
 }
 
-bool TrayService::activate_menu_item(const std::string &id, int itemId) {
+bool TrayService::activate_menu_item(const std::string &id, int itemId,
+                                     int32_t x, int32_t y, uint32_t button,
+                                     uint32_t timestampMs) {
     auto it = items.find(id);
     if (it == items.end() || !connection) {
         return false;
@@ -572,21 +574,36 @@ bool TrayService::activate_menu_item(const std::string &id, int itemId) {
         return false;
     }
 
-    // Some tray items update state lazily and require AboutToShow(itemId)
-    // before handling a click.
-    call_about_to_show(connection, item.publicData.busName,
-                       item.publicData.menuPath, itemId);
+    const guint32 nowMs = static_cast<guint32>(g_get_real_time() / 1000);
+    const guint32 ts    = timestampMs ? timestampMs : nowMs;
+
+    std::cerr << "[TrayService] MenuEvent id=" << id << " item=" << itemId
+              << " x=" << x << " y=" << y << " button=" << button
+              << " tsMs=" << ts << std::endl;
 
     // dbusmenu Event signature: (i s v u)
-    // For "clicked", the payload is typically an a{sv} dictionary.
-    // IMPORTANT: the 'v' argument must be a variant container, so we wrap.
+    // Some handlers (e.g., media players) look for both "timestamp" and
+    // "time" keys; send both alongside coords/button when available.
     GVariantBuilder dict;
     g_variant_builder_init(&dict, G_VARIANT_TYPE("a{sv}"));
+    g_variant_builder_add(&dict, "{sv}", "timestamp",
+                          g_variant_new_uint32(ts));
+    g_variant_builder_add(&dict, "{sv}", "time", g_variant_new_uint32(ts));
+
+    if (x != -1 && y != -1) {
+        g_variant_builder_add(&dict, "{sv}", "x", g_variant_new_int32(x));
+        g_variant_builder_add(&dict, "{sv}", "y", g_variant_new_int32(y));
+    }
+    if (button > 0) {
+        g_variant_builder_add(
+            &dict, "{sv}", "button",
+            g_variant_new_int32(static_cast<int32_t>(button)));
+    }
+
     GVariant *payloadDict = g_variant_builder_end(&dict);
     GVariant *payload     = g_variant_new_variant(payloadDict);
-    GVariant *params      = g_variant_new(
-        "(isvu)", itemId, "clicked", payload,
-        static_cast<guint32>(g_get_monotonic_time() / 1000));
+    GVariant *params      = g_variant_new("(isvu)", itemId, "clicked",
+                                          payload, ts);
 
     auto data        = new SimpleCallData();
     data->debugLabel = "MenuEvent(" + id + "," + std::to_string(itemId) + ")";

src/widgets/tray.cpp

@@ -6,6 +6,7 @@
 #include <cmath>
 #include <graphene.h>
 #include <utility>
+#include <iostream>
 #include "components/base/button.hpp"
 
 namespace {
@@ -140,11 +141,49 @@ bool try_get_global_pointer_coords(GtkWidget *widget, int32_t &outX,
     outY = static_cast<int32_t>(geom.y + std::lround(sy));
     return true;
 }
+
+bool has_popup_surface(GtkWidget *widget) {
+    if (!widget) {
+        return false;
+    }
+    GtkRoot *root = gtk_widget_get_root(widget);
+    if (!root) {
+        return false;
+    }
+    GtkNative *native = gtk_widget_get_native(widget);
+    if (!native) {
+        return false;
+    }
+    GdkSurface *surface = gtk_native_get_surface(native);
+    if (!surface) {
+        return false;
+    }
+    return gdk_surface_get_mapped(surface);
+}
+
+void log_menu_tree(const std::vector<TrayService::MenuNode> &nodes,
+                   int depth = 0) {
+    const std::string indent(static_cast<std::size_t>(depth) * 2, ' ');
+    for (const auto &node : nodes) {
+        if (!node.visible) {
+            continue;
+        }
+        std::cerr << "[TrayIconWidget] menu node id=" << node.id
+                  << " label='" << node.label << "' enabled="
+                  << (node.enabled ? "1" : "0") << " sep="
+                  << (node.separator ? "1" : "0") << " depth=" << depth
+                  << std::endl;
+        if (!node.children.empty()) {
+            log_menu_tree(node.children, depth + 1);
+        }
+    }
+}
 } // namespace
 
 TrayIconWidget::TrayIconWidget( std::string id)
     : Button(id),  id(std::move(id)),
       container(Gtk::Orientation::HORIZONTAL) {
+    aliveFlag = std::make_shared<bool>(true);
     set_has_frame(false);
     set_focusable(false);
     set_valign(Gtk::Align::CENTER);
@@ -175,6 +214,12 @@ TrayIconWidget::TrayIconWidget( std::string id)
         sigc::mem_fun(*this, &TrayIconWidget::on_primary_released));
     add_controller(primaryGesture);
 
+    middleGesture = Gtk::GestureClick::create();
+    middleGesture->set_button(GDK_BUTTON_MIDDLE);
+    middleGesture->signal_released().connect(
+        sigc::mem_fun(*this, &TrayIconWidget::on_middle_released));
+    add_controller(middleGesture);
+
     secondaryGesture = Gtk::GestureClick::create();
     secondaryGesture->set_button(GDK_BUTTON_SECONDARY);
     secondaryGesture->signal_released().connect(
@@ -182,6 +227,21 @@ TrayIconWidget::TrayIconWidget( std::string id)
     add_controller(secondaryGesture);
 }
 
+TrayIconWidget::~TrayIconWidget() {
+    if (aliveFlag) {
+        *aliveFlag = false;
+    }
+    if (menuPopover) {
+        menuPopover->popdown();
+        menuPopover->remove_action_group("dbusmenu");
+        menuPopover->set_menu_model({});
+        if (menuPopover->get_parent()) {
+            menuPopover->unparent();
+        }
+        menuPopover.reset();
+    }
+}
+
 void TrayIconWidget::update(const TrayService::Item &item) {
     hasRemoteMenu = item.menuAvailable;
     menuPopupPending = false;
@@ -194,7 +254,9 @@ void TrayIconWidget::update(const TrayService::Item &item) {
             menuPopover->insert_action_group(
                 "dbusmenu", Glib::RefPtr<Gio::ActionGroup>());
             menuPopover->set_menu_model({});
+            if (menuPopover->get_parent()) {
                 menuPopover->unparent();
+            }
             menuPopover.reset();
         }
     }
@@ -224,33 +286,78 @@ void TrayIconWidget::update(const TrayService::Item &item) {
 }
 
 void TrayIconWidget::on_primary_released(int /*n_press*/, double x, double y) {
-    // Intentionally no-op: some tray items (e.g. Spotify) misbehave when the
-    // host forwards primary clicks.
-    (void)x;
-    (void)y;
+    int32_t sendX = static_cast<int32_t>(std::lround(x));
+    int32_t sendY = static_cast<int32_t>(std::lround(y));
+
+    // Try the most accurate coordinates first; fall back to pointer and finally
+    // to -1/-1 so apps (e.g. Spotify) see a valid activate event on both
+    // Wayland and X11.
+    if (!try_get_global_click_coords(GTK_WIDGET(gobj()), x, y, sendX, sendY)) {
+        if (!try_get_global_pointer_coords(GTK_WIDGET(gobj()), sendX, sendY)) {
+            sendX = -1;
+            sendY = -1;
+        }
+    }
+
+    std::cerr << "[TrayIconWidget] Activate primary id=" << id << " x="
+              << sendX << " y=" << sendY << std::endl;
+    service.activate(id, sendX, sendY);
+}
+
+void TrayIconWidget::on_middle_released(int /*n_press*/, double x, double y) {
+    // Map middle click to the StatusNotifier SecondaryActivate event; some
+    // apps (e.g. media players) use this for alternate actions like toggling
+    // visibility.
+    int32_t sendX = static_cast<int32_t>(std::lround(x));
+    int32_t sendY = static_cast<int32_t>(std::lround(y));
+    if (!try_get_global_click_coords(GTK_WIDGET(gobj()), x, y, sendX, sendY)) {
+        if (!try_get_global_pointer_coords(GTK_WIDGET(gobj()), sendX, sendY)) {
+            sendX = -1;
+            sendY = -1;
+        }
+    }
+
+    std::cerr << "[TrayIconWidget] SecondaryActivate (middle) id=" << id
+              << " x=" << sendX << " y=" << sendY << std::endl;
+    service.secondaryActivate(id, sendX, sendY);
 }
 
 void TrayIconWidget::on_secondary_released(int /*n_press*/, double x,
                                            double y) {
+    // If we are not attached to a toplevel (e.g., window hidden), fall back to
+    // the item's own ContextMenu instead of trying to show a popover, which
+    // would crash without a mapped surface.
+    GtkWidget *selfWidget = GTK_WIDGET(gobj());
+    if (!gtk_widget_get_mapped(selfWidget) || !has_popup_surface(selfWidget)) {
+        std::cerr << "[TrayIconWidget] Secondary fallback ContextMenu (no surface) id="
+                  << id << std::endl;
+        service.contextMenu(id, -1, -1);
+        return;
+    }
+
     pendingX = x;
     pendingY = y;
 
-    // Prefer dbusmenu popover when available.
-    if (hasRemoteMenu) {
+    // Use dbusmenu popover when available and we have a mapped surface; else
+    // fall back to the item's ContextMenu.
+    if (hasRemoteMenu && has_popup_surface(selfWidget)) {
+        std::cerr << "[TrayIconWidget] Requesting dbusmenu for id=" << id
+                  << std::endl;
         menuPopupPending = true;
         if (menuRequestInFlight) {
             return;
         }
 
         menuRequestInFlight = true;
+        auto weak = std::weak_ptr<bool>(aliveFlag);
         service.request_menu_layout(
-            id, sigc::mem_fun(*this, &TrayIconWidget::on_menu_layout_ready));
-        return;
+            id, [weak, this](std::optional<TrayService::MenuNode> layout) {
+                if (auto locked = weak.lock()) {
+                    if (*locked) {
+                        on_menu_layout_ready(std::move(layout));
                     }
-
-    // No dbusmenu: defer to the item's own ContextMenu.
-    if (is_wayland_display(GTK_WIDGET(gobj()))) {
-        service.contextMenu(id, -1, -1);
+                }
+            });
         return;
     }
 
@@ -259,7 +366,15 @@ void TrayIconWidget::on_secondary_released(int /*n_press*/, double x,
     if (!try_get_global_click_coords(GTK_WIDGET(gobj()), x, y, sendX, sendY)) {
         (void)try_get_global_pointer_coords(GTK_WIDGET(gobj()), sendX, sendY);
     }
+    if (is_wayland_display(GTK_WIDGET(gobj()))) {
+        std::cerr << "[TrayIconWidget] ContextMenu wayland id=" << id
+                  << " x=-1 y=-1" << std::endl;
+        service.contextMenu(id, -1, -1);
+    } else {
+        std::cerr << "[TrayIconWidget] ContextMenu id=" << id << " x=" << sendX
+                  << " y=" << sendY << std::endl;
         service.contextMenu(id, sendX, sendY);
+    }
 }
 
 void TrayIconWidget::on_menu_layout_ready(
@@ -270,25 +385,22 @@ void TrayIconWidget::on_menu_layout_ready(
         return;
     }
 
-    if (!layoutOpt) {
+    GtkWidget *selfWidget = GTK_WIDGET(gobj());
+
+    if (!has_popup_surface(selfWidget)) {
         menuPopupPending = false;
-        // If dbusmenu layout fetch failed, fall back to ContextMenu.
-        if (is_wayland_display(GTK_WIDGET(gobj()))) {
-            service.contextMenu(id, -1, -1);
-        } else {
-            service.contextMenu(id, static_cast<int32_t>(std::lround(pendingX)),
-                                static_cast<int32_t>(std::lround(pendingY)));
-        }
         menuModel.reset();
         menuActions.reset();
-        if (menuPopover) {
-            menuPopover->remove_action_group("dbusmenu");
-            menuPopover->set_menu_model({});
+        return;
     }
+
+    if (!layoutOpt) {
+        menuPopupPending = false;
         return;
     }
 
     const auto &layout = *layoutOpt;
+    log_menu_tree(layout.children, 0);
     auto menu          = Gio::Menu::create();
     auto actions       = Gio::SimpleActionGroup::create();
 
@@ -322,6 +434,18 @@ void TrayIconWidget::on_menu_layout_ready(
     menuPopover->insert_action_group("dbusmenu", menuActions);
     menuPopover->set_menu_model(menuModel);
 
+    // Ensure popover is still parented to us and has a native/root before popup.
+    if (!menuPopover->get_parent()) {
+        menuPopover->set_parent(*this);
+    }
+
+    GtkWidget *popoverWidget = GTK_WIDGET(menuPopover->gobj());
+    if (!popoverWidget || !gtk_widget_get_root(popoverWidget) ||
+        !gtk_widget_get_native(popoverWidget) || !has_popup_surface(selfWidget)) {
+        menuPopupPending = false;
+        return;
+    }
+
     Gdk::Rectangle rect(static_cast<int>(pendingX), static_cast<int>(pendingY),
                         1, 1);
     menuPopover->set_pointing_to(rect);
@@ -378,10 +502,48 @@ void TrayIconWidget::populate_menu_items(
 
 void TrayIconWidget::on_menu_action(const Glib::VariantBase & /*parameter*/,
                                     int itemId) {
-    service.activate_menu_item(id, itemId);
+    // Pop down immediately so the popover doesn't outlive us if the item
+    // removes itself synchronously (e.g., "Exit"), which would otherwise lead
+    // to use-after-free.
     if (menuPopover) {
         menuPopover->popdown();
+        // Also detach to avoid double-unparent if the item disappears during
+        // the ensuing D-Bus call.
+        if (menuPopover->get_parent()) {
+            menuPopover->unparent();
         }
+    }
+
+    int32_t sendX = -1;
+    int32_t sendY = -1;
+    (void)try_get_pending_coords(sendX, sendY);
+
+    std::cerr << "[TrayIconWidget] Menu action id=" << this->id
+              << " item=" << itemId << " x=" << sendX << " y=" << sendY
+              << std::endl;
+
+    const uint32_t nowMs = static_cast<uint32_t>(g_get_monotonic_time() / 1000);
+    // Use button 1 for menu activation events; some dbusmenu handlers ignore
+    // secondary-button payloads for activate.
+    service.activate_menu_item(id, itemId, sendX, sendY, 1 /*button*/, nowMs);
+}
+
+bool TrayIconWidget::try_get_pending_coords(int32_t &outX, int32_t &outY) const {
+    outX = -1;
+    outY = -1;
+    int32_t sendX = static_cast<int32_t>(std::lround(pendingX));
+    int32_t sendY = static_cast<int32_t>(std::lround(pendingY));
+    if (!try_get_global_click_coords(GTK_WIDGET(gobj()), pendingX, pendingY,
+                                     sendX, sendY)) {
+        if (!try_get_global_pointer_coords(GTK_WIDGET(gobj()), sendX, sendY)) {
+            sendX = -1;
+            sendY = -1;
+        }
+    }
+
+    outX = sendX;
+    outY = sendY;
+    return (sendX != -1 || sendY != -1);
 }
 
 TrayWidget::TrayWidget()
@@ -444,7 +606,6 @@ void TrayWidget::on_item_removed(const std::string &id) {
     }
 
     remove(*it->second);
-    it->second->unparent();
     icons.erase(it);
 
     if (icons.empty()) {