Correct QStandardItemModel::setItemData to follow QAbstractItemModel

QStandardItemModel::setItemData replaces the content of an item data
with the new values rather than updating/inserting which is
the behavior for QAbstractItemModel. This patch aims to unify the
behavior.

[ChangeLog][QtWidgets][QStandardItemModel] Fixed setItemData()
incorrectly deleting unmodified data. That behavior is not
following QAbstractItemModel's documented behavior which is
no modification of data not provided in parameter.

Task-number: QTBUG-45114
Task-number: QTBUG-10872
Change-Id: I2be40cee372b68d9f71c976548ecda6dc3011241
Reviewed-by: David Faure <david.faure@kdab.com>
bb10
Samuel Gaist 2017-10-05 16:20:36 +02:00
parent d0a0a3c041
commit 1382374dea
3 changed files with 157 additions and 11 deletions

View File

@ -185,6 +185,72 @@ void QStandardItemPrivate::childDeleted(QStandardItem *child)
emit model->dataChanged(modelIndex, modelIndex);
}
namespace {
struct ByNormalizedRole
{
static int normalizedRole(int role)
{
return role == Qt::EditRole ? Qt::DisplayRole : role;
}
bool operator()(const QStandardItemData& standardItemData, const std::pair<const int &, const QVariant&>& roleMapIt) const
{
return standardItemData.role < normalizedRole(roleMapIt.first);
}
bool operator()(const std::pair<const int&, const QVariant &>& roleMapIt, const QStandardItemData& standardItemData) const
{
return normalizedRole(roleMapIt.first) < standardItemData.role;
}
};
/*
Based on std::transform with a twist. The inputs are iterators of <int, QVariant> pair.
The variant is checked for validity and if not valid, that element is not taken into account
which means that the resulting output might be shorter than the input.
*/
template<class Input, class OutputIt>
OutputIt roleMapStandardItemDataTransform(Input first1, Input last1, OutputIt d_first)
{
while (first1 != last1) {
if ((*first1).second.isValid())
*d_first++ = QStandardItemData(*first1);
++first1;
}
return d_first;
}
/*
Based on std::set_union with a twist. The idea is to create a union of both inputs
with an additional constraint: if an input contains an invalid variant, it means
that this one should not be taken into account for generating the output.
*/
template<class Input1, class Input2,
class OutputIt, class Compare>
OutputIt roleMapStandardItemDataUnion(Input1 first1, Input1 last1,
Input2 first2, Input2 last2,
OutputIt d_first, Compare comp)
{
for (; first1 != last1; ++d_first) {
if (first2 == last2) {
return roleMapStandardItemDataTransform(first1, last1, d_first);
}
if (comp(*first2, *first1)) {
*d_first = *first2++;
} else {
if ((*first1).second.isValid())
*d_first = QStandardItemData(*first1);
if (!comp(*first1, *first2))
++first2;
++first1;
}
}
return std::copy(first2, last2, d_first);
}
}
/*!
\internal
*/
@ -192,18 +258,25 @@ void QStandardItemPrivate::setItemData(const QMap<int, QVariant> &roles)
{
Q_Q(QStandardItem);
//let's build the vector of new values
QVector<QStandardItemData> newValues;
for (auto it = roles.begin(), end = roles.end(); it != end; ++it) {
const QVariant &value = it.value();
if (value.isValid()) {
int role = it.key();
role = (role == Qt::EditRole) ? Qt::DisplayRole : role;
newValues.append(QStandardItemData(role, value));
}
}
auto byRole = [](const QStandardItemData& item1, const QStandardItemData& item2) {
return item1.role < item2.role;
};
if (values!=newValues) {
std::sort(values.begin(), values.end(), byRole);
/*
Create a vector of QStandardItemData that will contain the original values
if the matching role is not contained in roles, the new value if it is and
if the new value is an invalid QVariant, it will be removed.
*/
QVector<QStandardItemData> newValues;
newValues.reserve(values.size());
roleMapStandardItemDataUnion(roles.keyValueBegin(),
roles.keyValueEnd(),
values.cbegin(), values.cend(),
std::back_inserter(newValues), ByNormalizedRole());
if (newValues != values) {
values.swap(newValues);
if (model)
model->d_func()->itemChanged(q);

View File

@ -61,6 +61,7 @@
#include <QtCore/qstack.h>
#include <QtCore/qvariant.h>
#include <QtCore/qvector.h>
#include <QtCore/qdebug.h>
QT_BEGIN_NAMESPACE
@ -69,6 +70,7 @@ class QStandardItemData
public:
inline QStandardItemData() : role(-1) {}
inline QStandardItemData(int r, const QVariant &v) : role(r), value(v) {}
inline QStandardItemData(const std::pair<const int&, const QVariant&> &p) : role(p.first), value(p.second) {}
int role;
QVariant value;
inline bool operator==(const QStandardItemData &other) const { return role == other.role && value == other.value; }
@ -91,6 +93,15 @@ inline QDataStream &operator<<(QDataStream &out, const QStandardItemData &data)
return out;
}
inline QDebug &operator<<(QDebug &debug, const QStandardItemData &data)
{
QDebugStateSaver saver(debug);
debug.nospace() << data.role
<< " "
<< data.value;
return debug.space();
}
#endif // QT_NO_DATASTREAM
class QStandardItemPrivate

View File

@ -130,6 +130,8 @@ private slots:
void getMimeDataWithInvalidModelIndex();
void supportedDragDropActions();
void taskQTBUG_45114_setItemData();
private:
QAbstractItemModel *m_model;
QPersistentModelIndex persistent;
@ -1701,5 +1703,65 @@ void tst_QStandardItemModel::supportedDragDropActions()
QCOMPARE(model.supportedDropActions(), Qt::CopyAction | Qt::MoveAction);
}
void tst_QStandardItemModel::taskQTBUG_45114_setItemData()
{
QStandardItemModel model;
QSignalSpy spy(&model, &QStandardItemModel::itemChanged);
QStandardItem *item = new QStandardItem("item");
item->setData(1, Qt::UserRole + 1);
item->setData(2, Qt::UserRole + 2);
model.appendRow(item);
QModelIndex index = item->index();
QCOMPARE(model.itemData(index).size(), 3);
QCOMPARE(spy.count(), 0);
QMap<int, QVariant> roles;
roles.insert(Qt::UserRole + 1, 1);
roles.insert(Qt::UserRole + 2, 2);
model.setItemData(index, roles);
QCOMPARE(spy.count(), 0);
roles.insert(Qt::UserRole + 1, 1);
roles.insert(Qt::UserRole + 2, 2);
roles.insert(Qt::UserRole + 3, QVariant());
model.setItemData(index, roles);
QCOMPARE(spy.count(), 0);
roles.clear();
roles.insert(Qt::UserRole + 1, 10);
roles.insert(Qt::UserRole + 3, 12);
model.setItemData(index, roles);
QCOMPARE(spy.count(), 1);
QMap<int, QVariant> itemRoles = model.itemData(index);
QCOMPARE(itemRoles.size(), 4);
QCOMPARE(itemRoles[Qt::UserRole + 1].toInt(), 10);
QCOMPARE(itemRoles[Qt::UserRole + 2].toInt(), 2);
QCOMPARE(itemRoles[Qt::UserRole + 3].toInt(), 12);
roles.clear();
roles.insert(Qt::UserRole + 3, 1);
model.setItemData(index, roles);
QCOMPARE(spy.count(), 2);
roles.clear();
roles.insert(Qt::UserRole + 3, QVariant());
model.setItemData(index, roles);
QCOMPARE(spy.count(), 3);
itemRoles = model.itemData(index);
QCOMPARE(itemRoles.size(), 3);
QVERIFY(!itemRoles.keys().contains(Qt::UserRole + 3));
}
QTEST_MAIN(tst_QStandardItemModel)
#include "tst_qstandarditemmodel.moc"