From f4dca7c512ab3f8860f711de971af1fea76a1665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Wed, 10 May 2023 10:25:02 +0200 Subject: [PATCH] Consult QIcon::fallbackThemeName() even for themes with explicit parents We would previously only use the fallback theme for themes that did not exist, or for themes that did not declare any parent theme. We now unconditionally use the fallback theme, even for themes that declare their own parent themes, so that a QIcon::fromTheme("foo") that doesn't exist in the current theme, nor any of its parents, nor in "hicolor", will still be looked up in the fallback theme. The reason this seemed to work in the existing tests was because our test themes inherit system themes such as crystalsvg and gnome, and we didn't provide a hicolor theme. Any of these themes missing would lead us into the code path where we use the fallback theme for a missing theme, masking that fact that we had not added the fallback theme to the list of fallbacks for the theme that had explicit parents declared. The logic has been moved out of the theme parsing and into an accessor in QIconTheme, so that we're not caching the fallback theme lookup. [ChangeLog][QtGui][QIcon] QIcon::fallbackThemeName() will now be used as fallback even for themes that declare a parent theme. Pick-to: 6.5 6.6 Change-Id: Ib0ce1dfe97030f23893460ed624073a719a3ebd1 Reviewed-by: Friedemann Kleint Reviewed-by: Axel Spoerl --- src/gui/image/qicon.cpp | 4 +++ src/gui/image/qiconloader.cpp | 28 +++++++++++------- src/gui/image/qiconloader_p.h | 2 +- tests/auto/gui/image/qicon/CMakeLists.txt | 2 ++ .../icons/hicolor/16x16/hicolor-icon.png | Bin 0 -> 267 bytes .../gui/image/qicon/icons/hicolor/index.theme | 11 +++++++ .../image/qicon/icons/testtheme/index.theme | 2 +- .../image/qicon/icons/themeparent/index.theme | 1 - 8 files changed, 36 insertions(+), 14 deletions(-) create mode 100644 tests/auto/gui/image/qicon/icons/hicolor/16x16/hicolor-icon.png create mode 100644 tests/auto/gui/image/qicon/icons/hicolor/index.theme diff --git a/src/gui/image/qicon.cpp b/src/gui/image/qicon.cpp index 1581ce5a797..3ebdd6ec23c 100644 --- a/src/gui/image/qicon.cpp +++ b/src/gui/image/qicon.cpp @@ -1233,6 +1233,10 @@ QString QIcon::fallbackThemeName() Sets the fallback icon theme to \a name. + The fallback icon theme is used for last resort lookup of icons + not provided by the \l{themeName()}{current icon theme}, + or if the \l{themeName()}{current icon theme} does not exist. + The \a name should correspond to a directory name in the themeSearchPath() containing an index.theme file describing its contents. diff --git a/src/gui/image/qiconloader.cpp b/src/gui/image/qiconloader.cpp index 10d5f65b805..2e3097ce56b 100644 --- a/src/gui/image/qiconloader.cpp +++ b/src/gui/image/qiconloader.cpp @@ -394,21 +394,27 @@ QIconTheme::QIconTheme(const QString &themeName) // Parent themes provide fallbacks for missing icons m_parents = indexReader.value("Icon Theme/Inherits"_L1).toStringList(); m_parents.removeAll(QString()); - - // Ensure a default platform fallback for all themes - if (m_parents.isEmpty()) { - const QString fallback = QIconLoader::instance()->fallbackThemeName(); - if (!fallback.isEmpty()) - m_parents.append(fallback); - } - - // Ensure that all themes fall back to hicolor - if (!m_parents.contains("hicolor"_L1)) - m_parents.append("hicolor"_L1); } #endif // settings } +QStringList QIconTheme::parents() const +{ + // Respect explicitly declared parents + QStringList result = m_parents; + + // Ensure a default fallback for all themes + const QString fallback = QIconLoader::instance()->fallbackThemeName(); + if (!fallback.isEmpty()) + result.append(fallback); + + // Ensure that all themes fall back to hicolor + if (!result.contains("hicolor"_L1)) + result.append("hicolor"_L1); + + return result; +} + QDebug operator<<(QDebug debug, const std::unique_ptr &entry) { QDebugStateSaver saver(debug); diff --git a/src/gui/image/qiconloader_p.h b/src/gui/image/qiconloader_p.h index 0b67dcdbad5..816fb7708cd 100644 --- a/src/gui/image/qiconloader_p.h +++ b/src/gui/image/qiconloader_p.h @@ -145,7 +145,7 @@ class QIconTheme public: QIconTheme(const QString &name); QIconTheme() : m_valid(false) {} - QStringList parents() { return m_parents; } + QStringList parents() const; QList keyList() { return m_keyList; } QStringList contentDirs() { return m_contentDirs; } bool isValid() { return m_valid; } diff --git a/tests/auto/gui/image/qicon/CMakeLists.txt b/tests/auto/gui/image/qicon/CMakeLists.txt index 2d212defb78..ce9528ae45e 100644 --- a/tests/auto/gui/image/qicon/CMakeLists.txt +++ b/tests/auto/gui/image/qicon/CMakeLists.txt @@ -58,6 +58,8 @@ set(tst_qicon_resource_files "./icons/themeparent/scalable/actions/appointment-new.svg" "./icons/fallbacktheme/index.theme" "./icons/fallbacktheme/16x16/edit-cut.png" + "./icons/hicolor/index.theme" + "./icons/hicolor/16x16/hicolor-icon.png" "./second_icons/testtheme/32x32/actions/appointment-new.png" "./styles/commonstyle/images/standardbutton-open-128.png" "./styles/commonstyle/images/standardbutton-open-16.png" diff --git a/tests/auto/gui/image/qicon/icons/hicolor/16x16/hicolor-icon.png b/tests/auto/gui/image/qicon/icons/hicolor/16x16/hicolor-icon.png new file mode 100644 index 0000000000000000000000000000000000000000..661ef1ad030c8889444d4bff3940e34267b0763a GIT binary patch literal 267 zcmeAS@N?(olHy`uVBq!ia0vp^0wB!61|;P_|4#%`oCO|{#S9GGLLkg|>2BR0px|Cl z7sn8b-nBv8qRj>Z-T@6Qo-WvkPeA22$!bT9sT=b0+MW7r+PfV{eR=grsmMZfdpyEV0&VP2eN zh0Nzu41NrG3~cW2%Z|I3Xdm+4*;Cd2K@sv2)XSnJ+B z_`vh8kb8HXih8F__Z8NV!orHl7j7I>