From 736ac191565196514e14f875c774771026f95d7e Mon Sep 17 00:00:00 2001 From: David Faure Date: Tue, 18 Nov 2014 21:47:41 +0100 Subject: QAbstractProxyModel: fix canDropMimeData/dropMimeData implementations The code in 4696e9dbaa4 was incorrect. It is perfectly valid to call these methods with row=-1 column=1 parent=some_index, this is exactly what happens in QListView and QTableView. Child row/column is only for trees. Move the coordinate mapping from QSortFilterProxyModel into a new mapDropCoordinatesToSource internal method, used by QAbstractProxyModel. Task-number: QTBUG-39549 Change-Id: I3312210473d84b639cbe4c01f70ea36437db3e91 Reviewed-by: Friedemann Kleint Reviewed-by: Stephen Kelly --- src/corelib/itemmodels/qabstractproxymodel.cpp | 34 +++++++++++-- src/corelib/itemmodels/qabstractproxymodel_p.h | 2 + src/corelib/itemmodels/qsortfilterproxymodel.cpp | 20 +------- .../tst_qsortfilterproxymodel.cpp | 56 ++++++++++++++++++++++ 4 files changed, 90 insertions(+), 22 deletions(-) diff --git a/src/corelib/itemmodels/qabstractproxymodel.cpp b/src/corelib/itemmodels/qabstractproxymodel.cpp index ce26293183..b7f988ef7c 100644 --- a/src/corelib/itemmodels/qabstractproxymodel.cpp +++ b/src/corelib/itemmodels/qabstractproxymodel.cpp @@ -384,6 +384,26 @@ QMimeData* QAbstractProxyModel::mimeData(const QModelIndexList &indexes) const return d->model->mimeData(list); } +void QAbstractProxyModelPrivate::mapDropCoordinatesToSource(int row, int column, const QModelIndex &parent, + int *sourceRow, int *sourceColumn, QModelIndex *sourceParent) const +{ + Q_Q(const QAbstractProxyModel); + *sourceRow = -1; + *sourceColumn = -1; + if (row == -1 && column == -1) { + *sourceParent = q->mapToSource(parent); + } else if (row == q->rowCount(parent)) { + *sourceParent = q->mapToSource(parent); + *sourceRow = model->rowCount(*sourceParent); + } else { + QModelIndex proxyIndex = q->index(row, column, parent); + QModelIndex sourceIndex = q->mapToSource(proxyIndex); + *sourceRow = sourceIndex.row(); + *sourceColumn = sourceIndex.column(); + *sourceParent = sourceIndex.parent(); + } +} + /*! \reimp \since 5.4 @@ -392,8 +412,11 @@ bool QAbstractProxyModel::canDropMimeData(const QMimeData *data, Qt::DropAction int row, int column, const QModelIndex &parent) const { Q_D(const QAbstractProxyModel); - const QModelIndex source = mapToSource(index(row, column, parent)); - return d->model->canDropMimeData(data, action, source.row(), source.column(), source.parent()); + int sourceDestinationRow; + int sourceDestinationColumn; + QModelIndex sourceParent; + d->mapDropCoordinatesToSource(row, column, parent, &sourceDestinationRow, &sourceDestinationColumn, &sourceParent); + return d->model->canDropMimeData(data, action, sourceDestinationRow, sourceDestinationColumn, sourceParent); } /*! @@ -404,8 +427,11 @@ bool QAbstractProxyModel::dropMimeData(const QMimeData *data, Qt::DropAction act int row, int column, const QModelIndex &parent) { Q_D(QAbstractProxyModel); - const QModelIndex source = mapToSource(index(row, column, parent)); - return d->model->dropMimeData(data, action, source.row(), source.column(), source.parent()); + int sourceDestinationRow; + int sourceDestinationColumn; + QModelIndex sourceParent; + d->mapDropCoordinatesToSource(row, column, parent, &sourceDestinationRow, &sourceDestinationColumn, &sourceParent); + return d->model->dropMimeData(data, action, sourceDestinationRow, sourceDestinationColumn, sourceParent); } /*! diff --git a/src/corelib/itemmodels/qabstractproxymodel_p.h b/src/corelib/itemmodels/qabstractproxymodel_p.h index 9402092fa8..24af5afc64 100644 --- a/src/corelib/itemmodels/qabstractproxymodel_p.h +++ b/src/corelib/itemmodels/qabstractproxymodel_p.h @@ -59,6 +59,8 @@ public: QAbstractProxyModelPrivate() : QAbstractItemModelPrivate(), model(0) {} QAbstractItemModel *model; virtual void _q_sourceModelDestroyed(); + void mapDropCoordinatesToSource(int row, int column, const QModelIndex &parent, + int *source_row, int *source_column, QModelIndex *source_parent) const; }; QT_END_NAMESPACE diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index 3c383532bb..0b2b0e4188 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -2030,30 +2030,14 @@ Qt::DropActions QSortFilterProxyModel::supportedDropActions() const return d->model->supportedDropActions(); } +// Qt6: remove unnecessary reimplementation /*! \reimp */ bool QSortFilterProxyModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) { - Q_D(QSortFilterProxyModel); - if ((row == -1) && (column == -1)) - return d->model->dropMimeData(data, action, -1, -1, mapToSource(parent)); - int source_destination_row = -1; - int source_destination_column = -1; - QModelIndex source_parent; - if (row == rowCount(parent)) { - source_parent = mapToSource(parent); - source_destination_row = d->model->rowCount(source_parent); - } else { - QModelIndex proxy_index = index(row, column, parent); - QModelIndex source_index = mapToSource(proxy_index); - source_destination_row = source_index.row(); - source_destination_column = source_index.column(); - source_parent = source_index.parent(); - } - return d->model->dropMimeData(data, action, source_destination_row, - source_destination_column, source_parent); + return QAbstractProxyModel::dropMimeData(data, action, row, column, parent); } /*! diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp index 38a274947f..d05ed6c20f 100644 --- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp +++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp @@ -144,6 +144,7 @@ private slots: void noMapAfterSourceDelete(); void forwardDropApi(); + void canDropMimeData(); protected: void buildHierarchy(const QStringList &data, QAbstractItemModel *model); @@ -3909,6 +3910,36 @@ void tst_QSortFilterProxyModel::chainedProxyModelRoleNames() QVERIFY(proxy2.roleNames().value(Qt::UserRole + 1) == "custom"); } +// A source model with ABABAB rows, where only A rows accept drops. +// It will then be sorted by a QSFPM. +class DropOnOddRows : public QAbstractListModel +{ + Q_OBJECT +public: + DropOnOddRows(QObject *parent = 0) : QAbstractListModel(parent) {} + + QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const + { + if (role == Qt::DisplayRole) + return (index.row() % 2 == 0) ? "A" : "B"; + return QVariant(); + } + + int rowCount(const QModelIndex &parent = QModelIndex()) const + { + Q_UNUSED(parent); + return 10; + } + + bool canDropMimeData(const QMimeData *, Qt::DropAction, + int row, int column, const QModelIndex &parent) const Q_DECL_OVERRIDE + { + Q_UNUSED(row); + Q_UNUSED(column); + return parent.row() % 2 == 0; + } +}; + class SourceAssertion : public QSortFilterProxyModel { Q_OBJECT @@ -3973,5 +4004,30 @@ void tst_QSortFilterProxyModel::forwardDropApi() QVERIFY(model.dropMimeData(0, Qt::CopyAction, 0, 0, QModelIndex())); } +static QString rowTexts(QAbstractItemModel *model) { + QString str; + for (int row = 0 ; row < model->rowCount(); ++row) + str += model->index(row, 0).data().toString(); + return str; +} + +void tst_QSortFilterProxyModel::canDropMimeData() +{ + // Given a source model which only supports dropping on even rows + DropOnOddRows sourceModel; + QCOMPARE(rowTexts(&sourceModel), QString("ABABABABAB")); + + // and a proxy model that sorts the rows + QSortFilterProxyModel proxy; + proxy.setSourceModel(&sourceModel); + proxy.sort(0, Qt::AscendingOrder); + QCOMPARE(rowTexts(&proxy), QString("AAAAABBBBB")); + + // the proxy should correctly map canDropMimeData to the source model, + // i.e. accept drops on the first 5 rows and refuse drops on the next 5. + for (int row = 0; row < proxy.rowCount(); ++row) + QCOMPARE(proxy.canDropMimeData(0, Qt::CopyAction, -1, -1, proxy.index(row, 0)), row < 5); +} + QTEST_MAIN(tst_QSortFilterProxyModel) #include "tst_qsortfilterproxymodel.moc" -- cgit v1.2.3