mate-desktop / mate-panel

MATE panel
https://mate-desktop.org
GNU General Public License v2.0
184 stars 118 forks source link

Telegram (>= 1.2) has 1 pixel wide systray icon on Mate (1.18) #695

Closed Maxxie closed 6 years ago

Maxxie commented 6 years ago

Expected behaviour

Telegram's systray icon should be visible.

Actual behaviour

Since version 1.2, Telegram's systray icon stopped being visible and became a seemingly invisible 1x1 icon in the systray. It's still clickable though.

MATE general version

1.18.0

Package version

1.18.6

Linux Distribution

Debian buster/sid

lukefromdc commented 6 years ago

Confirmed with mate-panel from git master (1.19.3 current as of Dec 21, 2017) on Debian Unstable. EDIT: Again I see from the dependencies this is a QT app, and at least some other QT apps have had this problem

Although I have no accounts for actually using Telegram, I was able to get far enough into startup to turn the tray icon on and off. Got the clickable but invisible 1px line, and these errors in terminal:

Cannot connect to server socket err = No such file or directory Cannot connect to server request channel jack server is not running or cannot be started JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for 4294967295, skipping unlock JackShmReadWritePtr::~JackShmReadWritePtr - Init not done for 4294967295, skipping unlock

None of those would impact the icon, but when I tried running mate-panel from terminal while running Telegram and turning the icon on and off, turning it on or turning it off generated one of these warnings each time:

(mate-panel:19063): GLib-GObject-CRITICAL **: 15:35:57.632: g_object_unref: assertion 'G_IS_OBJECT (object)' failed

Don't know if this is related or not

Maxxie commented 6 years ago

Those warnings shouldn't be related. I think the same warnings used to appear in earlier versions when the icon was showing fine. Also I forgot to add that both the official telegram build and the one from the debian repos seem to have this problem.

lukefromdc commented 6 years ago

Try testing both with and without sni support. Open dconf-editor, try setting org>mate>panel>general>enable-sni-support to false and restarting the panel (killall mate-panel or mate-panel --replace) and see what you get. Then try setting it back to true and restarting the panel again.

I strongly suspect this is another QT issue and will affect other QT apps. There is another QT bug affecting the tray https://github.com/mate-desktop/mate-panel/issues/629 but that one is an excessive CPU use bug

Maxxie commented 6 years ago

Thank you for your help, disabling SNI seems to work. What is SNI exactly? Can't seem to find much about it...

lukefromdc commented 6 years ago

I may have just found out what's wrong in Telegram: in GNOME panel with SNI support, you do get an icon-but it's the "image-missing" icon for a missing icon, meaning Telegram's icon is not found by either panel. Telegram puts its icon in /usr/share/icons/hicolor, yet GNOME panel did not find it and apparently neither did mate-panel. I will look at the GNOME panel commits tomorrow and see what is different concerning this, as if the icon is not found the "image-missing" icon should be shown, and that's a more general issue in mate-panel that I will investigate.

SNI is status-notifier(indicator or appindicator) support, same as Ubuntu's indicators and supported by many apps. It's a newer API than the Xembedded tray icon system, main difference I notice is that in indicator mode the tray draws the menus. Lots of old Ubuntu documentatio about indicators, now indicator/appindicator support has become much more widespread, support for it in mate-panel was ported over from the still-mantained GNOME panel.

My own interest in indicator support stems from using a theme that gives mate-panel menus a different them than the main GTK theme for menus, so on my setup I use indicator mode wherever possible to get consistant menus. Also would be easier for someone more skilled than me to do a future port to Wayland for indicator support as it would not require reimplementing Xembed. Unfortunately, as you found it's still a bit buggy.

lukefromdc commented 6 years ago

This is probably an instance of https://github.com/mate-desktop/mate-panel/issues/537

andreymal commented 6 years ago

@lukefromdc these issues are different. #537 about "Passive" items, this was resolved. As far as I know, Telegram is not "Passive" item.

lukefromdc commented 6 years ago

https://github.com/mate-desktop/mate-panel/pull/696 will show the "image-missing" icon in cases like this where the icon cannot be found or loaded. This is the same behavior with Telegram as gnome-panel, where the "image-missing" icon shows. Not sure whether Telegram is failing to properly pass the icon name, or something is broken in how QT handles this with mate and gnome panels, but the failure to show "image-missing" if for any reason the intended icon cannot be loaded is itself a bug.

lukefromdc commented 6 years ago

I have avidemux-qt installed, and it's status notifier icon works fine. I know it's using the status notifier because if it did not, the menu that opens when the icon is clicked on would follow my main GTK theme for menus, not the different theme my panel sets. Avidemux's GUI was ported from GTK to QT a while back. Thus, a QT issue breaking all status notifier icons in QT apps can be ruled out.

Thus it appears that Telegram is doing something different in passing the icon name to the status notifier, I will look through Telegram's code and see if I can find it.

