Add support for Activate method of SNI

[DarthGandalf]

Hm, this breaks left click for programs which don't connect to the activate-event signal... Looks like we'd need either separate dbus xml (depending on some parameter to constructor?), or somehow detect whether anything is connected to that signal, and if not, treat Activate as SecondaryActivate.

WDYT?

[Betterbird]

We wrote https://github.com/AyatanaIndicators/libayatana-appindicator/issues/4#issuecomment-1383906148.

We've forked libayatana-appindicator and https://github.com/AyatanaIndicators/libayatana-appindicator/pull/17 into Betterbird: https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/12-feature-linux-systray.patch https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/12-feature-linux-systray-tooltip.patch

We'd fork this PR too with no hesitation, but it's not clear how to use it. What we need is to register a callback which will be called upon activation. How can this be done? We have provision here already: https://github.com/Betterbird/thunderbird-patches/blob/4cad56fd72381bf55ea8e67f69cbcc7a5670f629/102/features/13-feature-linux-minimise-to-tray.patch#L270

Somehow register a "listener" for "activate-event". How would that be done? Could you provide a few lines of sample code. Thanks in advance!

[DarthGandalf]

Could you provide a few lines of sample code.

my $tray_libappindicator = AppIndicator::Indicator->new("Shutter", "shutter-panel", 'application-status');
$tray_libappindicator->set_menu($tray_menu);
$tray_libappindicator->set_status('active');
$tray_libappindicator->signal_connect(
    'activate-event' => sub {
        ...
    },
    $tray_libappindicator
) ;

[Betterbird]

Interesting, thanks.

Our code is in C: Here's where we create it: https://github.com/Betterbird/thunderbird-patches/blob/4cad56fd72381bf55ea8e67f69cbcc7a5670f629/115/features/12-feature-linux-systray-betterbird.patch#L356 ... set the status: https://github.com/Betterbird/thunderbird-patches/blob/4cad56fd72381bf55ea8e67f69cbcc7a5670f629/115/features/12-feature-linux-systray-betterbird.patch#L370

So which see function would be use for signal_connect(), I can't see anything like this in the library: https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/12-feature-linux-systray.patch

[DarthGandalf]

See g_signal_connect() from https://docs.gtk.org/gobject/tutorial.html#how-to-create-and-use-signals

[Betterbird]

After applying your changes, we added this to our test program:

static void activated (AppIndicator * self, gint x, gint y, gpointer data) {
  fprintf(stderr, "*** Activated!!\n");
}

