Merge pull request #5164 from Alexays/fix/tray-taskbar-mpd

fix: tray accel-group crash (#5142), wlr/taskbar shell-exec of built-in actions (#3284), MPD connect-timeout freeze (#1186)

include/AModule.hpp

@@ -98,6 +98,13 @@ class AModule : public IModule {
   bool disable_on_sleep_{false};
   GObject* menu_ = nullptr;
 
+  // Maps a configured event name (e.g. "on-click-middle") to a built-in module
+  // action name. Populated from the `actions` config section, and by modules
+  // that interpret on-click* config values as internal actions (e.g.
+  // wlr/taskbar). Entries here are dispatched through doAction() instead of
+  // being run as shell commands.
+  std::map<std::string, std::string> eventActionMap_;
+
  private:
   bool handleUserEvent(GdkEventButton* const& ev);
   const bool isTooltip;
@@ -106,7 +113,6 @@ class AModule : public IModule {
   gdouble distance_scrolled_y_;
   gdouble distance_scrolled_x_;
   sigc::connection cursor_timeout_conn_;
-  std::map<std::string, std::string> eventActionMap_;
   static const inline std::map<std::pair<uint, GdkEventType>, std::string> eventMap_{
       {std::make_pair(1, GdkEventType::GDK_BUTTON_PRESS), "on-click"},
       {std::make_pair(1, GdkEventType::GDK_BUTTON_RELEASE), "on-click-release"},

src/AModule.cpp

@@ -222,10 +222,18 @@ bool AModule::handleUserEvent(GdkEventButton* const& e) {
   }
   // Second call user scripts
   if (!format.empty()) {
-    if (config_[format].isString())
+    // If the configured value for this event is a recognized built-in module
-      format = config_[format].asString();
+    // action (registered in eventActionMap_), it has already been dispatched
-    else
+    // via doAction() above / handled by the module itself. Don't additionally
+    // run it as a shell command (issue #3284). Any other value is still treated
+    // as a user shell command.
+    const auto actionIt = eventActionMap_.find(format);
+    const bool isModuleAction = actionIt != eventActionMap_.cend() && config_[format].isString() &&
+                                config_[format].asString() == actionIt->second;
+    if (isModuleAction || !config_[format].isString())
       format.clear();
+    else
+      format = config_[format].asString();
   }
   if (!format.empty()) {
     const int width = gdk_window_get_width(e->window);

src/modules/mpd/mpd.cpp

@@ -294,8 +294,19 @@ void waybar::modules::MPD::tryConnect() {
     return;
   }
 
-  connection_ =
+  // tryConnect() runs on the GTK main thread (via Glib::signal_timeout), so a
-      detail::unique_connection(mpd_connection_new(server_, port_, timeout_), &mpd_connection_free);
+  // blocking connect freezes the whole bar. Bound the connect attempt to a
+  // short timeout so an unreachable MPD server fails fast instead of hanging
+  // the loop for the full user-facing `timeout_` (up to 30s by default, #1186).
+  // The third argument to mpd_connection_new() is also the default command
+  // read timeout, so restore `timeout_` once connected to avoid shortening
+  // reads for slow-but-alive servers.
+  static constexpr unsigned kConnectTimeoutMs = 2'000;
+  unsigned connect_timeout =
+      (timeout_ != 0 && timeout_ < kConnectTimeoutMs) ? timeout_ : kConnectTimeoutMs;
+
+  connection_ = detail::unique_connection(mpd_connection_new(server_, port_, connect_timeout),
+                                          &mpd_connection_free);
 
   if (connection_ == nullptr) {
     spdlog::error("{}: Failed to connect to MPD", module_name_);
@@ -303,6 +314,11 @@ void waybar::modules::MPD::tryConnect() {
     return;
   }
 
+  // Restore the user-configured timeout for subsequent command reads.
+  if (timeout_ != 0) {
+    mpd_connection_set_timeout(connection_.get(), timeout_);
+  }
+
   try {
     checkErrors(connection_.get());
     spdlog::debug("{}: Connected to MPD", module_name_);

src/modules/sni/item.cpp

@@ -577,6 +577,15 @@ void Item::makeMenu() {
     if (dbus_menu != nullptr) {
       g_object_ref_sink(G_OBJECT(dbus_menu));
       g_object_weak_ref(G_OBJECT(dbus_menu), (GWeakNotify)onMenuDestroyed, this);
+      // Provide an accel group to the dbusmenu client. Without one, items that export menu
+      // accelerators (e.g. Mattermost) trigger gtk_widget_set_accel_path() with a NULL accel group,
+      // which raises a Gtk-CRITICAL and corrupts menu state (or aborts under fatal-criticals).
+      DbusmenuGtkClient* client = dbusmenu_gtkmenu_get_client(DBUSMENU_GTKMENU(dbus_menu));
+      if (client != nullptr) {
+        GtkAccelGroup* accel_group = gtk_accel_group_new();
+        dbusmenu_gtkclient_set_accel_group(client, accel_group);
+        g_object_unref(accel_group);
+      }
       gtk_menu = Glib::wrap(GTK_MENU(dbus_menu));
       gtk_menu->attach_to_widget(event_box);
     }

src/modules/wlr/taskbar.cpp

@@ -696,6 +696,21 @@ Taskbar::Taskbar(const std::string& id, const waybar::Bar& bar, const Json::Valu
   box_.get_style_context()->add_class("empty");
   event_box_.add(box_);
 
+  // wlr/taskbar interprets on-click* config values as built-in actions, handled
+  // per-task in Task::handle_clicked. Register the recognized action names so
+  // AModule dispatches them via doAction() instead of also running them as shell
+  // commands (issue #3284). Values that are not built-in actions are left alone
+  // and still run as user shell commands.
+  const auto is_builtin_action = [](const std::string& v) {
+    return v == "activate" || v == "minimize" || v == "minimize-raise" || v == "maximize" ||
+           v == "fullscreen" || v == "close";
+  };
+  for (const auto* event : {"on-click", "on-click-middle", "on-click-right"}) {
+    if (config_[event].isString() && is_builtin_action(config_[event].asString())) {
+      eventActionMap_.insert({event, config_[event].asString()});
+    }
+  }
+
   // Make task buttons distribute evenly across the available width.
   if (config_["homogeneous"].isBool() && config_["homogeneous"].asBool()) {
     box_.set_homogeneous(true);