Skype Integration legacy icon removal sometimes fail

[ronjouch]

I use TopIcons-plus as well as Skype Integration, which has a feature that was broken by TopIcons-plus commit b3321279 "add center position". The now-broken feature is that Skype-Integration can hide Skype's legacy tray icon (because it adds a GNOME HIG-compliant indicator). So, being unable to hide the legacy tray icon and adding a new modern indicator nevertheless, I end up with two Skype "top-right icons": the legacy, and the modern one.

Looking at recent changes, it looks like this line of TopIcons-plus b3321279 is the culprit: it changes the icon wm_class with:

let wmClass = icon.wm_class ? icon.wm_class.toLowerCase() : '';

... so "Skype" → "skype". Unfortunately, Skype-Integration legacy-icon hiding function checks on the capitalized "Skype" string:

if(this._skypeHideOriginalTrayIcon && icon.wm_class == "Skype") {

→ I'd propose a patch removing this call to .toLowerCase(), but looking at b3321279 I fail to understand why it's necessary in the first place. Is it / can we remove it? (Note: looking at the commit, I realize this wm_class de-capitalization code is not new, it was just extracted into a function. Anyway, maybe the class-changing was happening later / less frequently before b3321279 or some changed logic now causes a race between TopIcons-plus and Skype-Integration; whatever: the end result is that I never had this problem until upgrading TopIcons-plus a few days ago, and now it's apparent)

Thanks for the continued maintenance of the extension :)

[phocean]

Hello,

Thank you for the report. I am using Skype version 4.3.0.37 and it works as expected. And I don't understand how the change I made would have some impact on this.

To clarify:

• TopIcons does not change wmClass, it hides Skype icon when the Skype integration extension is enable. What you see in the code in just a local variable allocation, in no way it is changing anything on the Skype window.

• It is normal that the code uncapitalises wmClass before it is compared with the blacklist. Not necessary but just to be safe.

• Skype integration extension also has an option to hide the tray based on "Skype" string. So both features are partially redondant, but TopIcons covers more wmClass string cases.

I believe for some reason your wmClass is not exactly Skype. What version of Skype are you using ?

[phocean]

After starting Skype, could you give me the output of:

% xprop | grep WM_CLASS

You will have to select the Skype contact window when the cross cursor shows up.

[ronjouch]

@phocean

TopIcons does not change wmClass, it hides Skype icon when the Skype integration extension is enable. What you see in the code in just a local variable allocation, in no way it is changing anything on the Skype window.

True, sorry; I did a quick parse/blame of recent commits and stumbled on this line but I didn't read what was done with it. So let me "de-pinpoint" my incorrect conclusion to a broader regression statement:

• Since I updated TopIcons-Plus on December 23, Skype-Integration fails to destroy the legacy icon (whose features are replaced/re-implemented with a proper Shell indicator), resulting into a duplicate Skype icon:
  
• Bug appears when starting Skype. If then I restart Shell (Alt+F2, r), then the legacy icon is properly destroyed:
  

What version of Skype are you using ?

~ skype --version              
Skype 4.3.0.37
© 2014 Skype and/or Microsoft

What does xprop | grep WM_CLASS return?

~ xprop | grep WM_CLASS
WM_CLASS(STRING) = "skype", "Skype"

[phocean]

Ok, thank you for the detailed information. ;-)

I could reproduce the issue myself, but only once in a while. The cause is quite obscure to me, I have to investigate.

[ronjouch]

One more thing: in the same timeframe (so maybe due to the same changes?), other applications started to fail to be TopIcon-ified. For example, Transmission doesn't get its TopIcon, and I have to restart Shell (Alt+F2, r) to get it. I can create a new bug if you think these are two separate problems.

[phocean]

EDIT: Ignore more or less my previous answer if you got it.

Again weirdness: on my 2 test machines, I can reproduce it only on Wayland and with either the legacy tray (Gnome on) or the topicons extension. On Xorg, it works without any problem.

It is strange because, if you can reload, it means you are on Xorg.

Have you tried with the legacy tray only?

[phocean]

It could be related to #42, by the way.

[ronjouch]

@phocean

It could be related to #42, by the way.

Indeed. Thanks.

It is strange because, if you can reload, it means you are on Xorg.

Yes I am running Xorg, I blacklisted Wayland because I need X for a few X-only tools I use (wmctrl, xdotool, autokey) with no Wayland equivalent as far as I know :-/ .

Have you tried with the legacy tray only?

If that's what you mean, I just tried disabling TopIcons (i.e. leaving Shell do its bottom-left corner job), and I couldn't get the Transmission icon to disappear like it happens with TopIcons; everything seems okay: Transmission's icon is always present in the bottom-left tray.

[phocean]

see issue #47 for Transmission, sorry but it is not something we can fix.

[phocean]

I pushed a fix attempt on the dev branch. And, wow, that was a stupid and obvious mistake of mine.

So far it is working for me, could you please test as well?

[ronjouch]

I pushed a fix attempt on the dev branch. And, wow, that was a stupid and obvious mistake of mine. So far it is working for me, could you please test as well?

@phocean seems to work fine here too; closing this bug, will re-open if I run into problems. Thanks for the fix!