After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 776278 - status-notifier: activation loop on toggle items
status-notifier: activation loop on toggle items
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Panel Maintainers
Panel Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-12-19 14:50 UTC by Colomban Wendling
Modified: 2016-12-20 15:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
status-notifier: avoid activation loop on toggle items (2.79 KB, patch)
2016-12-19 14:50 UTC, Colomban Wendling
none Details | Review
status-notifier: avoid activation loop on toggle items (2.67 KB, patch)
2016-12-20 13:44 UTC, Colomban Wendling
committed Details | Review

Description Colomban Wendling 2016-12-19 14:50:13 UTC
Created attachment 342220 [details] [review]
status-notifier: avoid activation loop on toggle items

Currently there is no guard against sending the activate signal over DBus in response to updating the item locally.  This is a problem for check and radio menu items that emit the activate signal locally then their state toggles, which leads to sending the even over to the application.  In return, the application will see a toggling event and notify the SNI again -- infinite loop.

This is visible e.g. in Mumble when activating either of the toggle items, which leads to endless toggling.

I'm not too happy with the implementation here, but it's the most straightforward one I thought of that doesn't add code duplication and doesn't require a bunch of indirection just for that.
Comment 1 Alberts Muktupāvels 2016-12-20 12:22:12 UTC
Review of attachment 342220 [details] [review]:

::: modules/external/status-notifier/sn-dbus-menu-item.c
@@ +371,3 @@
 
+              if (item->activate_handler != 0)
+                g_signal_handler_block (item->item, item->activate_handler);

Don't use if, it looks like signal id can be 0 only if we use invalid signal name with g_signal_connect.

::: modules/external/status-notifier/sn-dbus-menu-item.h
@@ +47,3 @@
   GtkMenu     *submenu;
+
+  gulong       activate_handler;

Please use activate_id.

::: modules/external/status-notifier/sn-dbus-menu.c
@@ +102,3 @@
       g_object_set_data (G_OBJECT (item->item), "item-id", GUINT_TO_POINTER (id));
       gtk_menu_shell_append (GTK_MENU_SHELL (gtk_menu), item->item);
+      item->activate_handler = g_signal_connect (item->item, "activate",

Please add empty line above this.
Comment 2 Colomban Wendling 2016-12-20 13:44:44 UTC
Created attachment 342261 [details] [review]
status-notifier: avoid activation loop on toggle items

Updated patch following review
Comment 3 Alberts Muktupāvels 2016-12-20 15:04:06 UTC
Thanks!