Applications can specify an absolute file path for an icon which won't work with QIcon::fromTheme.
If a theme icon isn't find, first try to load it using an absolute path before falling back to an unknown icon.
BUG: 365131
Details
Details
Diff Detail
Diff Detail
- Repository
- R119 Plasma Desktop
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
What's super weird about this one is that QIcon::fromTheme internally actually does
if (QDir::isAbsolutePath(name)) {
return QIcon(name);
}yet only with this patch I get an icon for .svg icons with an absolute path I configured. Maybe icon.availableSizes() is empty for svg images and thus the previously used fallback argument of fromTheme is always chosen.
Comment Actions
Please add a comment, and I wonder if the absolutePath check should maybe be in the code?
Comment Actions
Note this patch breaks fallback loading in the case where an absolute path for an SVG or pixmap is set.
It's possibly still worth it?
I think your analysis is right, and QIcon::fromTheme(name, fallback) also checks availableSizes because it can't rely on all engines implementing the new isNull virtual.
KIconLoader does, but even Qt's own ones QSvgIconEngine and QPixmapIconEngine don't currently.
| applets/kicker/plugin/appentry.cpp | ||
|---|---|---|
| 107–109 | This should be just: icon = QIcon::fromTheme(m_service->icon(); you don't need that bit in the middle. | |
Comment Actions
I keep stumbling upon this issue on bugzilla and wonder what we can do here?
- Fix the issue in Qt. @smartins Recently https://bugreports.qt.io/browse/QTBUG-63187 was opened, but besides triaging to P3 not much has happened yet. This may need someone submitting a patch. https://bugreports.qt.io/browse/QTBUG-59732 might be related.
- Add workarounds everywhere in our projects (not very appealing):
- Pursue this Diff.
- Try a different approach, e.g. strip the .svg from the absolute path in case of failure and try again.
Bugzillas (lxr will probably find even more cases):
https://bugs.kde.org/show_bug.cgi?id=371835
https://bugs.kde.org/show_bug.cgi?id=381442
https://bugs.kde.org/show_bug.cgi?id=315983
https://bugs.kde.org/show_bug.cgi?id=368951
https://bugs.kde.org/show_bug.cgi?id=365131
Comment Actions
KIconLoader does, but even Qt's own ones QSvgIconEngine and QPixmapIconEngine don't currently.
I did fix QPixmapIcon and QSvgIconEngine,
This should mean we can change QIcon::fromTheme to check isNull() which will then implicitly solve this.
Comment Actions
Yeah, putting workarounds everywhere isn't so appealing. My preference would be for a Qt fix, otherwise we'll be playing whack-a-mole here forever.