Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support StatusNotifiers properly on all desktops #73

Closed
ntninja opened this issue Jan 25, 2015 · 17 comments
Closed

Support StatusNotifiers properly on all desktops #73

ntninja opened this issue Jan 25, 2015 · 17 comments

Comments

@ntninja
Copy link

ntninja commented Jan 25, 2015

In the file syncthing_gtk/statusicon.py, you are checking whether you are in "THE_HELL"/Unity desktop environment to determine whether gi.repository.AppIndicator or gtk.StatusIcon should be used for displaying the notification area icon.
Aside from the fact that the old system tray tends to produce visual glitches and is not supported on wayland anymore the general trends seems to be to drop it entirely: Unity led the way in 2010 by inventing AppIndicators and supporting them in Unity and disabling the system tray for most applications 2 years later. Since then several desktop environments have added support for appindicators (including at least KDE, MATE and XFCE) and with KDE 5, KDE has dropped support for the legacy system tray as well.
You could, of course add another check that also tries to use AppIndicators on KDE 5, but it would probably be a better idea to just always use the AppIndicator library when its available and only fall back to gtk.StatusIcon if its not (please remember that libappindicator falls back to GtkStatusIcon internally if the current desktop/GNOME does not provide support). According to that page it is also possible to use libappindicator in a way that it allows you to render your own custom system tray icon if the desktop does not support appindicators (but the code example is unfortunately gone).

Please consider making AppIndicators the default! If you really don't like the idea at all please at least consider adding KDE to list of desktop environment that are "allowed" to have appindicators...

(For now the workaround on KDE 5 is to start SyncThing-GTK like this: env XDG_CURRENT_DESKTOP=Unity syncthing-gtk)

@kozec
Copy link
Owner

kozec commented Jan 25, 2015

Problem with AppIndicator is that it's little limited in features - namely, it's missing 'clicked' signal, preventing me from implementing traditional "left click shows window, right click shows menu" behavior. So while I can use it as fallback on environment that doesn't support Freedesktop tray specification, I don't see reason why use reduced functionality as default.

What XDG_CURRENT_DESKTOP value is set under KDE5?

@kozec
Copy link
Owner

kozec commented Jan 25, 2015

By the way, MATE and XFCE doesn't support AppIndicator. AppIndicator falls back to Freedesktop specification outside of Unity.
And from reading that blog, it seems to be more like KDE decided to drop support for what everyone else uses and ***** poor users - I really look forward to see what will this do with Java :) This approach really invites dev to say "ok KDE, let's **** them" :D

But, of course, if there is dependable way how to check that KDE5 is running, I have no problem with adding such check.

@somasis
Copy link
Contributor

somasis commented Jan 26, 2015

@kozec That's not entirely true, there's indicator applets made for both MATE and Xfce; their usage is just not as needed as it could be right now.
Due to the fallback to GtkStatusIcon, most users don't really notice a difference unless they use AppIndicators in addition; I think supporting AppIndicator for all desktops would be a better idea.

It'd also allow for possibly more creative design, status-wise.

@kozec
Copy link
Owner

kozec commented Feb 1, 2015

Sorry, I probably let most important question lost in that wall of text of mine:

What XDG_CURRENT_DESKTOP value is set under KDE5?
I need dependable way how to tell that ST-GTK is running under KDE5 and not, for example, under KDE4 or Gnome.

@ntninja
Copy link
Author

ntninja commented Feb 5, 2015

On KDE 5 its XDG_CURRENT_DESKTOP=KDE (which is the same as on KDE 4), for KDE 5 specifically one has to also check for KDE_SESSION_VERSION=5.
However I just discovered another (more future-proof) way! Since you have indicated that you want to use systray by default and fall back to appindicators (when systray is not available) I started to wonder if its possible to probe for systray availability and (after examining the WINE source code) I found out that you actually can! The following is some example code that will print True or False depending on whether there is systray support available AT ALL. You will still need that extra check for Unity since they provide systray support but only show items that are on their hard-coded whitelist.

import Xlib.display as x11
display = x11.Display()
systray_atom = display.get_atom("_NET_SYSTEM_TRAY_S%d" % (display.get_default_screen()), True)
print(bool(display.get_selection_owner(systray_atom)))

This will print True or False depending on whether there is a system tray agent available or not. The procedure for deciding on the tray system to use would then be: Check whether the AppIndicator library is available (if no: use systray), check if desktop environment's XDG_CURRENT_DESKTOP is whitelisted (if yes: use appindicators), check whether there is a system tray selection owner available (if no: use appindicators), use systray.
Please tell me if you interested in taking this route. I would provide (and test!) a patch for this.

