kiconthemes-5.80.0 ruins LXQt's colorizing of SVG icons

[tsujan]

I know that this may sound impossible but I'm 100% sure about it: kiconthemes-5.80.0 came here with a KDE upgrade and made all my icons disappear after a logout and login (or reboot). It took me more than an hour to find out that it was kiconthemes and not one ot the other 76 KDE packages (in Arch) that caused the problem.

The problem happens with Breeze or any SVG icon set that has stylesheet. If I uncheck "Colorize icons..." in LXQt Appearance Configuration → Icon Theme, the icons will be shown; if I check it again, the icons will disappear. It was checked before. Downgrading kiconthemes to 5.79.0 removes the issue.

For now, I don't have the slightest idea how a KDE package can affect our icon handling but it's very annoying!

[tsujan]

The intruding commit is https://github.com/KDE/kiconthemes/commit/5666c0c46e913fecbe2b41157f241685f126ab30. I got rid of the problem by reversing it. I haven't found another way yet; particularly, registering XdgIconLoaderEngine for svg had no effect.

The issue isn't limited to colorizing: some icon paths aren't seen either. For example, the start menu icons of some panel themes become blank.

An intrusion by another icon engine is bad. Something is missing from our code?

[tsujan]

OK, registering XdgIconLoaderEngine for svg was a baseless idea -- I'd forgotten the code of xdgiconloader.cpp → ScalableFollowsColorEntry::pixmap().

With the above-mentioned commit, kiconthemes practically hides qtsvg from us by stealing its key, namely "svg". Therefore, even if we use QSvgRenderer directly (with a self-made cache) or resort to the old method (https://github.com/lxde/libqtxdg/pull/144), we'll still be at the mercy of kiconthemes without colorizing and will suffer from its bugs (I mentioned one above).

It seems to me that the only ways out of this situation are (1) not installing kiconthemes -- which means not installing KDE alongside LXQt (not acceptable for many); or (2) reversing the bad commit; or (3) handling SVG completely as kiconthemes does (cumbersome).

[Chiitoo]

Interesting. This commit broke things in a different way for me, where Falkon and some other applications didn't even start (fixed in 65ee2fac [1]).

Unfortunately, I use the Oxygen icon theme which seems unaffected. Could have reported this, too, while reporting the other issue...

1. https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/22

[stefonarch]

@yan12125 mentioned this in IRC https://bugs.archlinux.org/task/69988

[tsujan]

@Chiitoo, @stefonarch, thanks!

Happy to know that @yan12125 had seen the problem before me -- Arch updates came to Manjaro Testing (my distro) only last night.

The solution No. 3 is always possible (I'll try it as soon as I find some time) but the above-mentioned commit isn't acceptable: no engine should introduce itself by using the key of another one, especially when the latter is so important.

[yan12125]

Thanks for creating this issue. I was going to open one :)

I mentioned the problematic commit in https://bugs.kde.org/show_bug.cgi?id=434451. Let's see what KDE devs think. The best option is fixing the issue in KDE. If no KDE devs respond in a few days, I will ask if KDE maintainers at Arch want to revert that commit like this PKGBUILD.

[tsujan]

I mentioned the problematic commit in...

Thanks a lot!

I hope KDE will reverse it but using of QSvgRenderer for all SVG icons in our icon engine should prevent such nasty issues. With an appropriate cache, I don't think there would be an overload but I should first test it. Our current code works fine but it isn't elegant because it depends on how things are done in qtsvg's code.

[stefonarch]

I blocked update of kiconthemes in/etc/pacman.conf, no issue.

[tsujan]

I tried QSvgRenderer with QPixmapCache and, as far as my quick tests showed, it worked fine in spite of the above-mentioned commit.

However, apparently, kiconthemes-5.80.0 is still an issue with icons of some stylesheets (like those of lxqt-panel). The problems that it creates can't be completely avoided. That commit should be reversed.

[yan12125]

KDE devs are discussing about reverting it: https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/25

Somehow I failed to create an account for logging into KDE's GitLab :/

[yan12125]

Merge request 25 is closed in favor of https://invent.kde.org/frameworks/kiconthemes/-/merge_requests/26. Looks like KDE devs are willing to fix bugs in kiconthemes while they also want to keep kiconthemes as the svg handler...

@tsujan Do you have an account on KDE's but tracker? I'd like explain why things are broken in LXQt, but I'm not familiar with libqtxdg or lxqt-qtplugin enough to write a good comment. If you don't, I can also forward explanations.

PS: with the new kiconthemes patch (MR 26), the issue in LXQt still persists.

[tsujan]

Looks like KDE devs are willing to fix bugs in kiconthemes while they also want to keep kiconthemes as the svg handler...

Very bad decision! I fixed the colorizing here by using QSvgRenderer but the non-colorized mode will be ignored by kiconthemes. I might be able to fix that too but I think SVG icons will still have problems in Qt stylesheets.

@tsujan Do you have an account on KDE's but tracker?

I do but I'm too angry now. Maybe later. For now, I prefer to focus on my workaround for LXQt not to be at the mercy of such decisions in future.

[tsujan]

This is my proposal: https://github.com/lxqt/libqtxdg/pull/247

It may seem complex in the form of a patch but its logic is as simple as this: use QSvgRenderer to draw SVG icons, whether they're colorized or not.

It passed my tests after I re-installed the problematic kiconthemes V5.80.0.

As for stylesheets, I can see only the lack of start menu icon in one LXQt theme (Kvantum). I can't explain it but it can be easily fixed by a simple change.

Of course, more tests are needed. Also, an LXQt dev may find and fix a problem in the PR; it definitely needs a review.

[tsujan]

OK, I can't see any problem in practice but it should be tested more.

I think the most important thing was preventing plugins like kiconthemes from ruining LXQt's handling of SVG icon themes. That's done by the PR.

The remaining issue is that, if kiconthemes is present, it'll still be active — with its bugs and "features" — wherever an SVG file is direcctly used as an icon. I don't think we can do anything about it without making the same mistake that KDE made. IMO, the possibility of abusing JSON files in this way is an issue in Qt.

A word about why I got angry at first (→ https://github.com/lxqt/libqtxdg/issues/246#issuecomment-801876169):

Suppose that I'd made the Oxygen widget style disappear by adding it to Kvantum's JSON file. That's quite possible. If I did so, you'd choose "Oxygen" in LXQt Appearance Configuration but you'd see only Kvantum! Would you congratulate me on my decision?

[palinek]

IMO, the possibility of abusing JSON files in this way is an issue in Qt.

Heh...isn't this even a vulnerability? With this, if you're able to fool the user to install your (other way valid) app (which will silently steel the svg rendering), then you can attack the victim with perfectly renderable .svg which includes malicious code/directions for your compromised "render engine".

[tsujan]

isn't this even a vulnerability?

Yes, it can be seen as a vulnerability. You provided a good example.

I meant the relation of "power" and responsibility. In my example, the "power" is k < o; in the case of kiconthemes, it's K < l (KIconEnginePlugin.so → libqsvgicon.so).

[tsujan]

I think this can be closed now.

If a user opens an issue about, for example, the wrong size of flags in Panel's keyboard indicator, he/she could be asked to open an issue against kiconthemes. I'll try to patch every new version of kiconthemes here for comparison.