From ff835a5030f3ec0aaa91ad0332739ca3259804e8 Mon Sep 17 00:00:00 2001
From: Luca Beldi <v.ronin@yahoo.it>
Date: Thu, 6 Dec 2018 08:48:35 +0000
Subject: Fix QStringListModel::setData to check for actual changes

QStringListModel::setData documentation states that
"The dataChanged() signal is emitted if the item is changed."
This patch actually respects the doc. setData will check that the data
actually changed before sending the dataChanged signal.

[ChangeLog][QtCore][QStringListModel] setData will now emit the
dataChanged() signal only if the string set is different from
the one already contained in the model

Change-Id: I4308a6f3b4851203fb899c5e29a36076e0c32f2f
Reviewed-by: David Faure <david.faure@kdab.com>
---
 src/corelib/itemmodels/qstringlistmodel.cpp          |  5 ++++-
 .../qstringlistmodel/tst_qstringlistmodel.cpp        | 20 ++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/src/corelib/itemmodels/qstringlistmodel.cpp b/src/corelib/itemmodels/qstringlistmodel.cpp
index 0adaba9b9a..f92a0d6676 100644
--- a/src/corelib/itemmodels/qstringlistmodel.cpp
+++ b/src/corelib/itemmodels/qstringlistmodel.cpp
@@ -184,7 +184,10 @@ 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());
+        const QString valueString = value.toString();
+        if (lst.at(index.row()) == valueString)
+            return true;
+        lst.replace(index.row(), valueString);
         QVector<int> roles;
         roles.reserve(2);
         roles.append(Qt::DisplayRole);
diff --git a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
index 9a54c0a70d..1b40e77648 100644
--- a/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
+++ b/tests/auto/corelib/itemmodels/qstringlistmodel/tst_qstringlistmodel.cpp
@@ -81,6 +81,8 @@ private slots:
     void setData_emits_both_roles_data();
     void setData_emits_both_roles();
 
+    void setData_emits_on_change_only();
+
     void supportedDragDropActions();
 };
 
@@ -246,6 +248,24 @@ void tst_QStringListModel::setData_emits_both_roles()
              expected);
 }
 
+void tst_QStringListModel::setData_emits_on_change_only()
+{
+    QStringListModel model(QStringList{QStringLiteral("one"), QStringLiteral("two")});
+    QSignalSpy dataChangedSpy(&model, &QAbstractItemModel::dataChanged);
+    QVERIFY(dataChangedSpy.isValid());
+    const QModelIndex modelIdx = model.index(0, 0);
+    const QString newStringData = QStringLiteral("test");
+    QVERIFY(model.setData(modelIdx, newStringData));
+    QCOMPARE(dataChangedSpy.count(), 1);
+    const QList<QVariant> spyList = dataChangedSpy.takeFirst();
+    QCOMPARE(spyList.at(0).value<QModelIndex>(), modelIdx);
+    QCOMPARE(spyList.at(1).value<QModelIndex>(), modelIdx);
+    const QVector<int> expectedRoles{Qt::DisplayRole, Qt::EditRole};
+    QCOMPARE(spyList.at(2).value<QVector<int> >(), expectedRoles);
+    QVERIFY(model.setData(modelIdx, newStringData));
+    QVERIFY(dataChangedSpy.isEmpty());
+}
+
 void tst_QStringListModel::supportedDragDropActions()
 {
     QStringListModel model;
-- 
cgit v1.2.3