Fix Plasma themes' quassel icon to match current quassel icon names
AbandonedPublic

Authored by kossebau on Apr 5 2019, 11:39 AM.

Details

Reviewers
None
Group Reviewers
Plasma
VDG
Summary

Since v0.13.0 (17 Nov 2018) quassel uses new names for the systemtray
status icons. This patch updates the icon name ids.

Diff Detail

Repository
R242 Plasma Framework (Library)
Branch
updatequasselicons
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 10459
Build 10477: arc lint + arc unit
kossebau created this revision.Apr 5 2019, 11:39 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptApr 5 2019, 11:39 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
kossebau requested review of this revision.Apr 5 2019, 11:39 AM

To test this quickly, please download the files (View Options>Show Raw File (Right)>Download File) and place them in the respective folders /usr/share/plasma/desktoptheme/{air,breeze}/icons/, then "rm .cache/plasma* -r", and restart "plasmashell --replace" e.g. via krunner.

apol added a reviewer: VDG.Apr 5 2019, 12:47 PM
broulik added a subscriber: broulik.Apr 5 2019, 2:10 PM

I don't see how this could work. IconItem uses the first section as filename, for active-quassel-tray the word active and looks for an active.svgz which there isn't. This is a bug in Quassel, a violation of the FDO spec, it should have ebeen quassel-tray-inactive or something, ordered from generic to most specific.

kossebau abandoned this revision.Apr 5 2019, 2:49 PM

I don't see how this could work. IconItem uses the first section as filename, for active-quassel-tray the word active and looks for an active.svgz which there isn't. This is a bug in Quassel, a violation of the FDO spec, it should have ebeen quassel-tray-inactive or something, ordered from generic to most specific.

So much for blindly done patches relying on testing by actual users :)

Indeed, IconItem uses this logic:

m_svgIcon->setImagePath(QLatin1String("icons/") + sourceString.section(QLatin1Char('-'), 0, 0));

so this change will not work.

But I wonder if this isn't a rather once heuristically-driven assumption of Plasma, over anything specified at freedesktop.org?
I could not find anything related in both
https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/
https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/Icons/
as well as
https://specifications.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html#file_formats

Where should/could Quassel developers be pointed to? (Besides, they also have some "-inverted" variant based on a config option, which somehow complicates things even more, besides the complexity of Plasma theming vs. normal icon theming ;) ).

zzag added a subscriber: zzag.Nov 18 2019, 11:35 AM

This is a bug in Quassel, a violation of the FDO spec, it should have ebeen quassel-tray-inactive or something

See https://github.com/quassel/quassel/pull/515