From 0e613141daddbaa9aed51c043c8a913dc61c24f8 Mon Sep 17 00:00:00 2001 From: Arif Hasanic Date: Sat, 3 Jan 2026 22:55:02 +0100 Subject: [PATCH] quick commit --- include/services/tray.hpp | 4 +- include/widgets/tray.hpp | 5 + src/services/tray.cpp | 37 +++++-- src/widgets/tray.cpp | 223 ++++++++++++++++++++++++++++++++------ 4 files changed, 227 insertions(+), 42 deletions(-) diff --git a/include/services/tray.hpp b/include/services/tray.hpp index 47f4ee0..60cdd9a 100644 --- a/include/services/tray.hpp +++ b/include/services/tray.hpp @@ -52,7 +52,9 @@ class TrayService { }; using MenuLayoutCallback = sigc::slot)>; 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 &signal_item_added(); sigc::signal &signal_item_removed(); diff --git a/include/widgets/tray.hpp b/include/widgets/tray.hpp index c182493..08cd265 100644 --- a/include/widgets/tray.hpp +++ b/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 primaryGesture; + Glib::RefPtr middleGesture; Glib::RefPtr secondaryGesture; Glib::RefPtr menuPopover; Glib::RefPtr menuActions; @@ -40,10 +42,12 @@ class TrayIconWidget : public Button { bool menuPopupPending = false; bool menuRequestInFlight = false; bool hasRemoteMenu = false; + std::shared_ptr 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 layout); void @@ -51,6 +55,7 @@ class TrayIconWidget : public Button { const Glib::RefPtr &menu, const Glib::RefPtr &actions); void on_menu_action(const Glib::VariantBase ¶meter, int itemId); + bool try_get_pending_coords(int32_t &outX, int32_t &outY) const; }; class TrayWidget : public Gtk::Box { diff --git a/src/services/tray.cpp b/src/services/tray.cpp index dc9e7be..95b8509 100644 --- a/src/services/tray.cpp +++ b/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(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(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(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) + ")"; diff --git a/src/widgets/tray.cpp b/src/widgets/tray.cpp index ff85457..e1b0da9 100644 --- a/src/widgets/tray.cpp +++ b/src/widgets/tray.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include "components/base/button.hpp" namespace { @@ -140,11 +141,49 @@ bool try_get_global_pointer_coords(GtkWidget *widget, int32_t &outX, outY = static_cast(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 &nodes, + int depth = 0) { + const std::string indent(static_cast(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(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()); menuPopover->set_menu_model({}); - menuPopover->unparent(); + 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(std::lround(x)); + int32_t sendY = static_cast(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(std::lround(x)); + int32_t sendY = static_cast(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) { - pendingX = x; - pendingY = 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; + } - // Prefer dbusmenu popover when available. - if (hasRemoteMenu) { + pendingX = x; + pendingY = y; + + // 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(aliveFlag); service.request_menu_layout( - id, sigc::mem_fun(*this, &TrayIconWidget::on_menu_layout_ready)); - return; - } - - // No dbusmenu: defer to the item's own ContextMenu. - if (is_wayland_display(GTK_WIDGET(gobj()))) { - service.contextMenu(id, -1, -1); + id, [weak, this](std::optional layout) { + if (auto locked = weak.lock()) { + if (*locked) { + on_menu_layout_ready(std::move(layout)); + } + } + }); 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); } - service.contextMenu(id, 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(std::lround(pendingX)), - static_cast(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(pendingX), static_cast(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(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(std::lround(pendingX)); + int32_t sendY = static_cast(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()) {