From bced3a2477e0a6fa4abbfe60ff7e704c6fef63a2 Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Wed, 28 Apr 2021 16:01:40 +0200 Subject: [PATCH] ItemViews: don't delete dragged items when a subclass accepted the move Amends 0f1008a5936c903ca9448193df7df6117e2c617b, which introduced the dropEventMoved private data member through which the drop-site itemview can notify the drag-site that the drop handler has taken care of the move operation. However, if a subclass of an item view overrides dropEvent to move and accept the event before calling the default implementation, then the flag would not be set, as the dropOn helper would return false. So QAbstractItemView still removed the item, resulting in two items being removed when one was move-dropped. Set the dropEventMoved member also when the QTreeWidget::dropEvent handler is called by a subclass override and the event is already accepted. This way, overrides don't have to artifically set the accepted action to "IgnoreAction" to disable the handling in drag site. [ChangeLog][QtWidgets][QAbstractItemView] Classes overriding dropEvent for MoveAction events to move data can call accept() on the event before calling the superclass to prevent QAbstractItemView from deleting the source item. Task-number: QTBUG-87057 Task-number: QTBUG-77427 Change-Id: Ibe75fc1b2ca60627c825ad9b9b6d48953577edec Reviewed-by: Richard Moe Gustavsen (cherry picked from commit 808a6dedcb4aabcb81f096f03d0b1bb4ae2ea0d1) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/itemviews/qlistview.cpp | 19 +++++++++++++------ src/widgets/itemviews/qtablewidget.cpp | 9 ++++++--- src/widgets/itemviews/qtreewidget.cpp | 9 ++++++--- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/widgets/itemviews/qlistview.cpp b/src/widgets/itemviews/qlistview.cpp index 27b4402e883..e3dbc097ccc 100644 --- a/src/widgets/itemviews/qlistview.cpp +++ b/src/widgets/itemviews/qlistview.cpp @@ -921,7 +921,8 @@ void QListView::dropEvent(QDropEvent *event) bool topIndexDropped = false; int col = -1; int row = -1; - if (d->dropOn(event, &row, &col, &topIndex)) { + // check whether a subclass has already accepted the event, ie. moved the data + if (!event->isAccepted() && d->dropOn(event, &row, &col, &topIndex)) { const QList selIndexes = selectedIndexes(); QList persIndexes; persIndexes.reserve(selIndexes.count()); @@ -940,23 +941,29 @@ void QListView::dropEvent(QDropEvent *event) QPersistentModelIndex dropRow = model()->index(row, col, topIndex); int r = row == -1 ? model()->rowCount() : (dropRow.row() >= 0 ? dropRow.row() : row); + bool dataMoved = false; for (int i = 0; i < persIndexes.count(); ++i) { const QPersistentModelIndex &pIndex = persIndexes.at(i); if (r != pIndex.row()) { // try to move (preserves selection) - d->dropEventMoved |= model()->moveRow(QModelIndex(), pIndex.row(), QModelIndex(), r); - if (!d->dropEventMoved) // can't move - abort and let QAbstractItemView handle this + dataMoved |= model()->moveRow(QModelIndex(), pIndex.row(), QModelIndex(), r); + if (!dataMoved) // can't move - abort and let QAbstractItemView handle this break; } else { // move onto itself is blocked, don't delete anything - d->dropEventMoved = true; + dataMoved = true; } r = pIndex.row() + 1; // Dropped items are inserted contiguously and in the right order. } - if (d->dropEventMoved) - event->accept(); // data moved, nothing to be done in QAbstractItemView::dropEvent + if (dataMoved) + event->accept(); } } + + // either we or a subclass accepted the move event, so assume that the data was + // moved and that QAbstractItemView shouldn't remove the source when QDrag::exec returns + if (event->isAccepted()) + d->dropEventMoved = true; } if (!d->commonListView->filterDropEvent(event) || !d->dropEventMoved) { diff --git a/src/widgets/itemviews/qtablewidget.cpp b/src/widgets/itemviews/qtablewidget.cpp index f01d392545b..5072cc9240e 100644 --- a/src/widgets/itemviews/qtablewidget.cpp +++ b/src/widgets/itemviews/qtablewidget.cpp @@ -2679,7 +2679,8 @@ void QTableWidget::dropEvent(QDropEvent *event) { QModelIndex topIndex; int col = -1; int row = -1; - if (d->dropOn(event, &row, &col, &topIndex)) { + // check whether a subclass has already accepted the event, ie. moved the data + if (!event->isAccepted() && d->dropOn(event, &row, &col, &topIndex)) { const QModelIndexList indexes = selectedIndexes(); int top = INT_MAX; int left = INT_MAX; @@ -2701,9 +2702,11 @@ void QTableWidget::dropEvent(QDropEvent *event) { } event->accept(); - // Don't want QAbstractItemView to delete it because it was "moved" we already did it - d->dropEventMoved = true; } + // either we or a subclass accepted the move event, so assume that the data was + // moved and that QAbstractItemView shouldn't remove the source when QDrag::exec returns + if (event->isAccepted()) + d->dropEventMoved = true; } QTableView::dropEvent(event); diff --git a/src/widgets/itemviews/qtreewidget.cpp b/src/widgets/itemviews/qtreewidget.cpp index e1f7da6fcdf..08b1137c5cd 100644 --- a/src/widgets/itemviews/qtreewidget.cpp +++ b/src/widgets/itemviews/qtreewidget.cpp @@ -3340,7 +3340,8 @@ void QTreeWidget::dropEvent(QDropEvent *event) { QModelIndex topIndex; int col = -1; int row = -1; - if (d->dropOn(event, &row, &col, &topIndex)) { + // check whether a subclass has already accepted the event, ie. moved the data + if (!event->isAccepted() && d->dropOn(event, &row, &col, &topIndex)) { const QList idxs = selectedIndexes(); QList indexes; const int indexesCount = idxs.count(); @@ -3387,9 +3388,11 @@ void QTreeWidget::dropEvent(QDropEvent *event) { } event->accept(); - // Don't want QAbstractItemView to delete it because it was "moved" we already did it - d->dropEventMoved = true; } + // either we or a subclass accepted the move event, so assume that the data was + // moved and that QAbstractItemView shouldn't remove the source when QDrag::exec returns + if (event->isAccepted()) + d->dropEventMoved = true; } QTreeView::dropEvent(event);