Icon problem on KDE

[cassiodoroVicinetti]

As I already wrote in https://github.com/Aluxian/Facebook-Messenger-Desktop/issues/296#issuecomment-168070347, in Plasma 5, after installing the libappindicator1 package (Debian/Ubuntu), the tray icon is not shown correctly: in particular, the 64x64 icon is cropped in the middle to a 24x24 square before being shown:

The crop doesn't change if the application bar height is changed.

The icon is currently a .png file, but .ico files should also work (nwjs/nw.js#88 (comment)), so I created a .ico file containing a 64x64 icon and a 24x24 icon.

Sadly I was not able to compile the application to test it, so I kindly ask @Sytten to compile it for me and provide me with the .deb so that I can test it.

[Sytten]

Just checked quickly what you modified, looks good for me except it will also modify the icon on windows. So I will have to modify some of your modifications to not break that compatibility (or maybe it works too with ico on windows, will need some testing).

https://github.com/Sytten/Facebook-Messenger-Desktop/tree/HotFix-Icon

[Sytten]

http://www.mediafire.com/download/um7eslf5e4t8zbo/Messenger_linux32_Test.deb

EDIT: I forgot to update the version number, so you will have to uninstall your current version first...

[cassiodoroVicinetti]

My system is 64-bit, so please compile the linux64 build.

[cassiodoroVicinetti]

Whoops no stop, I installed it on another computer with KDE 4 (32-bit), and the icon is not shown!

(of course, the current build in the master branch shows the icon correctly in KDE 4)

[Sytten]

There you go: http://www.mediafire.com/download/kwa22psuldv4xjb/Messenger_linux64_Test.deb

[cassiodoroVicinetti]

In Plasma 5 the icon does not appear at all and on the console it's written:

[6345:0104/213610:WARNING:app_indicator_icon.cc(274)] Could not encode icon

I think a different icon should be loaded based on whether Plasma 5 is running.

By reading here, I discovered that it is possible to detect Plasma 5 by means of the following environment variables: KDE_FULL_SESSION and KDE_SESSION_VERSION.

In my KDE 4 system:

• KDE_FULL_SESSION="true"
• KDE_SESSION_VERSION="4"

In my Plasma 5 system:

• KDE_FULL_SESSION="true"
• KDE_SESSION_VERSION="5"

The detection should be performed as follows:

1. check whether KDE_FULL_SESSION is not empty[1], in order to see if a KDE session is running;
2. check whether KDE_SESSION_VERSION is equal to 5, meaning that Plasma 5 is running;
3. load a 24x24 icon instead of the default 64x64 one.

What do you think?

In the meanwhile, it would be great if you provided me with a .deb containing just the 24x24 icon (in .png), just to see if loading a 24x24 icon solves the issue or the icon is still cropped.

Otherwise, if you want, I could try again to compile by myself the application, but I need your help because yesterday I had several problems (I've never used NodeJS or gulp).

[1] just checking whether the variable is equal to "true" is wrong, as stated in the KDE documentation:

If you plan on using this variable to detect a running KDE session, check if the value is not empty instead of seeing if it equals true. The value might be changed in the future to include KDE version information.

[Sytten]

Well, for me thats a bug in the NWJS framework. This work-around is quite messy but could be done. I currently don`t have the time to implement it, so I suggest you try to do it and make a pull request ;)

I don`t really have experience with node, I learn what I need as I go.
To compile it, it is quite simple. First pull the current project in a directory. Second, install npm (they have a ppa repo). Third, you go in the project directory and:

• install gulp:
  npm install -g gulp
• Install NW:
  npm install -g nw
• install dependencies:
  npm install

After, you move in the src folder and you can test the app by doing: nw . (the final dot is important)

If you want to package the app, thats a bit more complicated.
The easiest solution I found is to do:

• install Ruby (to get gem):
  apt-get install ruby-dev gcc make
• install FPM:
  gem install fpm

That allows you to do, in the directory of the project (not the src directory), that:
gulp pack:linux64:deb

Hope it helps

[cassiodoroVicinetti]

I tried to implement it, but I haven't tested it. Now I'm trying to follow you instructions.

[cassiodoroVicinetti]

It works like a charm in Plasma 5!!! Now I'm going to test it in my other computer with KDE 4.

I agree that this is a bug in the NW.js framework.

If you are interested for documentation purposes, I had to type the following commands in my system:

sudo apt-get install npm nodejs-legacy
sudo npm install -g gulp
sudo npm install -g nw
npm install request
npm install semver
npm install auto-launch
npm install jfs

[cassiodoroVicinetti]

OK, it's working in KDE 4 (it loads the old icon), so the pull request is ready!

[Sytten]

Ok perfect, I will merge it in the next release. I need to update nwjs to the new version when it will be stable, so your modification might become useless in a few weeks...

[Sytten]

Quick question, have you tried the libappindicator3-1 package?

[cassiodoroVicinetti]

I have both libappindicator1 and libappindicator3-1 packages installed, and if I remove libappindicator1 the icon disappears.

In my opinion you should make a new release now, then if the (possible) bug in NW.js will be fixed the workaround can be quickly reverted.

[cassiodoroVicinetti]

(sorry, I accidentally pressed Close and comment)

[Sytten]

I don`t really want to do a release only this specific bug, so I will wait until I have more stuff to push. But please refer to #12
We are talking of seriously moving on to the new app instead...

[cassiodoroVicinetti]

I updated my pull request removing merge conflicts. Is it time to merge it?

[Sytten]

I'm working on it right now ;)
I might try to do a more elegant fix with a .desktop like someone suggested me, what do you think?

[cassiodoroVicinetti]

I think there is no need, my patch is working and that's what matters. Who suggested you the alternative fix?

[Sytten]

It works, but I don't really like the idea of adding code for a desktop environment.
https://github.com/Aluxian/Facebook-Messenger-Desktop/issues/296#issuecomment-168990541

[Sytten]

@cassiodoroVicinetti can you test the fix he proposed?

[cassiodoroVicinetti]

EDIT: No, it's NOT working, see below.

It's working. You could detect Plasma 5 in the after-install.sh script and edit the .desktop file...

[cassiodoroVicinetti]

Whoops, the icon just turned into another icon!

[Sytten]

Thats odd... I will check it out after I finished what I'm doing ( I correct the escape key bug)

[cassiodoroVicinetti]

The icon intermittently shows as an empty page: for a few seconds it is normal, then suddenly it turns into an empty page, then after a while it turns back to normal, and so on.

The behaviour looks unpredictable, but I think it happens more often just after a change of the icon (I mean: when a message arrives, and when you read a message).

I think it is the same behaviour reported in https://github.com/Aluxian/Facebook-Messenger-Desktop/issues/296#issuecomment-170405173
so this should be another bug for Unity.

[Sytten]

Hum... I will have to investigate that too. If you try to set the desktop env to something else like gnome, does it work?

[cassiodoroVicinetti]

With XDG_CURRENT_DESKTOP=GNOME the tray icon works like a charm, but this has a side effect on the context menu, which appears using GTK 2 instead of a native KDE menu:

XDG_CURRENT_DESKTOP=LXDE doesn't change the bug.

[Sytten]

I think I will go with this fix, because it will also fix the bug on the unity desktop env.

[cassiodoroVicinetti]

I disagree. My fix gives both a working tray icon and a native context menu. Unity should be addressed in a different way.

[Sytten]

Maybe but adding specific code for a desktop env only for some user of linux in the general app is messy and I wont do it.
The only thing I see that would be good is to change the icons in the after-install script if the desktop is plasma5.

[cassiodoroVicinetti]

And if the user has two desktop environments installed both in the same system?

[Sytten]

Lets go discuss it on gitter, I fixed the esc problem so now its time to fix that problem ;)
https://gitter.im/Sytten/Facebook-Messenger-Desktop

[Sytten]

@cassiodoroVicinetti I just installed the latest version of openSUSE and the system tray works perfectly out of the box with plasma 5. No problem of icon size... You might want to update to check it.

[cassiodoroVicinetti]

Which version of Plasma are you using?

$ apt-cache policy plasma-workspace
plasma-workspace:
  Installed: 4:5.4.3-2
  Candidate: 4:5.4.3-2
  Version table:
 *** 4:5.4.3-2 990
        990 http://ftp.de.debian.org/debian testing/main amd64 Packages
        100 /var/lib/dpkg/status

[Sytten]

From what I see its the KDE version 5.16.0

[cassiodoroVicinetti]

In order to check the Plasma version, launch:

$ plasmashell --version
plasmashell 5.4.3

Anyway, I found that Plasma 5.5 restored the legacy tray icons: https://www.kde.org/announcements/plasma-5.4.95.php
so maybe the bug has been fixed in Plasma 5.5!

[Sytten]

I think it is, I use version 5.5.4 and no problem with the icon.

[cassiodoroVicinetti]

Mmm so I guess you won't fix this...

[Sytten]

No I wont fix it, anyway I already release the new version and I dont have time to work on it...

[cassiodoroVicinetti]

$ uname -a
Linux debian-luca 4.5.0-2-amd64 #1 SMP Debian 4.5.5-1 (2016-05-29) x86_64 GNU/Linux
$ plasmashell --version
plasmashell 5.6.4

The icon issue is still there.

If I remove the libappindicator1 package, the icon size is correct, BUT the context menu looks like if I used XDG_CURRENT_DESKTOP=GNOME!

@Sytten Maybe your test on openSUSE succeeded because of this.

[Sytten]

@cassiodoroVicinetti Maybe I dont remember :/
I'm currently working on the next version on NWJS 0.14.5, maybe that will be fixed by it.

If not then I suggest you open a new issue on the official repo

[cassiodoroVicinetti]

Two considerations:

• If the gtk3-engines-breeze package is installed and the "Breeze" theme is selected as the GTK2 theme, then the context menu of the GNOME version (or the version without the libappindicator1 package) will look like the KDE version.
• The GNOME version raises the CPU usage of the plasmashell process to 10% and beyond.

[cassiodoroVicinetti]

The 1.5 version appears to fix the second point (CPU usage).

[Sytten]

Nice to hear, so where are we now exactly?
(Sorry didn't follow everything, some many problems to fix lately)

[cassiodoroVicinetti]

Basically if you are using Plasma 5:

1. install the 1.5 version (to fix the CPU usage issue of the GNOME version)
2. make sure the gtk3-engines-breeze package is installed and the Breeze theme is the current GTK2 theme (so the GNOME version will look as the KDE version)
3. launch the GNOME version (to fix the size of the tray icon):
   • if you have libappindicator1 installed, use env XDG_CURRENT_DESKTOP=GNOME
   • if you haven't libappindicator1 installed, you don't need env XDG_CURRENT_DESKTOP=GNOME

[Sytten]

Good I will add that to tke wiki page :)