Models with a single role may not update due to "modelData" conflict.

Models with a single role also have a modelData property added.  These
role names both ended up in a hash, resulting in only one or the other
updating.  Now we handle modelData specially.

Task-number: QTBUG-13664
Reviewed-by: Michael Brasser

4 files changed, 134 insertions, 10 deletions

diff --git a/src/declarative/graphicsitems/qdeclarativevisualitemmodel.cpp b/src/declarative/graphicsitems/qdeclarativevisualitemmodel.cpp
index a46ee73..a70886e 100644
--- a/src/declarative/graphicsitems/qdeclarativevisualitemmodel.cpp
+++ b/src/declarative/graphicsitems/qdeclarativevisualitemmodel.cpp
@@ -287,16 +287,12 @@ public:
                 m_roles = m_listModelInterface->roles();
                 for (int ii = 0; ii < m_roles.count(); ++ii)
                     m_roleNames.insert(m_listModelInterface->toString(m_roles.at(ii)).toUtf8(), m_roles.at(ii));
-                if (m_roles.count() == 1)
-                    m_roleNames.insert("modelData", m_roles.at(0));
             } else if (m_abstractItemModel) {
                 for (QHash<int,QByteArray>::const_iterator it = m_abstractItemModel->roleNames().begin();
                         it != m_abstractItemModel->roleNames().end(); ++it) {
                     m_roles.append(it.key());
                     m_roleNames.insert(*it, it.key());
                 }
-                if (m_roles.count() == 1)
-                    m_roleNames.insert("modelData", m_roles.at(0));
                 if (m_roles.count())
                     m_roleNames.insert("hasModelChildren", -1);
             } else if (m_listAccessor) {
@@ -314,7 +310,8 @@ public:
         }
     }
 
-    QHash<int,int> roleToPropId;
+    QHash<int,int> m_roleToPropId;
+    int m_modelDataPropId;
     void createMetaData() {
         if (!m_metaDataCreated) {
             ensureRoles();
@@ -322,9 +319,12 @@ public:
                 QHash<QByteArray, int>::const_iterator it = m_roleNames.begin();
                 while (it != m_roleNames.end()) {
                     int propId = m_delegateDataType->createProperty(it.key()) - m_delegateDataType->propertyOffset();
-                    roleToPropId.insert(*it, propId);
+                    m_roleToPropId.insert(*it, propId);
                     ++it;
                 }
+                // Add modelData property
+                if (m_roles.count() == 1)
+                    m_modelDataPropId = m_delegateDataType->createProperty("modelData") - m_delegateDataType->propertyOffset();
                 m_metaDataCreated = true;
             }
         }
@@ -430,6 +430,11 @@ public:
     void setIndex(int index);
 
     int propForRole(int) const;
+    int modelDataPropertyId() const {
+        QDeclarativeVisualDataModelPrivate *model = QDeclarativeVisualDataModelPrivate::get(m_model);
+        return model->m_modelDataPropId;
+    }
+
     void setValue(int, const QVariant &);
     bool hasValue(int id) const {
         return m_meta->hasValue(id);
@@ -450,8 +455,8 @@ private:
 int QDeclarativeVisualDataModelData::propForRole(int id) const
 {
     QDeclarativeVisualDataModelPrivate *model = QDeclarativeVisualDataModelPrivate::get(m_model);
-    QHash<int,int>::const_iterator it = model->roleToPropId.find(id);
-    if (it != model->roleToPropId.end())
+    QHash<int,int>::const_iterator it = model->m_roleToPropId.find(id);
+    if (it != model->m_roleToPropId.end())
         return *it;
 
     return -1;
@@ -518,14 +523,16 @@ QVariant QDeclarativeVisualDataModelDataMetaObject::initialValue(int propId)
         }
     } else if (model->m_abstractItemModel) {
         model->ensureRoles();
+        QModelIndex index = model->m_abstractItemModel->index(data->m_index, 0, model->m_root);
         if (propName == "hasModelChildren") {
-            QModelIndex index = model->m_abstractItemModel->index(data->m_index, 0, model->m_root);
             return model->m_abstractItemModel->hasChildren(index);
         } else {
             QHash<QByteArray,int>::const_iterator it = model->m_roleNames.find(propName);
             if (it != model->m_roleNames.end()) {
-                QModelIndex index = model->m_abstractItemModel->index(data->m_index, 0, model->m_root);
                 return model->m_abstractItemModel->data(index, *it);
+            } else if (model->m_roles.count() == 1 && propName == "modelData") {
+                //for compatibility with other lists, assign modelData if there is only a single role
+                return model->m_abstractItemModel->data(index, model->m_roles.first());
             }
         }
     }
@@ -1195,6 +1202,19 @@ void QDeclarativeVisualDataModel::_q_itemsChanged(int index, int count,
                     qmlInfo(this) << "Changing role not present in item: " << roleName;
                 }
             }
