QListWidgetItem/QStandardItem: pass role to dataChanged() signal

QAbstractItemModel::dataChanged() gained an optional role parameter
with Qt5 which was not filled within QListWidgetItem/QStandardItem
setData() functions

Task-number: QTBUG-55903
Task-number: QTBUG-63766
Change-Id: I4da9346ef8401cc8633dc4b2ea7d00451d1e3942
Reviewed-by: Luca Beldi <v.ronin@yahoo.it>
Reviewed-by: Thorbjørn Lund Martsum <tmartsum@gmail.com>

src/gui/itemmodels/qstandarditemmodel.cpp

@@ -278,8 +278,24 @@ void QStandardItemPrivate::setItemData(const QMap<int, QVariant> &roles)
 
     if (newValues != values) {
         values.swap(newValues);
-        if (model)
+        if (model) {
-            model->d_func()->itemChanged(q);
+            QVector<int> roleKeys;
+            roleKeys.reserve(roles.size() + 1);
+            bool hasEditRole = false;
+            bool hasDisplayRole = false;
+            for (auto it = roles.keyBegin(); it != roles.keyEnd(); ++it) {
+                roleKeys.push_back(*it);
+                if (*it == Qt::EditRole)
+                    hasEditRole = true;
+                else if (*it == Qt::DisplayRole)
+                    hasDisplayRole = true;
+            }
+            if (hasEditRole && !hasDisplayRole)
+                roleKeys.push_back(Qt::DisplayRole);
+            else if (!hasEditRole && hasDisplayRole)
+                roleKeys.push_back(Qt::EditRole);
+            model->d_func()->itemChanged(q, roleKeys);
+        }
     }
 }
 
@@ -554,7 +570,7 @@ bool QStandardItemPrivate::insertColumns(int column, int count, const QList<QSta
 /*!
   \internal
 */
-void QStandardItemModelPrivate::itemChanged(QStandardItem *item)
+void QStandardItemModelPrivate::itemChanged(QStandardItem *item, const QVector<int> &roles)
 {
     Q_Q(QStandardItemModel);
     Q_ASSERT(item);
@@ -570,8 +586,8 @@ void QStandardItemModelPrivate::itemChanged(QStandardItem *item)
         }
     } else {
         // Normal item
-        QModelIndex index = q->indexFromItem(item);
+        const QModelIndex index = q->indexFromItem(item);
-        emit q->dataChanged(index, index);
+        emit q->dataChanged(index, index, roles);
     }
 }
 
@@ -885,6 +901,9 @@ void QStandardItem::setData(const QVariant &value, int role)
 {
     Q_D(QStandardItem);
     role = (role == Qt::EditRole) ? Qt::DisplayRole : role;
+    const QVector<int> roles((role == Qt::DisplayRole) ?
+                                QVector<int>({Qt::DisplayRole, Qt::EditRole}) :
+                                QVector<int>({role}));
     QVector<QStandardItemData>::iterator it;
     for (it = d->values.begin(); it != d->values.end(); ++it) {
         if ((*it).role == role) {
@@ -896,13 +915,13 @@ void QStandardItem::setData(const QVariant &value, int role)
                 d->values.erase(it);
             }
             if (d->model)
-                d->model->d_func()->itemChanged(this);
+                d->model->d_func()->itemChanged(this, roles);
             return;
         }
     }
     d->values.append(QStandardItemData(role, value));
     if (d->model)
-        d->model->d_func()->itemChanged(this);
+        d->model->d_func()->itemChanged(this, roles);
 }
 
 /*!

src/gui/itemmodels/qstandarditemmodel.h

@@ -419,6 +419,7 @@ public:
     bool dropMimeData (const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) override;
 
 Q_SIGNALS:
+    // ### Qt 6: add changed roles
     void itemChanged(QStandardItem *item);
 
 protected:

src/gui/itemmodels/qstandarditemmodel_p.h

@@ -200,7 +200,7 @@ public:
     }
 
     void sort(QStandardItem *parent, int column, Qt::SortOrder order);
-    void itemChanged(QStandardItem *item);
+    void itemChanged(QStandardItem *item, const QVector<int> &roles = QVector<int>());
     void rowsAboutToBeInserted(QStandardItem *parent, int start, int end);
     void columnsAboutToBeInserted(QStandardItem *parent, int start, int end);
     void rowsAboutToBeRemoved(QStandardItem *parent, int start, int end);

src/widgets/itemviews/qlistwidget.cpp

@@ -412,10 +412,10 @@ QList<QListWidgetItem*>::iterator QListModel::sortedInsertionIterator(
     return std::lower_bound(begin, end, item, QListModelGreaterThan());
 }
 
-void QListModel::itemChanged(QListWidgetItem *item)
+void QListModel::itemChanged(QListWidgetItem *item, const QVector<int> &roles)
 {
-    QModelIndex idx = index(item);
+    const QModelIndex idx = index(item);
-    emit dataChanged(idx, idx);
+    emit dataChanged(idx, idx, roles);
 }
 
 QStringList QListModel::mimeTypes() const
@@ -711,8 +711,12 @@ void QListWidgetItem::setData(int role, const QVariant &value)
     }
     if (!found)
         d->values.append(QWidgetItemData(role, value));
-    if (QListModel *model = (view ? qobject_cast<QListModel*>(view->model()) : 0))
+    if (QListModel *model = (view ? qobject_cast<QListModel*>(view->model()) : nullptr)) {
-        model->itemChanged(this);
+        const QVector<int> roles((role == Qt::DisplayRole) ?
+                                    QVector<int>({Qt::DisplayRole, Qt::EditRole}) :
+                                    QVector<int>({role}));
+        model->itemChanged(this, roles);
+    }
 }
 
 /*!
@@ -954,7 +958,8 @@ QDataStream &operator>>(QDataStream &in, QListWidgetItem &item)
 
     \sa Qt::ItemFlags
 */