lukefromdc commented 6 years ago

I could not find the code where Telegram actually sets the icon name, but I did find that the installed icon is findable by temporarily hardcoding "telegram" in place of "image-missing" and then running Telegram with #695 applied to mate-panel. Thus, it's not passing NULL or failing to pass any icon name, rather something other then "telegram" is coming up.

Also, telegram uses org.kde.StatusNotifierWatcher to set up the status notifier, and it looks like Avidemux-gtk is doing somthing different, because it only puts an icon into /usr/share/pixmaps and nowhere else. Thus I can no longer rule out a broader QT to mate-panel issue

lukefromdc commented 6 years ago

I tested this code in sn-item-v0.c with an added message function to print the icon name in terminal:

  if (v0->icon_name != NULL && v0->icon_name[0] != '\0')
    {
      GdkPixbuf *pixbuf;
      pixbuf = get_icon_by_name (v0->icon_name, icon_size);
       g_message (v0->icon_name);
      if (!pixbuf){
          /*deal with missing icon or failure to load icon*/
          pixbuf = get_icon_by_name ("image-missing", icon_size);
      }
      gtk_image_set_from_pixbuf (image, pixbuf);
      g_object_unref (pixbuf);
    }

mate-polkit gives this result if an internal volume has been mounted:

notification-area-applet-Message: 22:04:12.166: dialog-password

dialog-password is a valid icon name, it is used in get_icon_by_name (v0->icon_name, icon_size);

telegram-desktop gives

notification-area-applet-Message: 22:05:12.179: /home/luke/.local/share/TelegramDesktop/tdata/ticons/icomute_22_0.png and a path does not work in get_icon_by_name (v0->icon_name, icon_size);

In other words, the status notifier is looking for an icon NAME, not an icon PATH, which is what it is being sent

raveit65 commented 6 years ago

Btw. the status icon from dnfdragora, a package-manager for fedora, has the same problem. With the PR this application displays the missing-image icon too. And those app use the hicolor dir too. https://github.com/manatools/dnfdragora dnfdragora is the main package-manager for the MATE-Compiz spin from fedora. I disabled SNI in fedora for this reason.

andreymal commented 6 years ago

I turned off SNI, and Telegram icon returned. Thanks for this workaround :)

muktupavels commented 6 years ago

Incorrect use of API, simple as that...

https://wiki.ubuntu.com/DesktopExperienceTeam/ApplicationIndicators

Because app indicators are cross-desktop, the icon_name parameter expects an icon name according to the usual icon naming spec.

https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/

An icon can either be identified by its Freedesktop-compliant icon name, carried by this property of by the icon data itself, carried by the property IconPixmap.

https://specifications.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html There is no singe word that icon name can be path.

Normally you would open bug for application about incorrect API usage, but with SNI things are different - you can do whatever you want.

http://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk.16.10/revision/277 http://bazaar.launchpad.net/~indicator-applet-developers/libappindicator/trunk.16.10/revision/281

Icon Naming Specification? Lets accept path to workaround real problem. Should there be any update to specification? Why? No, when other implementations will break someone most likely will provide fix...

SNI is probably best specification that can be used as example to "how to NOT write specification".

lukefromdc commented 6 years ago

https://github.com/mate-desktop/mate-panel/pull/697 will load icons specified by path, tested with Telegram and it works here.

raveit65 commented 6 years ago

Sadly we have a icon size problem with dnfdragora now, see my comment in PR.

lukefromdc commented 6 years ago

That should be fixed by https://github.com/mate-desktop/mate-panel/pull/697/commits/e5c8646d8f6167fe2d3634a4604a1bf9ec8ee5af " status-notifier: handle wrong-size icons called by path "

monsta commented 6 years ago

So I was trying to debug a similar issue with VeraCrypt (reported here). After I built mate-panel 1.18.7 which has all the needed patches, I only got an "image-missing" icon. Here's what it does with appindicator support:

if (indicator == NULL) {
    indicator = app_indicator_new ("veracrypt", "veracrypt-indicator", APP_INDICATOR_CATEGORY_APPLICATION_STATUS);
    app_indicator_set_status (indicator, APP_INDICATOR_STATUS_ACTIVE);

    GtkWidget *menu = gtk_menu_new();

[...]

    gtk_widget_show_all (menu);
    app_indicator_set_menu (indicator, GTK_MENU (menu));
}

So it sets icon name to "veracrypt-indicator" which doesn't exist anywhere.

What happens in tray area without SNI:

What happens in tray area with SNI:

What happens in mate-indicator-applet (with indicator-application loaded):

I don't get it... 😕

muktupavels commented 6 years ago

I would say SNI behaviour is correct. What are you expecting if application sets icon name as veracrypt-indicator, but installs that icon as veracrypt? Open bug against application that sets wrong application icon.

muktupavels commented 6 years ago

