Ensure styles always get to polish the application palette

Before 0a93db4d82c051164923a10e4382b12de9049b45 we would polish the
application palette even when it was the default palette, as we
always recreated the system palette each time a style was set.

After the change we skipped polishing the palette unless it was set
by the user, under the assumption that the style would set its
own default palette if it wanted to override the system palette.

This turned out to break the style's ability to slightly tweak the
palette via polish (versus the more full on standardPalette approach).

We now polish both the default palette and user palettes, and we do
so as part of the normal palette update logic. This ensures that
the style also gets a chance to polish the palette when the platform
theme changes.

The polish will not have an effect on the resolve mask of the palette,
as the polish is conceptually the same as a base palette, and should
not affect e.g. Qt::AA_SetPalette.

Fixes: QTBUG-85469
Fixes: QTBUG-85188
Pick-to: 5.15
Change-Id: I869e9c442b177de4f1dc49eb75220709306f4d12
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

4 files changed, 107 insertions, 18 deletions

diff --git a/src/gui/kernel/qguiapplication_p.h b/src/gui/kernel/qguiapplication_p.h
index b469c94319..ddea50e3d0 100644
--- a/src/gui/kernel/qguiapplication_p.h
+++ b/src/gui/kernel/qguiapplication_p.h
@@ -329,6 +329,8 @@ public:
     virtual QShortcutPrivate *createShortcutPrivate() const;
 #endif
 
+    static void updatePalette();
+
 protected:
     virtual void notifyThemeChanged();
 
@@ -343,7 +345,6 @@ protected:
 
 private:
     static void clearPalette();
-    static void updatePalette();
 
     friend class QDragManager;
 
diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index 71e5ade234..f19a9d2e8e 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -947,10 +947,7 @@ QStyle *QApplication::style()
         // Take ownership of the style
         defaultStyle->setParent(qApp);
 
-        if (testAttribute(Qt::AA_SetPalette))
-            defaultStyle->polish(*QGuiApplicationPrivate::app_pal);
-        else
-            QApplicationPrivate::initializeWidgetPalettesFromTheme();
+        QGuiApplicationPrivate::updatePalette();
 
 #ifndef QT_NO_STYLE_STYLESHEET
         if (!QApplicationPrivate::styleSheet.isEmpty()) {
@@ -1021,13 +1018,10 @@ void QApplication::setStyle(QStyle *style)
         QApplicationPrivate::app_style = style;
     QApplicationPrivate::app_style->setParent(qApp); // take ownership
 
-    // take care of possible palette requirements of certain gui
-    // styles. Do it before polishing the application since the style
-    // might call QApplication::setPalette() itself
-    if (testAttribute(Qt::AA_SetPalette))
-        QApplicationPrivate::app_style->polish(*QGuiApplicationPrivate::app_pal);
-    else
-        QApplicationPrivate::initializeWidgetPalettesFromTheme();
+    // Take care of possible palette requirements of certain
+    // styles. Do it before polishing the application since the
+    // style might call QApplication::setPalette() itself.
+    QGuiApplicationPrivate::updatePalette();
 
     // The default widget font hash is based on the platform theme,
     // not the style, but the widget fonts could in theory have been
@@ -1121,6 +1115,13 @@ QPalette QApplicationPrivate::basePalette() const
     if (const QPalette *themePalette = platformTheme() ? platformTheme()->palette() : nullptr)
         palette = themePalette->resolve(palette);
 
+    // Finish off by letting the application style polish the palette. This will
+    // not result in the polished palette becoming a user-set palette, as the
+    // resulting base palette is only used as a fallback, with the resolve mask
+    // set to 0.
+    if (app_style)
+        app_style->polish(palette);
+
     return palette;
 }
 
@@ -1194,17 +1195,19 @@ QPalette QApplication::palette(const char *className)
 */
 void QApplication::setPalette(const QPalette &palette, const char* className)
 {
-    QPalette polishedPalette = palette;
-
-    if (QApplicationPrivate::app_style)
-        QApplicationPrivate::app_style->polish(polishedPalette);
-
     if (className) {
+        QPalette polishedPalette = palette;
+        if (QApplicationPrivate::app_style) {
+            auto originalResolveMask = palette.resolve();
+            QApplicationPrivate::app_style->polish(polishedPalette);
+            polishedPalette.resolve(originalResolveMask);
+        }
+
         QApplicationPrivate::widgetPalettes.insert(className, polishedPalette);
         if (qApp)
             qApp->d_func()->handlePaletteChanged(className);
     } else {
-        QGuiApplication::setPalette(polishedPalette);
+        QGuiApplication::setPalette(palette);
     }
 }
 
