From 5cff7d2b679d48a247b4630cb9e3d5b04fab0b55 Mon Sep 17 00:00:00 2001
From: Gabriel de Dietrich <gabriel.dedietrich@qt.io>
Date: Mon, 31 Oct 2016 11:54:51 -0700
Subject: [PATCH] QComboBox: Prioritize the model font for popup items
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On Mac, we use QComboMenuDelegate specifically as
item delegate for the popup list. It happens that
the order of resolving the font for each item
individually would prioritize QComboBox's font
instead of whatever the assigned model's FontRole
would specify.

The fix only requires checking whether FontRole is
valid before falling back QComboBox's properties.

Change-Id: I7208ad1911b30cc52c826c1884a1e19f5acd9fb4
Task-number: QTBUG-56693
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
---
 src/widgets/widgets/qcombobox.cpp             | 14 ++--
 .../widgets/qcombobox/tst_qcombobox.cpp       | 74 +++++++++++++++++++
 2 files changed, 81 insertions(+), 7 deletions(-)

diff --git a/src/widgets/widgets/qcombobox.cpp b/src/widgets/widgets/qcombobox.cpp
index 52e7962109..4358e568bf 100644
--- a/src/widgets/widgets/qcombobox.cpp
+++ b/src/widgets/widgets/qcombobox.cpp
@@ -170,18 +170,18 @@ QStyleOptionMenuItem QComboMenuDelegate::getStyleOption(const QStyleOptionViewIt
     menuOption.menuRect = option.rect;
     menuOption.rect = option.rect;
 
-    // Make sure fonts set on the combo box also overrides the font for the popup menu.
-    if (mCombo->testAttribute(Qt::WA_SetFont)
+    // Make sure fonts set on the model or on the combo box, in
+    // that order, also override the font for the popup menu.
+    QVariant fontRoleData = index.data(Qt::FontRole);
+    if (fontRoleData.isValid()) {
+        menuOption.font = fontRoleData.value<QFont>();
+    } else if (mCombo->testAttribute(Qt::WA_SetFont)
             || mCombo->testAttribute(Qt::WA_MacSmallSize)
             || mCombo->testAttribute(Qt::WA_MacMiniSize)
             || mCombo->font() != qt_app_fonts_hash()->value("QComboBox", QFont())) {
         menuOption.font = mCombo->font();
     } else {
-        QVariant fontRoleData = index.data(Qt::FontRole);
-        if (fontRoleData.isValid())
-            menuOption.font = fontRoleData.value<QFont>();
-        else
-            menuOption.font = qt_app_fonts_hash()->value("QComboMenuItem", mCombo->font());
+        menuOption.font = qt_app_fonts_hash()->value("QComboMenuItem", mCombo->font());
     }
 
     menuOption.fontMetrics = QFontMetrics(menuOption.font);
diff --git a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp
index 816fe1faba..edaf033678 100644
--- a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp
+++ b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp
@@ -62,6 +62,7 @@
 #include <qstyleditemdelegate.h>
 #include <qstandarditemmodel.h>
 #include <qproxystyle.h>
+#include <qfont.h>
 
 static inline void setFrameless(QWidget *w)
 {
@@ -160,6 +161,7 @@ private slots:
     void respectChangedOwnershipOfItemView();
     void task_QTBUG_39088_inputMethodHints();
     void task_QTBUG_49831_scrollerNotActivated();
+    void task_QTBUG_56693_itemFontFromModel();
 };
 
 class MyAbstractItemDelegate : public QAbstractItemDelegate
@@ -3250,5 +3252,77 @@ void tst_QComboBox::task_QTBUG_49831_scrollerNotActivated()
     }
 }
 
+class QTBUG_56693_Model : public QStandardItemModel
+{
+public:
+    QTBUG_56693_Model(QObject *parent = Q_NULLPTR)
+        : QStandardItemModel(parent)
+    { }
+
+    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override
+    {
+        if (role == Qt::FontRole) {
+            if (index.row() < 5) {
+                QFont font = QApplication::font();
+                font.setItalic(true);
+                return font;
+            } else {
+                return QApplication::font();
+            }
+        }
+        return QStandardItemModel::data(index, role);
+    }
+};
+
+class QTBUG_56693_ProxyStyle : public QProxyStyle
+{
+public:
+    QTBUG_56693_ProxyStyle(QStyle *style)
+        : QProxyStyle(style), italicItemsNo(0)
+    {
+
+    }
+
+    void drawControl(ControlElement element, const QStyleOption *opt, QPainter *p, const QWidget *w = Q_NULLPTR) const override
+    {
+        if (element == CE_MenuItem)
+            if (const QStyleOptionMenuItem *menuItem = qstyleoption_cast<const QStyleOptionMenuItem *>(opt))
+                if (menuItem->font.italic())
+                    italicItemsNo++;
+
+        baseStyle()->drawControl(element, opt, p, w);
+    }
+
+    mutable int italicItemsNo;
+};
+
+void tst_QComboBox::task_QTBUG_56693_itemFontFromModel()
+{
+    QComboBox box;
+    if (!qobject_cast<QComboMenuDelegate *>(box.itemDelegate()))
+        QSKIP("Only for combo boxes using QComboMenuDelegate");
+
+    QTBUG_56693_Model model;
+    box.setModel(&model);
+
+    QTBUG_56693_ProxyStyle *proxyStyle = new QTBUG_56693_ProxyStyle(box.style());
+    box.setStyle(proxyStyle);
+    box.setFont(QApplication::font());
+
+    for (int i = 0; i < 10; i++)
+        box.addItem(QLatin1String("Item ") + QString::number(i));
+
+    box.show();
+    QTest::qWaitForWindowExposed(&box);
+    box.showPopup();
+    QFrame *container = box.findChild<QComboBoxPrivateContainer *>();
+    QVERIFY(container);
+    QTest::qWaitForWindowExposed(container);
+
+    QCOMPARE(proxyStyle->italicItemsNo, 5);
+
+    box.hidePopup();
+}
+
 QTEST_MAIN(tst_QComboBox)
 #include "tst_qcombobox.moc"