http://bazaar.launchpad.net/~indicator-applet-developers/indicator-application/trunk.17.04/view/head:/src/indicator-application.c#L493

Indicator application uses helper from libindicator: http://bazaar.launchpad.net/~indicator-applet-developers/libindicator/trunk.16.10/view/head:/libindicator/indicator-image-helper.c

It works because it uses g_themed_icon_new_with_default_fallbacks: https://developer.gnome.org/gio/stable/GThemedIcon.html#g-themed-icon-new-with-default-fallbacks

I would say that icon works there by luck. Unless it is documented somewhere that SNI should do that, it is bug in application!

lukefromdc commented 6 years ago

That is DEFINATELY an application bug. It works only with g_themed_icon_new_with_default_fallbacks, and as https://github.com/muktupavels says, unless that is a documented part of the SNI specification it is not reasonable to expect such a misnamed icon to be supported.

g_themed_icon_new_with_default_fallbacks won't handle the case of a path to an image being passes a path to a filename as the icon "name," as Telegram does. We could use that for handling the named icon case I suppose.

monsta commented 6 years ago

Ok, I mailed the owner of PPA with VeraCrypt (it's only available from this PPA, and appindicator support is added via patch there).

spvkgn commented 6 years ago

@monsta Can you help, how to get VeraCrypt tray icon in 18.04 (MATE 1.20)? VeraCrypt installed from PPA you listed above. Telegram icon also not appears and no icons of Qt apps (Goldendict, Soulseek-qt, etc). In 17.10 worked perfectly.

lukefromdc commented 6 years ago

In the meantime, consider locally adding a copy of the icon named "veracrypt-indicator" in the same icon directory as "veracrypt"

spvkgn commented 6 years ago

This not helps.

$ sudo cp /usr/share/pixmaps/veracrypt.xpm /usr/share/pixmaps/veracrypt-indicator.xpm
$ ls /usr/share/pixmaps/veracrypt*
/usr/share/pixmaps/veracrypt-indicator.xpm  /usr/share/pixmaps/veracrypt.xpm
$ veracrypt
Gtk-Message: 10:08:51.795: Failed to load module "appmenu-gtk-module"
$ echo $GTK_MODULES
appmenu-gtk-module:gail:atk-bridge:canberra-gtk-module
lukefromdc commented 6 years ago

First restart the panel and restart the application setting the icon. If that does not work try the tests below:

If it is looking for a themed icon, it might be ignoring everything in pixmaps where it installed the icon. Try moving that copy to /usr/share/icons/hicolor/16x16 (or whatever the correct size for it) or into the equivalent directory for the theme you are using, see what happens. You might need to run gtk-update-icon-cache -q -t -f /usr/share/icons/hicolor (or the same for your theme directory) for the icon to show up.

If that works, we have an issue with icons in /usr/share/pixmap. Then try renaming that icon usr/share/icons/hicolor/16x16/veracrypt and restarting the panel and veracrypt as well. If the icon appears, we have only an issue with icons in /usr/share/pixmaps.

If the icon does not appear then, appeared when named usr/share/icons/hicolor/16x16/veracrypt-indicator.xpm, and did not appear when named /usr/share/pixmaps/veracrypt-indicator.xpm then we have both an issue with Veracrypt setting the wrong icon name AND an issue with the xpm icons installed into /usr/share/pixmaps. If the icon NEVER appears, we need to know if anyone has ever made any .xpm icon work with an indicator/SNI tray icon.

monsta commented 6 years ago

Ubuntu MATE 18.04 uses indicators in all panel layouts by default, therefore these icons are now handled by indicator-application. In my test system, Clementine and VLC show their icons there.

ssrublev commented 6 years ago

I have encountered multiple systray problems in newest MATE and I have posted recently some links here https://github.com/mate-desktop/mate-panel/issues/811

raveit65 commented 6 years ago

Ubuntu MATE 18.04 uses indicators in all panel layouts by default, therefore these icons are now handled by indicator-application.

In my ubuntu VM ( 18.04) are boths there, ubuntu indicator stuff and our na-tray. Eg. nm-applet use our na-tray.

monsta commented 6 years ago

It uses indicator-application here... If you're running your 18.04 since early stages (alpha, beta etc.), you might need to reset the panel layout. Indicators setup was changing during the dev phase.

http://ubuntu-mate.org/blog/ubuntu-mate-bionic-final-release/

Anyone upgrading from Ubuntu MATE 16.04 or 17.10 may need to use MATE Tweak to reset the panel layout to one of the bundled layouts post upgrade.

Migrating panel layouts, particularly those without Indicator support, is hit and miss. Mostly miss.

The same applies to early 18.04...

raveit65 commented 6 years ago

Screenshot with traditional layout and Blue-Submarine theme, VM is updated and Layout is switched. ubuntu-18 04-na-tray We have our na-tray-applet with volume and nm-applet + ubuntu indicator applet (right from na-tray). About windows don't lie ;)