@@ -1408,6 +1411,12 @@ void QApplicationPrivate::setSystemFont(const QFont &font)
 */
 QString QApplicationPrivate::desktopStyleKey()
 {
+#if defined(QT_BUILD_INTERNAL)
+    // Allow auto-tests to override the desktop style
+    if (qEnvironmentVariableIsSet("QT_DESKTOP_STYLE_KEY"))
+        return QString::fromLocal8Bit(qgetenv("QT_DESKTOP_STYLE_KEY"));
+#endif
+
     // The platform theme might return a style that is not available, find
     // first valid one.
     if (const QPlatformTheme *theme = QGuiApplicationPrivate::platformTheme()) {
diff --git a/tests/auto/widgets/kernel/qapplication/customstyle.json b/tests/auto/widgets/kernel/qapplication/customstyle.json
new file mode 100644
index 0000000000..35b80f636c
--- /dev/null
+++ b/tests/auto/widgets/kernel/qapplication/customstyle.json
@@ -0,0 +1,3 @@
+{
+    "Keys": [ "customstyle" ]
+}
diff --git a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
index 8a2297c359..a62f6c5699 100644
--- a/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
+++ b/tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp
@@ -26,6 +26,8 @@
 **
 ****************************************************************************/
 
+#define QT_STATICPLUGIN
+#include <QtWidgets/qstyleplugin.h>
 
 #include <qdebug.h>
 
@@ -54,6 +56,7 @@
 #include <QtWidgets/QScrollBar>
 #include <QtWidgets/private/qapplication_p.h>
 #include <QtWidgets/QStyle>
+#include <QtWidgets/qproxystyle.h>
 
 #include <qpa/qwindowsysteminterface.h>
 #include <qpa/qwindowsysteminterface_p.h>
@@ -128,6 +131,7 @@ private slots:
     void task109149();
 
     void style();
+    void applicationPalettePolish();
 
     void allWidgets();
     void topLevelWidgets();
@@ -1798,6 +1802,78 @@ void tst_QApplication::style()
     QVERIFY(QApplication::style() != nullptr);
 }
 
+class CustomStyle : public QProxyStyle
+{
+public:
+    CustomStyle() : QProxyStyle("Windows") { Q_ASSERT(!polished); }
+    ~CustomStyle() { polished = 0; }
+    void polish(QPalette &palette)
+    {
+        polished++;
+        palette.setColor(QPalette::Active, QPalette::Link, Qt::red);
+    }
+    static int polished;
+};
+
+int CustomStyle::polished = 0;
+
+class CustomStylePlugin : public QStylePlugin
+{
+    Q_OBJECT
+    Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QStyleFactoryInterface" FILE "customstyle.json")
+public:
+    QStyle *create(const QString &) { return new CustomStyle; }
+};
+
+Q_IMPORT_PLUGIN(CustomStylePlugin)
+
+void tst_QApplication::applicationPalettePolish()
+{
+    int argc = 1;
+
+#if defined(QT_BUILD_INTERNAL)
+    {
+        qputenv("QT_DESKTOP_STYLE_KEY", "customstyle");
+        QApplication app(argc, &argv0);
+        QVERIFY(CustomStyle::polished);
+        QVERIFY(!app.palette().resolve());
+        QCOMPARE(app.palette().color(QPalette::Link), Qt::red);
+        qunsetenv("QT_DESKTOP_STYLE_KEY");
+    }
+#endif
+
+    {
+        QApplication::setStyle(new CustomStyle);
+        QApplication app(argc, &argv0);
+        QVERIFY(CustomStyle::polished);
+        QVERIFY(!app.palette().resolve());
+        QCOMPARE(app.palette().color(QPalette::Link), Qt::red);
+    }
+
+    {
+        QApplication app(argc, &argv0);
+        app.setStyle(new CustomStyle);
+        QVERIFY(CustomStyle::polished);
+        QVERIFY(!app.palette().resolve());
+        QCOMPARE(app.palette().color(QPalette::Link), Qt::red);
+
+        CustomStyle::polished = 0;
+        app.setPalette(QPalette());
+        QVERIFY(CustomStyle::polished);
+        QVERIFY(!app.palette().resolve());
+        QCOMPARE(app.palette().color(QPalette::Link), Qt::red);
+
+        CustomStyle::polished = 0;
+        QPalette palette;
+        palette.setColor(QPalette::Active, QPalette::Highlight, Qt::green);
+        app.setPalette(palette);
+        QVERIFY(CustomStyle::polished);
+        QVERIFY(app.palette().resolve());
+        QCOMPARE(app.palette().color(QPalette::Link), Qt::red);
+        QCOMPARE(app.palette().color(QPalette::Highlight), Qt::green);
+    }
+}
+
 void tst_QApplication::allWidgets()
 {
     int argc = 1;