ubuntu / gnome-shell-extension-appindicator

Adds KStatusNotifierItem support to the Shell
https://extensions.gnome.org/extension/615/appindicator-support/
GNU General Public License v2.0
1.18k stars 160 forks source link

Some apps need XDG_CURRENT_DESKTOP=Unity #74

Open jhasse opened 7 years ago

jhasse commented 7 years ago

I think the problem is that many app developers don't know that GNOME Shell also supports appindicator via this extension so they add checks like this one: https://github.com/haiwen/seafile/commit/bb8eacce1cf999a32185c344e0737c2842eb60de#diff-b25e62244af579d5a0330d134c6fcb28R60

Maybe there's some kind of workaround for this or a better method to find out if appindicators should be used.

JasonLG1979 commented 7 years ago

It's not your bug. They should be checking to see if the org.kde.StatusNotifierWatcher DBus interface is owned. Like so.

https://github.com/pithos/pithos/blob/master/pithos/plugins/notification_icon.py#L99-L102

jhasse commented 7 years ago

Or com.canonical.dbusmenu. Thanks for the link, I've added the label but will leave this open for now, as it's a pretty severe bug.

JasonLG1979 commented 7 years ago

Or com.canonical.dbusmenu. Thanks for the link, I've added the label but will leave this open for now, as it's a pretty severe bug.

org.kde.StatusNotifierWatcher is a safer bet. com.canonical.dbusmenu is specific to appindicator which is just an abstraction layer/extension of org.kde.StatusNotifierWatcher. It's possible to implement a system tray based on org.kde.StatusNotifierWatcher without com.canonical.dbusmenu.

jhasse commented 7 years ago

Update: "We are thus working to push a proper fix upstream, and will reach to those application developers to rebuild their applications to get the indicator showing up in the incoming artful release." https://didrocks.fr/2017/08/23/ubuntu-gnome-shell-in-artful-day-7/ :)

JasonLG1979 commented 7 years ago

To elaborate @TingPing recently rewrote our tray icon plugin. https://github.com/pithos/pithos/blob/master/pithos/plugins/notification_icon.py

The flow seems to be: flow

Apps would need to check with X to see if the DE has legacy tray support. Checking XDG_CURRENT_DESKTOP is just bad coding. It's the equivalent of a site sniffing the user agent of a browser.

jhasse commented 7 years ago

But doesn't libappindicator support falling back to a legacy tray icon? As long as you don't want to override the single-click behavior it should be enough to just rely on it.

JasonLG1979 commented 7 years ago

But doesn't libappindicator support falling back to a legacy tray icon? As long as you don't want to override the single-click behavior it should be enough to just rely on it.

On GNOME Shell at least, legacy tray fallback is broken in appindicator. The appindicator legacy icon causes weird bugs when it comes to hiding and showing windows. A "real" appindicator and the legacy Gtk.StatusIcon have no problem with it.

jhasse commented 7 years ago

Ah that's too bad. Hopefully this can be fixed in libappindicator though since Gtk.StatusIcon is deprecated.

JasonLG1979 commented 7 years ago

Ah that's too bad. Hopefully this can be fixed in libappindicator though since Gtk.StatusIcon is deprecated.

Somewhat off topic but what I'd like to see happen is standard DBus interfaces for common use cases. We already have that for media players with MPRIS. It separates info/functionality from implementation. An example is there could be a IM/Chat MPRIS like DBus spec that allowed chat apps to expose their info/functionality and it's up to the DE how to expose it to the user.

TingPing commented 7 years ago

Considering you can't even build libappindicator and libdbusmenu without patches I don't think either are maintained so I wouldn't expect the fallback support to ever be improved.

jhasse commented 6 years ago

Doesn't mean that they couldn't be maintained in the future.

@didrocks You seem to be the last committer to http://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk.16.10/files. Do you know more if Canonical plans to maintain those? If I may suggest: Move them to GitHub and merge the needed downstream patches TingPing is talking about ;)

didrocks commented 6 years ago

I think those indicators are used for Ubuntu Mate, so I would let flexidorion (would pin him IRL as we are at the ubuntu rally sprint) have a look at those as it may impact his flavor.

JasonLG1979 commented 6 years ago

@didrocks if Ubuntu is to include this extension in the default desktop install and put forth libappindicator and libdbusmenu as a replacement for tray icons as they have then it would most definitely be Ubuntu/Canonical's duty to at least fix them and not just push it off on Ubuntu Mate.