@kozec
Copy link
Owner

kozec commented Feb 5, 2015

Dunno, doing that would introduce dependency on python-xlib package, so blacklisting two DE's where system tray doesn't work sounds like better solution. I don't expect (or rather I do hope :D) that no other DE plans to break system tray support in future :)

kozec added a commit that referenced this issue Feb 5, 2015
@ntninja
Copy link
Author

ntninja commented Feb 12, 2015

Found something interesting: When an StatusNotifier application exposes the
Activate(Int32 x, Int32 y) method on the DBus interface org.kde.StatusNotifierItem (the interface that is being used for appindicator icons) it will be called whenever the user left-clicks on the icon in the system tray. This mechanism does not work on Unity, but it does work on KDE 4 and 5 (haven't tested anything else) and is used by transmission-qt amongst others to display the main window on left-click.

Now the bad news: The libappindicator3 library that you are using does not provide this method on DBus (and has no API to do so) and therefor makes it impossible to use to functionality. (It is not something Unity will support so it won't be implemented anytime soon...).

Would you accept a pull-request that implements the StatusNotifierItem on top of GDBus and ditches the AppIndicator library entirely? It would probably take ~2000 LOC...

@kozec
Copy link
Owner

kozec commented Feb 12, 2015

Would you accept a pull-request that implements the StatusNotifierItem on top of GDBus and ditches the AppIndicator library entirely? It would probably take ~2000 LOC...

Dunno, that's about 1/4 of entire codebase for fixing something that isn't broken :D
Is there any library that allows detecting left-click on appindicator that can be used on KDE5? Something that StatusIcon wrapper can use by default and fallback to AppIndicator if it's unavailable.

@ntninja
Copy link
Author

ntninja commented Feb 13, 2015

Dunno, that's about 1/4 of entire codebase for fixing something that isn't broken :D

Yes, it's kind of broken IMHO... syncthing-gtk should feel like "any other icon" and not "stand out".

Is there any library that allows detecting left-click on appindicator that can be used on KDE5?

Hmm... One could use the original KStatusNotifierItem API, there are no KDE 5 bindings of this yet, but the KDE 4 one's should work. Unfortunately using this API would load most of the KDE frameworks 4 libraries at startup.
Alternatively there is also support for this in PyQt5 since version 5.4 (some lower versions have been patched in several distros to support it through). It's less stuff to load but would not guarantee that you are achieving anything better than what GtkStatusIcon has to offer.
I also found this attempt to "add another layer" in Python but I guess it never was completed. It might be a good foundation if the important missing parts where added...

Some things I just found out about GtkStatusIcon in the meantime:

  • Its use has been deprecated in Gtk 3.14 and the GNOME guys recommend relying on notification popups instead. (Thanks GNOME for pushing your agenda yet another time using GTK...! :-[ )
  • The python-xlib stuff I posted above (for querying if the current desktop environment supports the classical system tray) can also be done directly using Gtk (without adding another dependency), look here

@kozec
Copy link
Owner

kozec commented Feb 13, 2015

Yes, it's kind of broken IMHO... syncthing-gtk should feel like "any other icon" and not "stand out".

And that's what it does :) It looks and works as any other, systray specification conforming icon. It's certainly bad when some DE decides to remove support for tray icons, but it's not bug in ST-GTK.

I'm all for workaround, if it's easy enough to do, but workaround sized as 1/4 of codebase is simply not maintainable. And if I understood correctly, current workaround should work in KDE5. Am I mistaken?

Anyway, using gtk_status_icon_is_embedded and trying to load & use AppIndicator if status icon is not embedded sounds like good idea. I'll check what will it break...

@TiZ-HugLife
Copy link

Actually, what he means by it standing out is this:
Mismatched tray icon

I'm not a fan of when things don't blend in the way they're supposed to. I use the Numix themes and ST-GTK's icon sticks out like a sore thumb. Numix has a process of getting new icons into its theme, but your application doesn't even support theming the tray icon so I can't use that. What I've done in the interim is replaced all of the tray icons--system wide--with Numix's UbuntuOne icons. Now it looks much better:
Fixed tray icons
(I fixed the wired network icon while I was at it; it was bugging me too.)

But this is not a proper solution. ST-GTK should support theming the tray icon.

EDIT: Fixed image links, oops.

@ntninja
Copy link
Author

ntninja commented Feb 16, 2015

Yes, current workaround kind-of works on KDE5, but there are several issues:

  • It doesn't show a name for this application when opening the "Status & Notification" list -- libappindicator3 exports the "Name" property so this probably is an easy fix
  • It always shows a broken icon instead of whatever si-*.png icon syncthing-gtk is trying to display -- probably a KDE bug, but I haven't checked yet (exported DBus values all look sane)
  • Main window does not open when left-clicking on the StatusNotifierItem -- see above

Also the idea of using GDBus from Python to do anything is completely illusionary because of this bug (yes there has been some patch for this for >3 years, but every year around fall they then decided they would like to fix it differently, another patch was proposed and they waited until fall...). Found that out the hard way :-(

Not sure if I'll look at KStatusNotifierItem or PyQt5 next...

@kozec
Copy link
Owner

kozec commented Feb 17, 2015

Not sure if I'll look at KStatusNotifierItem or PyQt5 next...

Bear in mind that entire application uses GTK and runs around gtk_main loop. I never tried it, but it combining QT and GTK in one process can turn out problematic.

Main window does not open when left-clicking on the StatusNotifierItem -- see above

But that's not bug, that's feature. Let me quote AppIndicator project site
Application indicators are more consistent – no more left and right-click inconsistency. Always left click to see the items.
Dunno how they managed to get it wrong, but they surely did :)

Actually, what he means by it standing out is this:

Support for icon theming is separate problem and can be done with Gtk.StatusIcon. I just never thought it's important enough (and probably failed to do it right in first place)

@kozec kozec removed the info needed label Feb 17, 2015
@ntninja
Copy link
Author

ntninja commented Feb 17, 2015

Bear in mind that entire application uses GTK and runs around gtk_main loop. I never tried it, but it combining QT and GTK in one process can turn out problematic.

According to the LXDE people (they migrated to Qt entirely) its not a big issue, but we'll see...

But that's not bug, that's feature. Let me quote AppIndicator project site...

I guess I got that wrong when opening this bug: AppIndicators are NOT StatusNotifiers! AppIndicators are a LIMITED SUBSET of StatusNotifiers for the Unity desktop. StatusNotifiers allow several things that Canonical doesn't want you to do... I didn't know that back when I opened this bug, but (as is documented here) I've since done a lot of research. The title should probably be changed to something like "Support StatusNotifiers properly on all desktops".

Support for icon theming is separate problem and can be done with Gtk.StatusIcon

Probably a good idea to implement that while we're at it ;-) I'm pretty confident it will solve the broken icon problem on KDE5 as well.

@TiZ-EX1 By "standing out" I actually meant something like this:
Screenshot

@kozec
Copy link
Owner

kozec commented Feb 18, 2015

Sooo, If I understand correctly, what KDE5 actually uses is third, completely non-standard specification for tray icons? :D Is there any python binding for that?

@ntninja
Copy link
Author

ntninja commented Feb 18, 2015

No, its not that bad. :-)
It's just that Unity rebranded a limited subset of what KDE's StatusNotifiers as AppIndicators and just ignores all other features (Passive/Active states, icon activation/left-clicking, application titles, tooltips, ...).

That means:

  • An application exporting the complete StatusNotifier interface on Unity will work but the user will not be able to use all the features provided by the application (i.e.: left-click) -> That is not a problem however: It's the user's choice whether he/she is content with his/her desktops features; it's not an app developers problem if something is not displayed when he supplies the correct information
  • An application using libappindicator3 on non-Unity however will also have the same amount of reduced functionality, but will still kind of work (see this bug) -> This is still a problem for the app developer: His/her app will lack features that the user is probably accustomed to by other apps that offer full support on that desktop environment, he might complain ;-)

I already have a working version of Syncthing-GTK that works well on KDE, but I haven't tested it on non-KDE (especially Unity) yet and the original gtk.StatusIcon/appindicator.AppIndicator code is currently hidden behind a wall of comments. :-)
I'll make a pull-request when its ready...

@kozec
Copy link
Owner

kozec commented Feb 18, 2015

It's the user's choice whether he/she is content with his/her desktops features; it's not an app developers problem if something is not displayed when he supplies the correct information

I just hope that you do realize that this equally applies to ST-GTK supplying correct icon using system tray protocol and KDE5 ignoring it :D

Anyway, I have nothing against pull request, as long as it doesn't break anything else.

@kozec kozec closed this as completed Feb 26, 2015
@ntninja ntninja changed the title AppIndicator isn't a Unity-only thing anymore Support StatusNotifiers properly on all desktops Aug 7, 2016
Repository owner locked and limited conversation to collaborators Nov 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants