Skip to content
This repository has been archived by the owner on May 8, 2024. It is now read-only.

StatusNotifier uses the wrong icon name #465

Open
mtwebster opened this issue Sep 11, 2020 · 14 comments
Open

StatusNotifier uses the wrong icon name #465

mtwebster opened this issue Sep 11, 2020 · 14 comments

Comments

@mtwebster
Copy link

mtwebster commented Sep 11, 2020

Hi, we recently began supporting the StatusNotifierItem spec in Cinnamon/Mint, and it seems as though it uses the incorrect icon when no im is selected.

See:
https://user-images.githubusercontent.com/262776/92781618-48b71200-f372-11ea-8498-c9dd3f6222c9.png

This is an icon from the active icon theme. I think it needs to use the fcitx-specific name (which the xembed tray icon appears to use)

edit: I updated this to really the only pertinent thing, the no-im-active icon being a generic theme name, rather than an fcitx icon name.

mtwebster referenced this issue in linuxmint/cinnamon Sep 11, 2020
name and object path.

The name is not necessarily enough now due to changes here:
linuxmint/xapp@f6db3f7
@mtwebster mtwebster changed the title StatusNotifier item never updates its icon name StatusNotifier uses the wrong icon name Sep 11, 2020
@wengxt
Copy link
Member

wengxt commented Sep 14, 2020

I think the behavior matches the intention of the code.

Try to use Pinyin to see if it shows a "拼" icon.

The keyboard layout always uses that icon, unfortunately. The actual thing I'd like to show is some text (like "us" "ru"), but I don't think StatusNotifierItem supports something like show a text string.

There's a extension property XAyatanaLabel, I'm not sure if you guys support that.

@wengxt
Copy link
Member

wengxt commented Sep 14, 2020

