Bug 479712 - Plasma 6's SNI implementation doesn't respect the IconThemePath property
Summary: Plasma 6's SNI implementation doesn't respect the IconThemePath property
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: System Tray widget (show other bugs)
Version: 5.92.0
Platform: Compiled Sources Linux
: HI normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords: qt6, regression
: 481682 481999 482201 482522 482610 482962 483049 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-01-12 23:22 UTC by Ilya Bizyaev
Modified: 2024-03-27 11:40 UTC (History)
16 users (show)

See Also:
Latest Commit: https://invent.kde.org/plasma/plasma-workspace/-/commit/976575921d2a1bbd2551afba335be08ebf0f2a25
Version Fixed In: 6.0.1
Sentry Crash Report:

Attachments
missing icon in 6.0.1 (924.76 KB, image/png)
2024-03-10 17:57 UTC, Matthias 		Details
Icons in System Tray look OK but a couple of them are very dim (28.73 KB, image/png)
2024-03-27 03:38 UTC, Patrick Van Rinsvelt 		Details
View All Add an attachment

Note You need to log in before you can comment on or make changes to this bug.
Description Ilya Bizyaev 2024-01-12 23:22:18 UTC 
SUMMARY

Due to some changes in Plasma 6 or KF6, apps that rely on the `IconThemePath` property of StatusNotifierItems can no longer display their logo in the system tray.

I've dumped the details of my investigation below, as I don't now how to fix the issue correctly myself.

STEPS TO REPRODUCE
Launch JetBrains Toolbox in a Plasma 6 session.

OBSERVED RESULT
A placeholder icon is displayed in Plasma's system tray.

EXPECTED RESULT
The JetBrains Toolbox logo is displayed.

SOFTWARE/OS VERSIONS
Operating System: openSUSE Tumbleweed 20240111
KDE Plasma Version: 6.0.80
KDE Frameworks Version: 5.249.0
Qt Version: 6.6.1
Kernel Version: 6.6.10-1-default (64-bit)
Graphics Platform: Wayland

ADDITIONAL INFORMATION

• Here:
https://invent.kde.org/plasma/plasma-workspace/-/blob/201436dda62f46d70df120cd113bf3eaefff1116/applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml#L35
— the icon theme path is lost, this name is only tried against the systemwide icon theme.
• The QIcon has the correct image because it is loaded with a configured KIconLoader:
https://invent.kde.org/plasma/plasma-workspace/-/blob/201436dda62f46d70df120cd113bf3eaefff1116/applets/systemtray/statusnotifieritemsource.cpp#L250-275
• But because loading icons by name is prioritized over QIcons, it's not used.

To solve this properly, Kirigami.Icon would need to be configured to look at `model.IconThemePath`, but it currently has no "themePath" property. To add it, this function would need to accept a list of paths or a KIconLoader, then pass it on to KDE::icon (where the latter is already an optional argument):
https://invent.kde.org/frameworks/qqc2-desktop-style/-/blob/3f5d157c923ad8eca85b592e80531703fafd47e3/kirigami-plasmadesktop-integration/plasmadesktoptheme.cpp#L248
But it's an override, the declaration is in Kirigami, and Kirigami does not depend on KIconThemes, so KIconLoader is probably not an option. On the other hand, if we pass paths, we'd have to create a KIconLoader from them every time, which is probably too expensive (I think every KIconLoader creates its own cache).

A hack would be:
```
return model.IconName.length > 0 && model.IconThemePath.length == 0 ? model.IconName : model.Icon
```
— but it would not solve the issue completely because a) monochrome icon coloring would be broken for such icons, and b) I think overlay icons are currently also broken because overlaying is also implemented with QIcons on the C++ side.

Fun fact: there's a DataEngine that contains a full duplicate of the above code as https://invent.kde.org/plasma/plasma-workspace/-/blob/master/dataengines/statusnotifieritem/statusnotifieritemsource.cpp?ref_type=heads.
Comment 1 Nate Graham 2024-02-23 22:22:12 UTC 
*** Bug 481682 has been marked as a duplicate of this bug. ***
Comment 2 Nicolas Fella 2024-03-04 21:11:09 UTC 
*** Bug 482201 has been marked as a duplicate of this bug. ***
Comment 3 Nicolas Fella 2024-03-04 21:11:16 UTC 
*** Bug 481999 has been marked as a duplicate of this bug. ***
Comment 4 Bug Janitor Service 2024-03-04 22:02:47 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/4002
Comment 5 Nate Graham 2024-03-04 23:35:02 UTC 
Git commit a713f2351ad7f9ea3b43d2ec436ff1f20d6c036a by Nate Graham, on behalf of Nicolas Fella.
Committed on 04/03/2024 at 23:34.
Pushed by ngraham into branch 'master'.

applets/systemtray: Prefer Icon over IconName

The systemtray has a custom KIconLoader in order to make the SNI's IconThemePath work.

When passing an icon name to Kirigami.Icon we skip that. Instead prefer the Icon role
from the model, which goes through the custom loader

This restores the logic to how it was before d31b985b6ba3d68b2ac018839864b305b6780574.
FIXED-IN: 6.0.1

M  +1    -1    applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