+            if (roles.count() == 1) {
+                // Handle the modelData role we add if there is just one role.
+                int propId = data->modelDataPropertyId();
+                if (data->hasValue(propId)) {
+                    int role = roles.at(0);
+                    if (d->m_listModelInterface) {
+                        data->setValue(propId, d->m_listModelInterface->data(idx, QList<int>() << role).value(role));
+                    } else if (d->m_abstractItemModel) {
+                        QModelIndex index = d->m_abstractItemModel->index(idx, 0, d->m_root);
+                        data->setValue(propId, d->m_abstractItemModel->data(index, role));
+                    }
+                }
+            }
         }
     }
 }
diff --git a/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole1.qml b/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole1.qml
new file mode 100644
index 0000000..7ea74f2
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole1.qml
@@ -0,0 +1,11 @@
+import Qt 4.7
+
+ListView {
+    width: 100
+    height: 100
+    anchors.fill: parent
+    model: myModel
+    delegate: Component {
+        Text { objectName: "name"; text: name }
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole2.qml b/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole2.qml
new file mode 100644
index 0000000..6654d6b
--- /dev/null
+++ b/tests/auto/declarative/qdeclarativevisualdatamodel/data/singlerole2.qml
@@ -0,0 +1,11 @@
+import Qt 4.7
+
+ListView {
+    width: 100
+    height: 100
+    anchors.fill: parent
+    model: myModel
+    delegate: Component {
+        Text { objectName: "name"; text: modelData }
+    }
+}
diff --git a/tests/auto/declarative/qdeclarativevisualdatamodel/tst_qdeclarativevisualdatamodel.cpp b/tests/auto/declarative/qdeclarativevisualdatamodel/tst_qdeclarativevisualdatamodel.cpp
index 95ef4fc..d73a872 100644
--- a/tests/auto/declarative/qdeclarativevisualdatamodel/tst_qdeclarativevisualdatamodel.cpp
+++ b/tests/auto/declarative/qdeclarativevisualdatamodel/tst_qdeclarativevisualdatamodel.cpp
@@ -75,6 +75,39 @@ static void initStandardTreeModel(QStandardItemModel *model)
     model->insertRow(2, item);
 }
 
+class SingleRoleModel : public QAbstractListModel
+{
+    Q_OBJECT
+
+public:
+    SingleRoleModel(QObject *parent = 0) {
+        QHash<int, QByteArray> roles;
+        roles.insert(Qt::DisplayRole , "name");
+        setRoleNames(roles);
+        list << "one" << "two" << "three" << "four";
+    }
+
+public slots:
+    void set(int idx, QString string) {
+        list[idx] = string;
+        emit dataChanged(index(idx,0), index(idx,0));
+    }
+
+protected:
+    int rowCount(const QModelIndex &parent = QModelIndex()) const {
+        return list.count();
+    }
+    QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const {
+        if (role == Qt::DisplayRole)
+            return list.at(index.row());
+        return QVariant();
+    }
+
+private:
+    QStringList list;
+};
+
+
 class tst_qdeclarativevisualdatamodel : public QObject
 {
     Q_OBJECT
@@ -86,6 +119,7 @@ private slots:
     void updateLayout();
     void childChanged();
     void objectListModel();
+    void singleRole();
 
 private:
     QDeclarativeEngine engine;
@@ -282,6 +316,54 @@ void tst_qdeclarativevisualdatamodel::objectListModel()
     QCOMPARE(name->text(), QString("Changed"));
 }
 
+void tst_qdeclarativevisualdatamodel::singleRole()
+{
+    {
+        QDeclarativeView view;
+
+        SingleRoleModel model;
+
+        QDeclarativeContext *ctxt = view.rootContext();
+        ctxt->setContextProperty("myModel", &model);
+
+        view.setSource(QUrl::fromLocalFile(SRCDIR "/data/singlerole1.qml"));
+
+        QDeclarativeListView *listview = qobject_cast<QDeclarativeListView*>(view.rootObject());
+        QVERIFY(listview != 0);
+
+        QDeclarativeItem *contentItem = listview->contentItem();
+        QVERIFY(contentItem != 0);
+
+        QDeclarativeText *name = findItem<QDeclarativeText>(contentItem, "name", 1);
+        QCOMPARE(name->text(), QString("two"));
+
+        model.set(1, "Changed");
+        QCOMPARE(name->text(), QString("Changed"));
+    }
+    {
+        QDeclarativeView view;
+
+        SingleRoleModel model;
+
+        QDeclarativeContext *ctxt = view.rootContext();
+        ctxt->setContextProperty("myModel", &model);
+
+        view.setSource(QUrl::fromLocalFile(SRCDIR "/data/singlerole2.qml"));
+
+        QDeclarativeListView *listview = qobject_cast<QDeclarativeListView*>(view.rootObject());
+        QVERIFY(listview != 0);
+
+        QDeclarativeItem *contentItem = listview->contentItem();
+        QVERIFY(contentItem != 0);
+
+        QDeclarativeText *name = findItem<QDeclarativeText>(contentItem, "name", 1);
+        QCOMPARE(name->text(), QString("two"));
+
+        model.set(1, "Changed");
+        QCOMPARE(name->text(), QString("Changed"));
+    }
+}
+
 template<typename T>
 T *tst_qdeclarativevisualdatamodel::findItem(QGraphicsObject *parent, const QString &objectName, int index)
 {