static gboolean change_icon0(AppIndicator *ci) {
  g_signal_connect(ci, APP_INDICATOR_SIGNAL_ACTIVATE_EVENT, G_CALLBACK(activated), NULL);
  if (icon == 0) {
    fprintf(stderr, "Setting icon to %s and title to '%s 1'\n", icon_path0, TEXT_ICON_TITLE);

Sadly the activated() function doesn't get called.

A bit of debugging showed that bus_method_call() is never called with method "Activate", hence the signal is not emitted.

What are we missing?

[DarthGandalf]

Which DE?

I was testing it on KDE

[Betterbird]

I was afraid you were asking that. Mint/Xfce. We already know that libayatana-appindicator gives different results on different destops 😢.

Attached the test program. We added your patch manually, so the formatting is off (like in the original). Maybe we made a mistake applying your changes. make && ./betterbird-systray-icon. Thanks for your help.

appindicator.zip

[DarthGandalf]

$ ./betterbird-systray-icon
Setting icon to /var/tmp/ayata/appindicator/default22.png and title to 'Test Icon Title 1'
=== Method Activate
*** Activated!!
=== Method Activate
*** Activated!!
=== Method Activate
*** Activated!!
=== Method Activate
*** Activated!!

Your code works for me

[Betterbird]

Thanks for testing. I'd say that your code works for you 😉. We've just added a few lines to the test program after applying your changes to libayatana-appindicator. KDE is known to have the best "system tray" support, so it's not a surprise that it works there but not on Xfce. As we wrote bus_method_call() isn't activated. So you just clicked on the icon with the left mouse button? Or is there another way to "activate"?

Any chance to investigate what's going on other desktops and to make it work there too? Gnome doesn't even support the tooltips, so that's not worth supporting.

[DarthGandalf]

So you just clicked on the icon with the left mouse button?

yep!

[Betterbird]

OK thanks, and getting it to work on other desktops?

[DarthGandalf]

If other desktops refuse to send the activate signal, there's not much we can do?

[Betterbird]

Maybe they send something else?

[Betterbird]

OK, just for reference, we've added this to Betterbird, we have some people who want to try it on KDE: https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/13-feature-linux-systray-activate.patch

[Betterbird]

We did a survey of which desktops work and which don't. So working are KDE and Gnome with extension "AppIndicator and KStatusNotifierItem Support". On Gnome the activation appears to be a double-click. Not working: Cinnamon, Mate, Xfce.

[Tamaranch]

Not working: Cinnamon, Mate, Xfce.

Hi, I'm the maintainer of xfce4-panel. I stumble here because I may use libayatana-appindicator in Xfce to use status notifiers instead of GtkStatusIcon for Wayland support. And I would need Activate to be supported :)

This PR isn't enough to make this signal work on Xfce because libayatana-appindicator also lacks support for the ItemIsMenu property. It's set to TRUE by default in xfce4-panel, because historically it's not supported in libappindicator: https://gitlab.xfce.org/xfce/xfce4-panel/-/blob/2c2ce90d52bbcea7381250101edfd16db19a3ae0/plugins/systray/sn-item.c#L284

By adding this patch to the PR, it works correctly (sorry, apparently it doesn't want to attach the patch directly, so I had to make an archive...): 0001-Add-support-for-ItemIsMenu-property-of-SNI.patch.tar.gz

Maybe it's the same problem with the other DEs?

[Betterbird]

Thanks for the patch, you can add it as text file. I'll add it to Betterbird and see how it goes, I was using Xfce before having to try all those other desktops. So it would be great to get it going there, too. Currently afk, I'd let you know mid next week.

0001-Add-support-for-ItemIsMenu-property-of-SNI.patch.txt

[Tamaranch]

Hm, this breaks left click for programs which don't connect to the activate-event signal... Looks like we'd need either separate dbus xml (depending on some parameter to constructor?), or somehow detect whether anything is connected to that signal, and if not, treat Activate as SecondaryActivate.

@DarthGandalf Is this fixed by applying the patch above and setting item-is-menu to TRUE?

[DarthGandalf]

@Tamaranch I can't apply it. Do you have some other patches applied before this?

I'm using 0.5.92 as the base

patching file src/app-indicator.c
Hunk #2 FAILED at 145.
Hunk #3 FAILED at 163.
Hunk #4 succeeded at 498 (offset -19 lines).
Hunk #5 succeeded at 675 (offset -35 lines).
Hunk #6 succeeded at 1046 with fuzz 1 (offset -40 lines).
Hunk #7 succeeded at 1136 with fuzz 1 (offset -44 lines).
Hunk #8 succeeded at 1282 (offset -47 lines).
2 out of 8 hunks FAILED -- saving rejects to file src/app-indicator.c.rej
patching file src/notification-item.xml

[Tamaranch]

Do you have some other patches applied before this?

Yes, I committed on your branch.

[DarthGandalf]

Can you send the whole src/app-indicator.c ?

[Tamaranch]

app-indicator.c.txt

[DarthGandalf]

Yes, sorry, I somehow was testing my patch on top of 0.5.92 directly, without all changes which happened since that, and those changes conflicted with your patch.

Adding $tray_libappindicator->set('item-is-menu', FALSE); (nor TRUE) doesn't do anything for me - if I don't add the handler to activate-event, left click does nothing now even with your patch

[Tamaranch]

Well it probably depends on the implementation, but setting item-is-menu to TRUE should indicate to ignore Activate in some way according to the specification. In xfce4-panel, this switches the menu display to left-click instead of activate.

[DarthGandalf]

Apparently menu is a separate property, maybe it needs to be set to something for ItemIsMenu to work?

[DarthGandalf]

I can't find where the source of KDE's systray is, to understand what exactly it does

[Betterbird]

We added @Tamaranch'es patch to our source and in the test program a click now activates: https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/13-feature-linux-systray-activate-xfce.patch We haven't tested Mate and Cinnamon yet. UPDATE: Mate and Cinnamon don't work. So in total KDE, Gnome (double-click) and Xfce work.

[dark-penguin]

If other desktops refuse to send the activate signal, there's not much we can do?

They do - I'm on MATE, and any other tray application does handle left-clicks (and I use a lot of them). Including my own PyQt5 programs. So MATE definitely does send something - the question is, how to others catch it.

(I can do some testing - I'm very much after getting it to work on MATE, and I'm good with testing, but I'm not familiar at all with C and GTK...)

[dark-penguin]

I got Betterbird to restore from tray on MATE: https://github.com/Betterbird/thunderbird-patches/issues/111

[Narthorn]

Should the fallback GtkStatusIcon also emit that activate event? eg:

 static void
 status_icon_activate (GtkStatusIcon * icon, gpointer data)
 {
+    AppIndicator *app = APP_INDICATOR(data);
+    g_signal_emit(app, signals[ACTIVATE_EVENT], 0, 0, 0);
+
     GtkMenu * menu = app_indicator_get_menu(APP_INDICATOR(data));
     if (menu == NULL)
         return;

(x,y coords are 0, since I don't think the native GTK activate event provides those).

This allows Betterbird's tray icon to work with older systrays like trayer.

[jinzhongjia]

Is this PR currently available?

[clopez]

Thanks for this patches!

I was able to make caffeine-ng activate/deactivate with the left click. See: https://codeberg.org/WhyNotHugo/caffeine-ng/issues/99#issuecomment-1802302

I had to apply the patch from this PR but also the patch from @Tamaranch because I'm a XFCE user and without this last patch it was not emitting the signal

It would be nice to have this two patches merged

[joakim-tjernlund]

I guess this has stalled? Would be great to adjust this PR so it works on all DEs

[sunweaver]

@DarthGandalf Are you still available for fine-polishing this PR?

[sunweaver]

Thanks for this patches!

I was able to make caffeine-ng activate/deactivate with the left click. See: https://codeberg.org/WhyNotHugo/caffeine-ng/issues/99#issuecomment-1802302

I had to apply the patch from this PR but also the patch from @Tamaranch because I'm a XFCE user and without this last patch it was not emitting the signal

It would be nice to have this two patches merged

@tari01 Could you take a look at the above?

[Betterbird]

You can find the rebased patches here: https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/13-feature-linux-systray-activate.patch https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/13-feature-linux-systray-activate-xfce.patch https://github.com/Betterbird/thunderbird-patches/blob/main/115/features/13-feature-linux-systray-activate-mate.patch Or maybe they didn't need rebasing, and only the ones for https://github.com/AyatanaIndicators/libayatana-appindicator/pull/17 did. We have them rebased, too.

[DarthGandalf]

@DarthGandalf Are you still available for fine-polishing this PR?

Likely I will be in a couple months, unlikely before that.

[qwertychouskie]

@DarthGandalf Are you still available for fine-polishing this PR?

Likely I will be in a couple months, unlikely before that.

Any chance you're able to look at this now?

[DarthGandalf]

Considering that #82 has appeared, probably it doesn't make sense anymore to do this in the previous codebase.

Another concern is back compatibility with old clients of this library, which don't expect a separate Activate signal - at least my patch by itself was breaking those, and I couldn't make the item-is-menu to work back then. Maybe that patch actually solves it though, I'd need to test it again.