Add support for Activate method of SNI #71

DarthGandalf · 2023-08-11T22:34:35Z

No description provided.

DarthGandalf · 2023-08-12T09:13:08Z

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 · 2023-08-24T21:29:39Z

We wrote #4 (comment).

We've forked libayatana-appindicator and #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 · 2023-08-24T22:15:00Z

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 · 2023-08-24T22:47:55Z

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 · 2023-08-24T22:59:32Z

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

Betterbird · 2023-08-25T01:01:26Z

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 · 2023-08-25T01:05:16Z

Which DE?

I was testing it on KDE

Betterbird · 2023-08-25T08:02:29Z

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 · 2023-08-25T18:47:12Z

$ ./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 · 2023-08-25T19:25:40Z

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 · 2023-08-25T19:26:51Z

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

yep!

Betterbird · 2023-08-25T20:04:44Z

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

DarthGandalf · 2023-08-25T20:16:03Z

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

Betterbird · 2023-08-25T20:29:56Z

Maybe they send something else?

Betterbird · 2023-08-26T17:06:34Z

src/app-indicator.h

@@ -221,13 +227,17 @@ struct _AppIndicatorClass {
    void (*unfallback)              (AppIndicator * indicator,
                                     GtkStatusIcon * status_icon);

+    void (* activate_event)          (AppIndicator * indicator,


Suggest to move this up after scroll_event.

Betterbird · 2023-08-26T17:07:07Z

src/app-indicator.c

+	 * @arg1: X
+	 * @arg2: Y
+     *
+     * Signaled when the #AppIndicator receives a scroll event.


Copy/paste error in the comment: you mean "activate".

Betterbird · 2023-08-26T21:42:59Z

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 · 2023-08-29T07:43:51Z

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 · 2023-09-03T17:24:24Z

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 · 2023-09-03T19:35:59Z

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 · 2023-09-04T09:58:39Z

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 · 2023-09-04T20:06:21Z

@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 · 2023-09-04T20:19:50Z

Do you have some other patches applied before this?

Yes, I committed on your branch.

DarthGandalf · 2023-09-04T20:25:14Z

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

Tamaranch · 2023-09-04T20:29:51Z

app-indicator.c.txt

DarthGandalf · 2023-09-04T20:48:31Z

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 · 2023-09-04T20:58:36Z

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 · 2023-09-04T21:20:38Z

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

DarthGandalf · 2023-09-04T21:21:38Z

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

Betterbird · 2023-09-04T23:01:58Z

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 · 2023-09-05T15:11:39Z

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 · 2023-09-09T07:53:17Z

I got Betterbird to restore from tray on MATE: Betterbird/thunderbird-patches#111

Narthorn · 2023-09-30T02:51:03Z

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 · 2024-01-03T05:27:08Z

Is this PR currently available?

clopez · 2024-05-02T18:13:12Z

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 · 2024-05-17T09:13:52Z

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

sunweaver · 2024-05-18T19:29:14Z

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

sunweaver · 2024-05-18T19:30:01Z

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 · 2024-05-18T19:39:42Z

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 #17 did. We have them rebased, too.

DarthGandalf · 2024-05-19T22:27:26Z

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

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

qwertychouskie · 2024-10-03T01:14:52Z

@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 · 2024-10-03T09:01:33Z

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.

Add support for Activate method of SNI

20591e6

This was referenced Aug 18, 2023

Multiple tray icons of shutter appeared on Ubuntu 22.04.2 LTS. shutter-project/shutter#602

Open

handle click events and modifiers #4

Open

Betterbird mentioned this pull request Aug 25, 2023

Support "minimise to tray" on Linux Betterbird/thunderbird-patches#111

Closed

Betterbird reviewed Aug 26, 2023

View reviewed changes

Betterbird mentioned this pull request Sep 12, 2023

Clicking the tray icon does not raise the window on Linux Betterbird/thunderbird-patches#202

Closed

Betterbird mentioned this pull request Sep 30, 2023

Activation via system tray click for older systrays like trayer Betterbird/thunderbird-patches#208

Closed

Add support for Activate method of SNI #71

Are you sure you want to change the base?

Add support for Activate method of SNI #71

Conversation

DarthGandalf commented Aug 11, 2023

DarthGandalf commented Aug 12, 2023

Betterbird commented Aug 24, 2023

DarthGandalf commented Aug 24, 2023

Betterbird commented Aug 24, 2023

DarthGandalf commented Aug 24, 2023

Betterbird commented Aug 25, 2023

DarthGandalf commented Aug 25, 2023

Betterbird commented Aug 25, 2023

DarthGandalf commented Aug 25, 2023

Betterbird commented Aug 25, 2023

DarthGandalf commented Aug 25, 2023

Betterbird commented Aug 25, 2023

DarthGandalf commented Aug 25, 2023

Betterbird commented Aug 25, 2023

Betterbird Aug 26, 2023

Choose a reason for hiding this comment

Betterbird Aug 26, 2023

Choose a reason for hiding this comment

Betterbird commented Aug 26, 2023

Betterbird commented Aug 29, 2023

Tamaranch commented Sep 3, 2023 • edited Loading

Betterbird commented Sep 3, 2023

Tamaranch commented Sep 4, 2023 • edited Loading

DarthGandalf commented Sep 4, 2023 • edited Loading

Tamaranch commented Sep 4, 2023

DarthGandalf commented Sep 4, 2023

Tamaranch commented Sep 4, 2023

DarthGandalf commented Sep 4, 2023

Tamaranch commented Sep 4, 2023

DarthGandalf commented Sep 4, 2023

DarthGandalf commented Sep 4, 2023

Betterbird commented Sep 4, 2023 • edited Loading

dark-penguin commented Sep 5, 2023

dark-penguin commented Sep 9, 2023

Narthorn commented Sep 30, 2023

jinzhongjia commented Jan 3, 2024

clopez commented May 2, 2024

joakim-tjernlund commented May 17, 2024

sunweaver commented May 18, 2024

sunweaver commented May 18, 2024

Betterbird commented May 18, 2024

DarthGandalf commented May 19, 2024

qwertychouskie commented Oct 3, 2024

DarthGandalf commented Oct 3, 2024

Tamaranch commented Sep 3, 2023 •

edited

Loading

Tamaranch commented Sep 4, 2023 •

edited

Loading

DarthGandalf commented Sep 4, 2023 •

edited

Loading

Betterbird commented Sep 4, 2023 •

edited

Loading