diff options
author | Axel Spoerl <axel.spoerl@qt.io> | 2023-03-25 19:11:13 +0100 |
---|---|---|
committer | Axel Spoerl <axel.spoerl@qt.io> | 2023-03-31 13:09:39 +0200 |
commit | a815c40e736d0914b898a896f3dcee454804a7a2 (patch) | |
tree | 7b2f537f013ba72a9db3ed92c0abf87d871dfbff | |
parent | a94ba94695fe20d0cddc9b33396262ba3ed2b07f (diff) |
QListView: No-op, when a list item is dropped directly behind itself
QListView generates a move, when an item is dropped directly behind
itself. This causes unexpected behavior, e.g. item widgets getting
discarded.
This patch prevents a move from being generated in that case.
It adds an autotest to tst_QWidget, to verify item widgets and item
data do not get discarded in case of a no-op drag&drop.
Fixes: QTBUG-100128
Pick-to: 6.5 6.2
Change-Id: I02a755320a7b71dad218293c9c94c88da6507423
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
-rw-r--r-- | src/widgets/itemviews/qlistview.cpp | 3 | ||||
-rw-r--r-- | tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp | 54 |
2 files changed, 56 insertions, 1 deletions
diff --git a/src/widgets/itemviews/qlistview.cpp b/src/widgets/itemviews/qlistview.cpp index b03c1a47f5..2513bb2a0c 100644 --- a/src/widgets/itemviews/qlistview.cpp +++ b/src/widgets/itemviews/qlistview.cpp @@ -910,7 +910,8 @@ void QListView::dropEvent(QDropEvent *event) bool dataMoved = false; for (int i = 0; i < persIndexes.size(); ++i) { const QPersistentModelIndex &pIndex = persIndexes.at(i); - if (r != pIndex.row()) { + // only generate a move when not same row or behind itself + if (r != pIndex.row() && r != pIndex.row() + 1) { // try to move (preserves selection) dataMoved |= model()->moveRow(QModelIndex(), pIndex.row(), QModelIndex(), r); if (!dataMoved) // can't move - abort and let QAbstractItemView handle this diff --git a/tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp b/tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp index cbb0fdcc0c..07973d7225 100644 --- a/tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp +++ b/tests/auto/widgets/itemviews/qlistwidget/tst_qlistwidget.cpp @@ -8,6 +8,7 @@ #include <QSignalSpy> #include <QStyledItemDelegate> #include <QTest> +#include <QLabel> #include <private/qlistwidget_p.h> #include <QtWidgets/private/qapplication_p.h> @@ -105,6 +106,7 @@ private slots: void moveRows(); void moveRowsInvalid_data(); void moveRowsInvalid(); + void noopDragDrop(); protected slots: void rowsAboutToBeInserted(const QModelIndex &parent, int first, int last) @@ -1889,6 +1891,58 @@ void tst_QListWidget::createPersistentOnLayoutAboutToBeChangedAutoSort() // QTBU QCOMPARE(layoutChangedSpy.size(), 1); } +// Test that dropping an item on or beneath itself remains a no-op +void tst_QListWidget::noopDragDrop() // QTBUG-100128 +{ + QListWidget listWidget; + QList<QListWidgetItem *> items; + for (int i = 0; i < 5; ++i) { + const QString number = QString::number(i); + QListWidgetItem *item = new QListWidgetItem(&listWidget); + item->setData(Qt::UserRole, number); + QLabel *label = new QLabel(number); + listWidget.setItemWidget(item, label); + items.append(item); + } + + listWidget.show(); + QVERIFY(QTest::qWaitForWindowExposed(&listWidget)); + + const QRect &lastItemRect = listWidget.visualItemRect(items.at(4)); + const QPoint &dragStart = lastItemRect.center(); + const QPoint &dropPointNirvana = lastItemRect.center() + QPoint(20, 2 * lastItemRect.height()); + + // Implement check as a macro (not a method) to safely determine the error location. + // The macro checks that item data and item widget remain unchanged when drag&drop are executed. + // In order to verify that the assets do *not* change, we can't use QTRY*: These macros would + // spin the event loop only once, while 3/4 mouse events need to get processed. + // That's why we spin the event loop 13 times, to make sure other unexpected or pending events + // get processed. +#define CHECK_ITEM {\ + const QString number = QString::number(4);\ + for (int i = 0; i < 13; ++i)\ + QApplication::processEvents();\ + QLabel *label = qobject_cast<QLabel *>(listWidget.itemWidget(items.at(4)));\ + QVERIFY(label);\ + QCOMPARE(label->text(), number);\ + const QString &data = items.at(4)->data(Qt::UserRole).toString();\ + QCOMPARE(data, number);\ + } + + // Test dropping last item beneath itself + QTest::mousePress(&listWidget, Qt::LeftButton, Qt::KeyboardModifiers(), dragStart); + QTest::mouseMove(&listWidget, dropPointNirvana); + QTest::mouseRelease(&listWidget, Qt::LeftButton); + CHECK_ITEM; + + // Test dropping last item on itself + QTest::mousePress(&listWidget, Qt::LeftButton, Qt::KeyboardModifiers(), dragStart); + QTest::mouseMove(&listWidget, dropPointNirvana); + QTest::mouseMove(&listWidget, dragStart); + QTest::mouseRelease(&listWidget, Qt::LeftButton); + CHECK_ITEM; +} + #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) void tst_QListWidget::clearItemData() { |