https://invent.kde.org/plasma/plasma-workspace/-/commit/a713f2351ad7f9ea3b43d2ec436ff1f20d6c036a
Comment 6 Ilya Bizyaev 2024-03-04 23:56:42 UTC 
The commit fixes this issue, but reintroduces coloring problems for monochrome icons
Comment 7 Nate Graham 2024-03-04 23:58:23 UTC 
Git commit 976575921d2a1bbd2551afba335be08ebf0f2a25 by Nate Graham, on behalf of Nicolas Fella.
Committed on 04/03/2024 at 23:39.
Pushed by ngraham into branch 'Plasma/6.0'.

applets/systemtray: Prefer Icon over IconName

The systemtray has a custom KIconLoader in order to make the SNI's IconThemePath work.

When passing an icon name to Kirigami.Icon we skip that. Instead prefer the Icon role
from the model, which goes through the custom loader

This restores the logic to how it was before d31b985b6ba3d68b2ac018839864b305b6780574.
FIXED-IN: 6.0.1


(cherry picked from commit a713f2351ad7f9ea3b43d2ec436ff1f20d6c036a)

34efcc14 [applets/systemtray] Prefer Icon over IconName

M  +1    -1    applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

https://invent.kde.org/plasma/plasma-workspace/-/commit/976575921d2a1bbd2551afba335be08ebf0f2a25
Comment 8 Bug Janitor Service 2024-03-05 10:14:06 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/4004
Comment 9 Nicolas Fella 2024-03-06 10:41:39 UTC 
*** Bug 482522 has been marked as a duplicate of this bug. ***
Comment 10 Nicolas Fella 2024-03-07 10:58:44 UTC 
*** Bug 482610 has been marked as a duplicate of this bug. ***
Comment 11 firewalker 2024-03-09 15:42:40 UTC 
I have also noticed missing icons (oxygen theme) in the tray. No Bluetooth, Notifications, Vault, Brightness and Color icons. Those icons seems to be missing from everywhere (eg System Settings). Is it the same bug?
Comment 12 Nicolas Fella 2024-03-09 15:57:14 UTC 
*** Bug 482962 has been marked as a duplicate of this bug. ***
Comment 13 Arka Bhuiyan 2024-03-09 22:12:22 UTC 
I am using Nobara Linux and the plasma version is 6.0.1. Unfortunately, I am seeing the fix yet. Is there something we need to do to fix the issue?
Comment 14 Nicolas Fella 2024-03-10 13:18:58 UTC 
*** Bug 483049 has been marked as a duplicate of this bug. ***
Comment 15 Matthias 2024-03-10 17:53:16 UTC 
At least two people on 6.0.1 confirm that this bug is still active. 
Tested with QBittorrent. 

NixOS and Arch Linux.
Comment 16 Matthias 2024-03-10 17:57:37 UTC 
Created attachment 166894 [details]
missing icon in 6.0.1
Comment 17 Matthias 2024-03-10 18:53:12 UTC 
Sorry, we failed. That issue here is indeed solved and we encountered an independent problem in Qbittorrent. Closing
Comment 18 Bug Janitor Service 2024-03-10 22:18:02 UTC 
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/4039
Comment 19 Nicolas Fella 2024-03-11 17:27:47 UTC 
Git commit 257ccabc526ab7f4d5ca12758c1e29af35b048ca by Nicolas Fella.
Committed on 11/03/2024 at 17:01.
Pushed by nicolasfella into branch 'master'.

[applets/systemtray] Load icons with Plasma palette

When using Breeze Twilight, i.e. dark Plasma and lights apps, we need to color the icons with the Plasma colors
Related: bug 482645

M  +24   -6    applets/systemtray/statusnotifieritemsource.cpp
M  +1    -0    applets/systemtray/statusnotifieritemsource.h

https://invent.kde.org/plasma/plasma-workspace/-/commit/257ccabc526ab7f4d5ca12758c1e29af35b048ca
Comment 20 Nicolas Fella 2024-03-11 17:30:06 UTC 
Git commit d73c6c4bb938bbd252c5bd69cc93862cba554e32 by Nicolas Fella.
Committed on 11/03/2024 at 17:06.
Pushed by nicolasfella into branch 'Plasma/6.0'.

[applets/systemtray] Load icons with Plasma palette

When using Breeze Twilight, i.e. dark Plasma and lights apps, we need to color the icons with the Plasma colors

To achieve this always set the current palette on the icon loader, which means always creating a custom loader

For the icons in the context menu we want the application palette, so create a separate loader for that
Related: bug 482645

M  +56   -24   applets/systemtray/statusnotifieritemsource.cpp
M  +2    -0    applets/systemtray/statusnotifieritemsource.h

https://invent.kde.org/plasma/plasma-workspace/-/commit/d73c6c4bb938bbd252c5bd69cc93862cba554e32
Comment 21 Patrick Van Rinsvelt 2024-03-27 03:37:11 UTC 
Not sure if you want feedback from a user but here it is. I package update every day and have been seeing the icons in the system tray and expanded tray all displaying properly now and for the past few days. This is using the Breeze, Breeze Dark as well as contributor icons as well. The only thing I would comment on is that the Printer and Weather icons in the system tray are pretty dim.

See attachment userSystemTraySnapshot_20240327

Let me know if I should not be commenting here
Comment 22 Patrick Van Rinsvelt 2024-03-27 03:38:06 UTC 
Created attachment 167830 [details]
Icons in System Tray look OK but a couple of them are very dim

As mentioned in ticket comment
Comment 23 Nicolas Fella 2024-03-27 11:40:37 UTC 
The breeze icons for printer and power management look correct here.

This looks like the third-party icon theme is not using the right color roles for some icons. Please report this to the theme author.