-void QListWidgetItem::setFlags(Qt::ItemFlags aflags) {
+void QListWidgetItem::setFlags(Qt::ItemFlags aflags)
+{
     itemFlags = aflags;
     if (QListModel *model = (view ? qobject_cast<QListModel*>(view->model()) : 0))
         model->itemChanged(this);

src/widgets/itemviews/qlistwidget.h

@@ -269,6 +269,7 @@ Q_SIGNALS:
     void itemDoubleClicked(QListWidgetItem *item);
     void itemActivated(QListWidgetItem *item);
     void itemEntered(QListWidgetItem *item);
+    // ### Qt 6: add changed roles
     void itemChanged(QListWidgetItem *item);
 
     void currentItemChanged(QListWidgetItem *current, QListWidgetItem *previous);

src/widgets/itemviews/qlistwidget_p.h

@@ -119,7 +119,7 @@ public:
         const QList<QListWidgetItem*>::iterator &end,
         Qt::SortOrder order, QListWidgetItem *item);
 
-    void itemChanged(QListWidgetItem *item);
+    void itemChanged(QListWidgetItem *item, const QVector<int> &roles = QVector<int>());
 
     // dnd
     QStringList mimeTypes() const override;

tests/auto/corelib/itemmodels/qsortfilterproxymodel_recursive/tst_qsortfilterproxymodel_recursive.cpp

@@ -34,6 +34,8 @@
 
 Q_DECLARE_METATYPE(QModelIndex)
 
+static const int s_filterRole = Qt::UserRole + 1;
+
 class ModelSignalSpy : public QObject {
     Q_OBJECT
 public:
@@ -67,7 +69,7 @@ private Q_SLOTS:
         mSignals << QStringLiteral("rowsMoved");
     }
     void onDataChanged(const QModelIndex &from, const QModelIndex& ) {
-        mSignals << QStringLiteral("dataChanged(%1)").arg(from.data().toString());
+        mSignals << QStringLiteral("dataChanged(%1)").arg(from.data(Qt::DisplayRole).toString());
     }
     void onLayoutChanged() {
         mSignals << QStringLiteral("layoutChanged");
@@ -78,7 +80,7 @@ private Q_SLOTS:
 private:
     QString textForRowSpy(const QModelIndex &parent, int start, int end)
     {
-        QString txt = parent.data().toString();
+        QString txt = parent.data(Qt::DisplayRole).toString();
         if (!txt.isEmpty())
             txt += QLatin1Char('.');
         txt += QString::number(start+1);
@@ -95,13 +97,14 @@ public:
     TestModel(QAbstractItemModel *sourceModel)
         : QSortFilterProxyModel()
     {
+        setFilterRole(s_filterRole);
         setRecursiveFilteringEnabled(true);
         setSourceModel(sourceModel);
     }
 
     virtual bool filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const override
     {
-        return sourceModel()->index(sourceRow, 0, sourceParent).data(Qt::UserRole +1).toBool()
+        return sourceModel()->index(sourceRow, 0, sourceParent).data(s_filterRole).toBool()
                 && QSortFilterProxyModel::filterAcceptsRow(sourceRow, sourceParent);
     }
 };
@@ -114,7 +117,7 @@ public:
 // - - E
 // as a single string, englobing children in brackets, like this:
 // [A[B[C D] E]]
-// In addition, items that match the filtering (data(UserRole+1) == true) have a * after their value.
+// In addition, items that match the filtering (data(s_filterRole) == true) have a * after their value.
 static QString treeAsString(const QAbstractItemModel &model, const QModelIndex &parent = QModelIndex())
 {
     QString ret;
@@ -126,8 +129,8 @@ static QString treeAsString(const QAbstractItemModel &model, const QModelIndex &
                 ret += ' ';
             }
             const QModelIndex child = model.index(row, 0, parent);
-            ret += child.data().toString();
+            ret += child.data(Qt::DisplayRole).toString();
-            if (child.data(Qt::UserRole+1).toBool())
+            if (child.data(s_filterRole).toBool())
                 ret += QLatin1Char('*');
             ret += treeAsString(model, child);
         }
@@ -146,7 +149,7 @@ static void fillModel(QStandardItemModel &model, const QString &str)
         const QChar ch = str.at(i);
         if ((ch == '[' || ch == ']' || ch == ' ') && !data.isEmpty()) {
             if (data.endsWith('*')) {
-                item->setData(true, Qt::UserRole + 1);
+                item->setData(true, s_filterRole);
                 data.chop(1);
             }
             item->setText(data);
@@ -231,10 +234,10 @@ private Q_SLOTS:
 
         QCOMPARE(treeAsString(proxy), QStringLiteral("[1[1.1[ME*]]]"));
 
+        // filterRole is Qt::UserRole + 1, so parents are not checked and
+        // therefore no dataChanged for parents
         QCOMPARE(spy.mSignals, QStringList()
-                 << QStringLiteral("dataChanged(ME)")
+                 << QStringLiteral("dataChanged(ME)"));
-                 << QStringLiteral("dataChanged(1.1)")
-                 << QStringLiteral("dataChanged(1)"));
     }
 
     // Test changing a role that is unrelated to the filtering, in a hidden item.
@@ -319,8 +322,8 @@ private Q_SLOTS:
         ModelSignalSpy spy(proxy);
         // When changing the data on the designated item to show this row
         QStandardItem *itemToChange = itemByText(model, add);
-        QVERIFY(!itemToChange->data().toBool());
+        QVERIFY(!itemToChange->data(s_filterRole).toBool());
-        itemToChange->setData(true);
+        itemToChange->setData(true, s_filterRole);
 
         // The proxy should update as expected
         QCOMPARE(treeAsString(proxy), expectedProxyStr);
@@ -408,8 +411,8 @@ private Q_SLOTS:
 
         // When changing the data on the designated item to exclude this row again
         QStandardItem *itemToChange = itemByText(model, remove);
-        QVERIFY(itemToChange->data().toBool());
+        QVERIFY(itemToChange->data(s_filterRole).toBool());
-        itemToChange->setData(false);
+        itemToChange->setData(false, s_filterRole);
 
         // The proxy should update as expected
         QCOMPARE(treeAsString(proxy), expectedProxyStr);
@@ -431,7 +434,7 @@ private Q_SLOTS:
         ModelSignalSpy spy(proxy);
         QStandardItem *item_1_1_1 = model.item(0)->child(0)->child(0);
         QStandardItem *item_1_1_1_1 = new QStandardItem(QStringLiteral("1.1.1.1"));
-        item_1_1_1_1->setData(true);
+        item_1_1_1_1->setData(true, s_filterRole);
         item_1_1_1->appendRow(item_1_1_1_1);
         QCOMPARE(treeAsString(proxy), QStringLiteral("[1[1.1[1.1.1[1.1.1.1*]]]]"));
 
@@ -456,7 +459,7 @@ private Q_SLOTS:
         ModelSignalSpy spy(proxy);
         {
             QStandardItem *item_1_1_1_1 = new QStandardItem(QStringLiteral("1.1.1.1"));
-            item_1_1_1_1->setData(true);
+            item_1_1_1_1->setData(true, s_filterRole);
             QStandardItem *item_1_1_1 = model.item(0)->child(0)->child(0);
             item_1_1_1->appendRow(item_1_1_1_1);
         }
@@ -484,7 +487,7 @@ private Q_SLOTS:
         {
             QStandardItem *item_1_1_1 = new QStandardItem(QStringLiteral("1.1.1"));
             QStandardItem *item_1_1_1_1 = new QStandardItem(QStringLiteral("1.1.1.1"));
-            item_1_1_1_1->setData(true);
+            item_1_1_1_1->setData(true, s_filterRole);
             item_1_1_1->appendRow(item_1_1_1_1);
 
             QStandardItem *item_1_1 = model.item(0)->child(0);
@@ -511,7 +514,7 @@ private Q_SLOTS:
         {
             QStandardItem *item_1_1_2 = new QStandardItem(QStringLiteral("1.1.2"));
             QStandardItem *item_1_1_2_1 = new QStandardItem(QStringLiteral("1.1.2.1"));
-            item_1_1_2_1->setData(true);
+            item_1_1_2_1->setData(true, s_filterRole);
             item_1_1_2->appendRow(item_1_1_2_1);
 
             QStandardItem *item_1_1 = model.item(0)->child(0);
@@ -706,6 +709,7 @@ private Q_SLOTS:
         ModelSignalSpy spy(proxy);
 
         //qDebug() << "setFilterFixedString";
+        proxy.setFilterRole(Qt::DisplayRole);
         proxy.setFilterFixedString(filter);
 
         QCOMPARE(treeAsString(proxy), expectedProxyStr);

tests/auto/gui/itemmodels/qstandarditemmodel/tst_qstandarditemmodel.cpp

@@ -137,6 +137,7 @@ private:
     QVector<QModelIndex> rcParent;
     QVector<int> rcFirst;
     QVector<int> rcLast;
+    QVector<int> currentRoles;
 
     //return true if models have the same structure, and all child have the same text
     bool compareModels(QStandardItemModel *model1, QStandardItemModel *model2);
@@ -186,6 +187,12 @@ void tst_QStandardItemModel::init()
     connect(m_model, SIGNAL(columnsRemoved(QModelIndex,int,int)),
             this, SLOT(columnsRemoved(QModelIndex,int,int)));
 
+    connect(m_model, &QAbstractItemModel::dataChanged,
+            this, [this](const QModelIndex &, const QModelIndex &, const QVector<int> &roles)
+    {
+        currentRoles = roles;
+    });
+
     rcFirst.fill(-1);
     rcLast.fill(-1);
 }
@@ -712,15 +719,20 @@ void tst_QStandardItemModel::checkChildren()
 
 void tst_QStandardItemModel::data()
 {
+    currentRoles.clear();
     // bad args
     m_model->setData(QModelIndex(), "bla", Qt::DisplayRole);
+    QCOMPARE(currentRoles, {});
 
     QIcon icon;
     for (int r=0; r < m_model->rowCount(); ++r) {
         for (int c=0; c < m_model->columnCount(); ++c) {
             m_model->setData(m_model->index(r,c), "initialitem", Qt::DisplayRole);
+            QCOMPARE(currentRoles, QVector<int>({Qt::DisplayRole, Qt::EditRole}));
             m_model->setData(m_model->index(r,c), "tooltip", Qt::ToolTipRole);
+            QCOMPARE(currentRoles, {Qt::ToolTipRole});
             m_model->setData(m_model->index(r,c), icon, Qt::DecorationRole);
+            QCOMPARE(currentRoles, {Qt::DecorationRole});
         }
     }
 

tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp

@@ -1381,20 +1381,38 @@ void tst_QListWidget::changeDataWithSorting_data()
         << false;
 }
 
+class QListWidgetDataChanged : public QListWidget
+{
+    Q_OBJECT
+public:
+    using QListWidget::QListWidget;
+
+    void dataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight, const QVector<int> &roles) override
+    {
+        QListWidget::dataChanged(topLeft, bottomRight, roles);
+        currentRoles = roles;
+    }
+    QVector<int> currentRoles;
+};
+
 void tst_QListWidget::itemData()
 {
-    QListWidget widget;
+    QListWidgetDataChanged widget;
     QListWidgetItem item(&widget);
     item.setFlags(item.flags() | Qt::ItemIsEditable);
     item.setData(Qt::DisplayRole,  QString("0"));
+    QCOMPARE(widget.currentRoles, QVector<int>({Qt::DisplayRole, Qt::EditRole}));
     item.setData(Qt::CheckStateRole, Qt::PartiallyChecked);
-    item.setData(Qt::UserRole + 0, QString("1"));
+    QCOMPARE(widget.currentRoles, {Qt::CheckStateRole});
-    item.setData(Qt::UserRole + 1, QString("2"));
+    for (int i = 0; i < 4; ++i)
-    item.setData(Qt::UserRole + 2, QString("3"));
+    {
-    item.setData(Qt::UserRole + 3, QString("4"));
+        item.setData(Qt::UserRole + i, QString::number(i + 1));
+        QCOMPARE(widget.currentRoles, {Qt::UserRole + i});
+    }
     QMap<int, QVariant> flags = widget.model()->itemData(widget.model()->index(0, 0));
     QCOMPARE(flags.count(), 6);
-    QCOMPARE(flags[Qt::UserRole + 0].toString(), QString("1"));
+    for (int i = 0; i < 4; ++i)
+        QCOMPARE(flags[Qt::UserRole + i].toString(), QString::number(i + 1));
 }
 
 void tst_QListWidget::changeDataWithSorting()