So here's the thing, XAyatanaLabel was only supported on ubuntu initially and it's never supported on KDE, so I never add such support to fcitx 5 (mainly because ubuntu uses gnome-shell now, but on gnome-shell we recommend to use https://extensions.gnome.org/extension/261/kimpanel/). But I think fcitx 4 supports it, but only emit it under unity for now, unfortunately

DBusMessage* msg = dbus_message_new_signal(NOTIFICATION_ITEM_DEFAULT_OBJ,

@wengxt
Copy link
Member

wengxt commented Sep 14, 2020

another reason is that, this property is undocumented, and I never get what's the point of XAyatanaLabelGuide because no one use it nowadays.

ubuntu's appindicator extension seems to show only the label string without touching XAyatanaLabelGuide.
https://github.com/ubuntu/gnome-shell-extension-appindicator/blob/c71f76e793d3371fbd5bdecbaa9736b4a8e623cc/indicatorStatusIcon.js#L71

wengxt added a commit to fcitx/fcitx5 that referenced this issue Sep 14, 2020
@wengxt
Copy link
Member

wengxt commented Sep 14, 2020

图片

This is what fcitx 5 it looks like with https://extensions.gnome.org/extension/615/appindicator-support/

@janos-r
Copy link

janos-r commented Sep 14, 2020

I thought the issue is the requested name...
When not using statusNotifier, it asks for /usr/share/icons/hicolor/22x22/apps/fcitx-kbd.png - this is the usual icon.
But with statusNotifier it asks for "input-keyboard" and it doesn't find much. What if it also asked for fcitx-kbd when under statusNotifier? Would we have the expected behavior (the previous fcitx-kbd.png icon)?

@wengxt
Copy link
Member

wengxt commented Sep 14, 2020

ah, I see. This is actually an intended behavior (hardcoded). Because input-keyboard is provided by most icon theme so this specific icon can looks consistent with the system icon theme.

Fcitx's own xembed tray icon doesn't use xdg icon theme, it only uses /usr/share/fcitx/imicon/.... .
Fcitx support xdg icon theme, but still, input-keyboard would be used.

@mtwebster
Copy link
Author

Yeah I think the main complaint was that moving from xembed to notifier is the (unexpected) change to the icons.

We support labels, but don't actually see them used anywhere, which is fine with me. The label guide was used to tell the panel "this is the maximum amount of space a label might take" so it could be pre-allocated, and adjacent items on the panel wouldn't be bumped around as the label changed. But I definitely would want to avoid any properties outside of the official StatusNotifier spec.

A couple of things to maybe keep in mind going forward:

  • You could set the IconThemePath property - it's implemented in notificationitem.c but is just a stub. If a path is supplied, it could be set to your private theme folder so these custom icons could be used.
  • You can use the IconPixmap property to send serialized icon data.

Anyhow, we're going to change our themes' input-keyboards icon to something less awful which should help with the original issue a bit.

Thanks a lot

@clefebvre
Copy link

Hi @wengxt,

The code is OK in my opinion. There are things which could be improved (using symbolics, hidpi support etc..) but these are different topics.

The best way to load the icon is by icon name (i.e. using the icon theme), and that's perfectly OK as well. But the problem is the name of the icon you're using... it's a very generic one. It's also used by an application in MATE (the Keyboard preferences) and by an application in Cinnamon (the Virtual Keyboard app).

The icon theme can make input-keyboard look either like an app icon, or a status icon, but not both.. these are two different styles of icons but there's only one name here.

If you use fcitx-kbd instead of input-keyboard, then it will look in the theme for it and won't find it, and it will fall back to the icon you provided:

image

We can also make it look different by adding it to our icon theme, and this time it won't conflict with any other apps that use input-keyboard, since only fcitx uses fcitx-kbd.

@clefebvre
Copy link

In terms of code I'm only talking about changing the default/kbd icon name:

image

@wengxt
Copy link
Member

wengxt commented Dec 22, 2020

So, first of all, fcitx 4 reaches its end of life and only in maintenance mode for certain bug fixing. So after all it is not gonna be changed. fcitx 4 xembed icon doesn't support xdg icon theme so I have to add a "kbd" for it anyway. That's basically why there's a fcitx-kbd there, but never be used for SNI.

Secondly, even on fcitx 5, we still gonna use input-keyboard ( or "-symbolic". based on xdg icon fallback rule it will fallback to input-keyboard if symbolic version is not available, but on certain desktop input-keyboard works better so that's where we prefer input-keyboard as name). I think it looks ok on major desktop (KDE/GNOME). If we gonna use a keyboard icon anyway, I would prefer the icon comes from desktop icon theme because it IS a generic icon considering the fact that fcitx manages keyboard layout. I think using input-keyboard is the right choice instead of adding an inconsistent icon of ourselves.

@clefebvre
Copy link

OK, it's a pity. We could have made fcitx status icons to make Mozc and the keyboard status consistent with each other. We can't theme this properly unless it uses its own icon name.

@clefebvre
Copy link

@wengxt in case it helps, we talk about this at linuxmint/mint20.1-beta#44.

On our side we'll stop using input-keyboard in Cinnamon and remove it from our icon theme. This fixes the issue for the original bug report (it makes input-keyboard used by fcitx look like Adwaita icon basically so it no longer looks like an application).

We'll also consider making a Cinnamon applet for it. Neither input-keyboard (from GNOME) nor fcitx-kbd (the provided one) look great. Switching to a symbolic and having the layout beside it ("fr", "de", "us"..etc..) would be much better.

@wengxt
Copy link
Member

wengxt commented Dec 22, 2020

Yeah, if you check above fcitx5's screenshot, it's using input-keyboard-symbolic now for non-KDE (in kde input-keyboard looks better in panel).

Different DE uses icon in a different way. Another thing FYI, gnome-shell used to replace all icon with -symbolic suffix when being displayed by gnome-shell, but now they just don't do that so you see the ugly colorful "input-keyboard". I'd suggest you try to patch it to input-keybord-symbolic to see if works better for mint.

BTW here's what it looks like in KDE,
image

Also, as a side note for "virtual keyboard" in your issue, KDE's breeze has "input-keyboard-virtual", which basically fallback to input-keyboard when "input-keyboard-virtual" is not available.

While some people may think that fcitx should have its own branding, but I think Fcitx is a system component and should be as generic as possible. Especially fcitx when managing keyboard layout, which is traditionally put under system's keyboard section.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants