From b57f743c469f16f0c9ad5a9f0182454b74deff97 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Wed, 3 Aug 2016 12:11:38 +0300
Subject: QStringListModel: fix dataChanged's roles parameter

In QStringListModel, the display and the edit roles are synonyms,
so when one is changed, the other changes with it. However, in
setData() we only emitted a vector with just the role that was
passed in by the user.

Fix by always passing both roles, regardless of which one was used
to set the data.

Change-Id: I498e7cb33796fae266901817b01ad85d861d4bb4
Reviewed-by: Olivier Goffart (Woboq GmbH) <ogoffart@woboq.com>
---
 src/corelib/itemmodels/qstringlistmodel.cpp        |  8 +++-
 .../tst_qsortfilterproxymodel.cpp                  | 10 ++++-
 .../qstringlistmodel/tst_qstringlistmodel.cpp      | 45 ++++++++++++++++++++++
 3 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/src/corelib/itemmodels/qstringlistmodel.cpp b/src/corelib/itemmodels/qstringlistmodel.cpp
index c6a1fac9c8..725b7356ea 100644
--- a/src/corelib/itemmodels/qstringlistmodel.cpp
+++ b/src/corelib/itemmodels/qstringlistmodel.cpp
@@ -181,7 +181,13 @@ bool QStringListModel::setData(const QModelIndex &index, const QVariant &value,
     if (index.row() >= 0 && index.row() < lst.size()
         && (role == Qt::EditRole || role == Qt::DisplayRole)) {
         lst.replace(index.row(), value.toString());
-        emit dataChanged(index, index, QVector<int>() << role);
+        QVector<int> roles;
+        roles.reserve(2);
+        roles.append(Qt::DisplayRole);
+        roles.append(Qt::EditRole);
+        emit dataChanged(index, index, roles);
+        // once Q_COMPILER_UNIFORM_INIT can be used, change to:
+        // emit dataChanged(index, index, {Qt::DisplayRole, Qt::EditRole});
         return true;
     }
     return false;
diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
index 5928ee8688..4dd0b19ce8 100644
--- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
+++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp
@@ -2134,17 +2134,23 @@ void tst_QSortFilterProxyModel::changeSourceDataForwardsRoles_qtbug35440()
 
     QModelIndex index;
 
+    // QStringListModel doesn't distinguish between edit and display roles,
+    // so changing one always changes the other, too.
+    QVector<int> expectedChangedRoles;
+    expectedChangedRoles.append(Qt::DisplayRole);
+    expectedChangedRoles.append(Qt::EditRole);
+
     index = model.index(0, 0);
     QVERIFY(index.isValid());
     model.setData(index, QStringLiteral("teststring"), Qt::DisplayRole);
     QCOMPARE(spy.length(), 1);
-    QCOMPARE(spy.at(0).at(2).value<QVector<int> >(), QVector<int>() << Qt::DisplayRole);
+    QCOMPARE(spy.at(0).at(2).value<QVector<int> >(), expectedChangedRoles);
 
     index = model.index(1, 0);
     QVERIFY(index.isValid());
     model.setData(index, QStringLiteral("teststring2"), Qt::EditRole);
     QCOMPARE(spy.length(), 2);
-    QCOMPARE(spy.at(1).at(2).value<QVector<int> >(), QVector<int>() << Qt::EditRole);
+    QCOMPARE(spy.at(1).at(2).value<QVector<int> >(), expectedChangedRoles);
 }
 
 void tst_QSortFilterProxyModel::sortFilterRole()
diff --git a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
index 53f3d9c0a2..60952616d5 100644
--- a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
+++ b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
@@ -40,6 +40,8 @@
 #include "qmodellistener.h"
 #include <qstringlistmodel.h>
 
+#include <algorithm>
+
 void QModelListener::rowsAboutToBeRemovedOrInserted(const QModelIndex & parent, int start, int end )
 {
     for (int i = 0; start + i <= end; i++) {
@@ -80,6 +82,9 @@ private slots:
 
     void rowsAboutToBeInserted_rowsInserted();
     void rowsAboutToBeInserted_rowsInserted_data();
+
+    void setData_emits_both_roles_data();
+    void setData_emits_both_roles();
 };
 
 void tst_QStringListModel::rowsAboutToBeRemoved_rowsRemoved_data()
@@ -216,5 +221,45 @@ void tst_QStringListModel::rowsAboutToBeInserted_rowsInserted()
     delete model;
 }
 
+void tst_QStringListModel::setData_emits_both_roles_data()
+{
+    QTest::addColumn<int>("row");
+    QTest::addColumn<QString>("data");
+    QTest::addColumn<int>("role");
+
+#define ROW(row, string, role) \
+    QTest::newRow(#row " -> " string) << row << QString(string) << int(Qt::role)
+    ROW(0, "1", EditRole);
+    ROW(1, "2", DisplayRole);
+#undef ROW
+}
+
+template <class C>
+C sorted(C c)
+{
+    std::sort(c.begin(), c.end());
+    return qMove(c);
+}
+
+void tst_QStringListModel::setData_emits_both_roles()
+{
+    QFETCH(int, row);
+    QFETCH(QString, data);
+    QFETCH(int, role);
+
+    QStringListModel model(QStringList() << "one" << "two");
+    QVector<int> expected;
+    expected.reserve(2);
+    expected.append(Qt::DisplayRole);
+    expected.append(Qt::EditRole);
+
+    QSignalSpy spy(&model, &QAbstractItemModel::dataChanged);
+    QVERIFY(spy.isValid());
+    model.setData(model.index(row, 0), data, role);
+    QCOMPARE(spy.size(), 1);
+    QCOMPARE(sorted(spy.at(0).at(2).value<QVector<int> >()),
+             expected);
+}
+
 QTEST_MAIN(tst_QStringListModel)
 #include "tst_qstringlistmodel.moc"
-- 
cgit v1.2.3