didrocks commented 6 years ago

Marco is looking and is going to fix the issues for appindicator for sure (and he's already on it, to fix for some apps), we won't touch indicator-* daemons though (that we don't change). However, we need to ensure we don't regress Mate, so, it's still valid to cooperate with them :)

JasonLG1979 commented 6 years ago

Cooperation is always good but I'm not talking about indicators. I'm talking about libappindicator and libdbusmenu. As @TingPing and I have said they are broken. If they are to be put forth as the new "tray icon" standard as you are doing by including this extension in the default Ubuntu install then they need fixed. Lets not make this another Ubuntu, one distro solution.

jbicha commented 6 years ago

@JasonLG1979 Debian MATE developer @sunweaver is also working on supporting indicators. Indicators are not just for Ubuntu.

TingPing commented 6 years ago

@jbicha I think that was the point, libappindicator, libindicator, and libdbusmenu barely build on other distros without patches, so the question was are they even maintained?

sunweaver commented 6 years ago

Hi

On Saturday, September 30, 2017, TingPing wrote:

@jbicha I think that was the point, libappindicator, libindicator, and libdbusmenu barely build on other distros without patches, so the question was are they even maintained?

Not on Launchpad so much anymore. I have forked most of them and made them build and run on Debian.

https://github.com/AyatanaIndicators

Currently, there is an issue with libdbusmenu due to rwcent changes in gtkdoc-check and an FTBFS on Debian GNU/Hurd due to usage of the non-portable PATH_MAX constant.

But we are getting there... All efforts have already been uploaded to Debian.

See https://sunweavers.net for more info.

Mike

Reply to this email directly, view it on GitHub, or mute the thread.

-- Sent from my Fairphone (running Sailfish OS)...

JasonLG1979 commented 6 years ago

I think we are conflating libappindicator/libdbusmenu and the actual Ubuntu indicator widgets. They are not the same. This extension is proof that you can use both libs without the Ubuntu indicators. My point is that that legacy tray icon support is broken and as @TingPing has pointed out they impossible to build on other distros without patches. This has nothing to do with the Ubuntu widgets.

TingPing commented 6 years ago

I have forked most of them

Ok but that doesn't help anybody, nobody ships these forks and these forks break API/ABI making them useless for me.

sunweaver commented 6 years ago

Hi,

On Saturday, September 30, 2017, TingPing wrote:

I have forked most of them Ok but that doesn't help anybody, nobody ships these forks and these forks break API/ABI making them useless for me. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread. These forks are shipped in Debian already, they should build on Fedora, too. Ubuntu probably switches over to them for 18.04.

They are maintained and about to support all Debian architectures soon.

Watch for announcement and watch my talk on DebConf 17. There I met @xnox and we sort of agreed on moving indicators upstream efforts to AyatanaIndicators as a joint effort.

Mike

-- Sent from my Fairphone (running Sailfish OS)...

TingPing commented 6 years ago

These forks are shipped in Debian already, they should build on Fedora, too. Ubuntu probably switches over to them for 18.04.

I don't understand why you broke API if it is going to replace it...

sunweaver commented 6 years ago

Hi

On Saturday, September 30, 2017, TingPing wrote:

These forks are shipped in Debian already, they should build on Fedora, too. Ubuntu probably switches over to them for 18.04. I don't understand why you broke API if it is going to replace it...

ABI nor API have been changed. Only change done is a namespace change, so that Ubuntu Indicators and Ayatana Indicators can be co-installed. This is important for a smooth transition within Ubuntu. My goal is to make Indicators to non-Ubuntu distros. This requires severe changes in the code. This has been done while keeping everything 99% compatible.

Also, more desktop support is currently added. Other desktops will be supported by native indicators soon, too.

-- Sent from my Fairphone (running Sailfish OS)...

TingPing commented 6 years ago

ABI nor API have been changed. Only change done is a namespace change, so that Ubuntu Indicators and Ayatana Indicators can be co-installed.

Namespace is part of API, how do you think Vala and GIR bindings work. Every language but C has a broken API. The pkgconfig name was also broken, which I would call part of the API.

ABI may not be broken but soname is broken, meaning it is not a replacement for binary applications.

TingPing commented 6 years ago

Anyway @didrocks if Canonical cares at all for these libraries that are a core part of the Ubuntu platform they need to either step up and maintain it or hand over the rights to somebody that will. A fork that breaks API/soname (and doesn't seem to understand that) is not acceptable as either a distributor or a developer.

JasonLG1979 commented 6 years ago

Also, more desktop support is currently added

If adding support for more desktops means changing your code you're doing it wrong...

They're just suppose to be interfaces, meaning the interface is suppose to be separate from implementation...

All you're doing is making things more complicated then they need to be...

All libappindicator and libdbusmenu are are abstraction layers on top of DBus interfaces. @TingPing at this point I think it would just be better to just write our own DBus proxies/interfaces...

didrocks commented 6 years ago

Again, as told, this bug was raised before the extension was shipped by default here. Ubuntu has always supports com.canonical.dbusmenu, so no API have changed compared to Unity itself or the original extension. org.kde.StatusNotifierWatcher have a slightly different API than the indicator one, hence different namespace, but this isn't new since 2011.

The issue I mentioned on the blog is that chromium (and so, every electron apps) have a hardcoded check for desktop name to match Unity. This is the part @azzar1 is working with upstream. Nothing for you guys to change in that regards compared to the last 6 years.

JasonLG1979 commented 6 years ago

Again, as told, this bug was raised before the extension was shipped by default here.

That doesn't magically make libappindicator/libdbusmenu any less broken...

Ubuntu has always supports com.canonical.dbusmenu, so no API have changed compared to Unity itself or the original extension

Tell that to the guy who's forking your stuff and changing the namespaces... The project that Ubuntu/Canonical seems to be blessing as a continuation of the project.

org.kde.StatusNotifierWatcher have a slightly different API than the indicator one, hence different namespace, but this isn't new since 2011.

I still don't think you guys understand... All of it is just basically an abstraction layer on top of org.kde.StatusNotifierWatcher. If the API is different it's because it's just internally translated. The OP is changing the appindicator namespaces...

No one gives a crap about the actual Ubuntu indicator widgets, again interface separate from implementation. I only care that appindicator actually works, ATM it's buggy and can't be built without patches. To put it bluntly, it's poor quality software. As @TingPing has already said, Canonical needs to step up and put some work in the code or they need to give up the rights to someone who will. Plain and simple.

TingPing commented 6 years ago

This discussion is entirely unrelated to this issue though so sorry for going off-topic.

JasonLG1979 commented 6 years ago

This discussion is entirely unrelated to this issue though so sorry for going off-topic.

At least it's getting a little attention from someone at Canonical. The only bug I ever filed at launchpad got zero attention...

I'd love to continue this conversation where someone will actually listen and do something about it...

jhasse commented 6 years ago

Chromium is thinking about using libappindicator so I would like to revive the discussion here:

@didrocks What do you think about moving the libappindicator to https://github.com/Ubuntu so that MATE and other downstream projects can add their patches as PRs there?

didrocks commented 6 years ago

I'm not upstream for appindicator, so I can't take that decision. I don't see why having it in launchpad prevents patch contribution to the current appindicator project though.

Maybe ask the upstream authors on launchpad + the unity community effort who are going to deal with it regularly?

jhasse commented 6 years ago

I don't see why having it in launchpad prevents patch contribution to the current appindicator project though.

Because the bugs / questions are disabled on the project: https://bugs.launchpad.net/libappindicator

Maybe ask the upstream authors on launchpad + the unity community effort who are going to deal with it regularly?

Judging from http://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk.16.10/changes/284?start_revid=284 you and @3v1n0 are the only ones who have committed something in the last years. Or do you mean https://launchpad.net/~pspmteam with "upstream authors"?

didrocks commented 6 years ago

It's not used outside ubuntu AFAIK (but maybe in Arch? I'm unsure), this is why the tracker is https://bugs.launchpad.net/ubuntu/+source/libappindicator (the ubuntu package). I think we can open the upstream bug tracker if needed (do you have any bug to report there?).

@3v1n0, I'll leave the decision up to you :)

jhasse commented 6 years ago

It's not used outside ubuntu AFAIK

Debian: https://packages.debian.org/stretch/libappindicator1 Fedora: https://apps.fedoraproject.org/packages/libappindicator openSUSE: https://software.opensuse.org/package/libappindicator

do you have any bug to report there?

The API should report, when no tray is available, and the fallback should map the secondary action on left click, not middle-click.

sunweaver commented 6 years ago

Hi.

On Do 01 Mär 2018 18:26:34 CET, Jan Niklas Hasse wrote:

It's not used outside ubuntu AFAIK

Debian: https://packages.debian.org/stretch/libappindicator1

The libappindicator package in Debian is unmaintained / orphaned.
Currently, there are efforts taken by me, Martin Wimpress and others
to make libappindicator available outside of Ubuntu under a new
upstream umbrella [1,2].

Fedora: https://apps.fedoraproject.org/packages/libappindicator openSUSE: https://software.opensuse.org/package/libappindicator

No clue about there status.

Mike

[1] https://github.com/AyatanaIndicators [2]
https://qa.debian.org/developer.php?login=pkg-ayatana-devel%40lists.alioth.debian.org

--

DAS-NETZWERKTEAM mike gabriel, herweg 7, 24357 fleckeby mobile: +49 (1520) 1976 148 landline: +49 (4354) 8390 139

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22 0782 9AF4 6B30 2577 1B31 mail: mike.gabriel@das-netzwerkteam.de, http://das-netzwerkteam.de

TingPing commented 6 years ago

[1] https://github.com/AyatanaIndicators

Their fork breaks API so isn't a drop in replacement and likely won't see wide adoption.

jhasse commented 6 years ago

No clue about there status.

I'm using the Fedora version and it works perfectly. Syncthing-GTK, Telegram Desktop, Dropbox are examples for apps that use it.

angystardust commented 6 years ago

@jhasse same happy user here on Fedora 27. But i'm afraid things are going to change as 3.28 will be GA :(

orschiro commented 6 years ago

@angystardust same here for Fedora 27.

But i'm afraid things are going to change as 3.28 will be GA

What do you mean?

angystardust commented 6 years ago

@orschiro I mean...gnome 3.28 is going to change many things (for example: the desktop icons will go away) and I'm afraid that this extension will no more work but I'm hope I'm wrong :)

jbicha commented 6 years ago

@angystardust This extension works fine with GNOME 3.28.

mrprobot commented 6 years ago

Is there any news from the app developers? I tested Slack and Seafile and they still have that issue, btw also Discord is affected. With TopIcons Plus and Unite Shell they are loading fine but aren't looking that great.

https://github.com/hardpixel/unite-shell https://github.com/phocean/TopIcons-plus

Edit: On the feedback site of Discord there is already a thread opened for this issue. Vote for it so it gets some attention. Should we file a support request too?

https://feedback.discordapp.com/forums/326712-discord-dream-land/suggestions/32935801-ubuntu-indicators

TingPing commented 6 years ago

@mrprobot The issue for Electron apps like Discord is discussed here: https://github.com/electron/electron/issues/10427

Status is a real solution needs API changes, nobody has stepped up to do any of the work.

jhasse commented 5 years ago

Status is a real solution needs API changes, nobody has stepped up to do any of the work.

No, Electron just needs to check the org.kde.StatusNotifierWatcher DBus interface, no API changes needed.

TingPing commented 5 years ago

No, Electron just needs to check the org.kde.StatusNotifierWatcher DBus interface, no API changes needed.

How does an Electron app know if a tray is available? It can't because Electron needs to grow new API for that.

Blindly assuming a tray exists is why we are in this mess in the first place.

colonelpanic8 commented 5 years ago

How does an Electron app know if a tray is available? It can't because Electron needs to grow new API for that.

@TingPing Nope, you're wrong, The IsStatusNotifierHostRegistered property and the StatusNotifierHostRegistered signal have EXACTLY this purpose.

See: http://www.notmart.org/misc/statusnotifieritem/statusnotifierwatcher.html

colonelpanic8 commented 5 years ago

This is honestly getting really frustrating. The problems that are being mentioned are already well solved by the StatusNotifierItem interface.

jhasse commented 5 years ago

@IvanMalison I think @TingPing meant an API for Electron apps, i.e. a high-level JavaScript/Electron API for Electron users, not an API for Electron.

BUT:

How does an Electron app know if a tray is available?

That's not what this issue is about. This issue is about Electron apps which currently always show a tray icon, won't display their tray icon with this extension on anything but Ubuntu (and Pantheon I think?).

That Electron apps always assume a tray is there, even though it might not, is a different issue. I think you should create a separate issue for that (wouldn't make any sense in this extension's repo).

TingPing commented 5 years ago

Your right my point is a bit off, after all this bug is being kept alive for years...

Nothing here is actionable because it has literally nothing to do with the extension.