From cc02da23a849315aa777ac4464b34caa4f4d0e7a Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 28 May 2021 16:32:52 +0200 Subject: [PATCH] QAbstractItemView: don't block dragging after double click After d6551fe12520 it was no longer possible to start a drag with a double click (where the first click selects an item, and the second press+move starts the drag). Resetting the pressedItem variable to block the emission of the clicked() signal had this unwanted side effect. Instead, use an explicit boolean to store that the next release event will be the result of a double click, so that the clicked() signal is not emitted again (preventing the double-emission was the purpose of change d6551fe12520). Task-number: QTBUG-77771 Fixes: QTBUG-94087 Change-Id: I082c5169d89eb980dcd7985ef3d302b6ff060fb9 Reviewed-by: Samuel Gaist Reviewed-by: Richard Moe Gustavsen Reviewed-by: Olivier BARTHELEMY Reviewed-by: Lars Knoll (cherry picked from commit 17c1ebf8bfd254ff75cc55e335d1c1fb01da547f) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/itemviews/qabstractitemview.cpp | 7 +- src/widgets/itemviews/qabstractitemview_p.h | 1 + src/widgets/itemviews/qtreeview.cpp | 2 +- .../tst_qabstractitemview.cpp | 95 +++++++++++++++++++ 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index bd89f43ca1b..8a6c197b5d5 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -89,6 +89,7 @@ QAbstractItemViewPrivate::QAbstractItemViewPrivate() pressedModifiers(Qt::NoModifier), pressedPosition(QPoint(-1, -1)), pressedAlreadySelected(false), + releaseFromDoubleClick(false), viewportEnteredNeeded(false), state(QAbstractItemView::NoState), stateBeforeAnimation(QAbstractItemView::NoState), @@ -1913,6 +1914,8 @@ void QAbstractItemView::mouseMoveEvent(QMouseEvent *event) void QAbstractItemView::mouseReleaseEvent(QMouseEvent *event) { Q_D(QAbstractItemView); + const bool releaseFromDoubleClick = d->releaseFromDoubleClick; + d->releaseFromDoubleClick = false; QPoint pos = event->position().toPoint(); QPersistentModelIndex index = indexAt(pos); @@ -1925,7 +1928,7 @@ void QAbstractItemView::mouseReleaseEvent(QMouseEvent *event) return; } - bool click = (index == d->pressedIndex && index.isValid()); + bool click = (index == d->pressedIndex && index.isValid() && !releaseFromDoubleClick); bool selectedClicked = click && (event->button() == Qt::LeftButton) && d->pressedAlreadySelected; EditTrigger trigger = (selectedClicked ? SelectedClicked : NoEditTriggers); const bool edited = click ? edit(index, trigger, event) : false; @@ -1980,7 +1983,7 @@ void QAbstractItemView::mouseDoubleClickEvent(QMouseEvent *event) if ((event->button() == Qt::LeftButton) && !edit(persistent, DoubleClicked, event) && !style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick, nullptr, this)) emit activated(persistent); - d->pressedIndex = QModelIndex(); + d->releaseFromDoubleClick = true; } #if QT_CONFIG(draganddrop) diff --git a/src/widgets/itemviews/qabstractitemview_p.h b/src/widgets/itemviews/qabstractitemview_p.h index 83012a650d3..521d341b8f1 100644 --- a/src/widgets/itemviews/qabstractitemview_p.h +++ b/src/widgets/itemviews/qabstractitemview_p.h @@ -371,6 +371,7 @@ public: Qt::KeyboardModifiers pressedModifiers; QPoint pressedPosition; bool pressedAlreadySelected; + bool releaseFromDoubleClick; //forces the next mouseMoveEvent to send the viewportEntered signal //if the mouse is over the viewport and not over an item diff --git a/src/widgets/itemviews/qtreeview.cpp b/src/widgets/itemviews/qtreeview.cpp index 49585d8b26e..8d7057deff8 100644 --- a/src/widgets/itemviews/qtreeview.cpp +++ b/src/widgets/itemviews/qtreeview.cpp @@ -1992,7 +1992,7 @@ void QTreeView::mouseDoubleClickEvent(QMouseEvent *event) if (!style()->styleHint(QStyle::SH_ItemView_ActivateItemOnSingleClick, nullptr, this)) emit activated(persistent); - d->pressedIndex = QModelIndex(); + d->releaseFromDoubleClick = true; d->executePostedLayout(); // we need to make sure viewItems is updated if (d->itemsExpandable && d->expandsOnDoubleClick diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 10320c80d64..3d851dc10cb 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -154,6 +154,8 @@ private slots: void checkFocusAfterActivationChanges_data(); void checkFocusAfterActivationChanges(); void dragSelectAfterNewPress(); + void dragWithSecondClick_data(); + void dragWithSecondClick(); void selectionCommand_data(); void selectionCommand(); private: @@ -2600,6 +2602,99 @@ void tst_QAbstractItemView::dragSelectAfterNewPress() QVERIFY(selected.contains(model.index(i, 0))); } +void tst_QAbstractItemView::dragWithSecondClick_data() +{ + QTest::addColumn("viewClass"); + QTest::addColumn("doubleClick"); + for (QString viewClass : {"QListView", "QTreeView"}) { + QTest::addRow("DoubleClick") << viewClass << true; + QTest::addRow("Two Single Clicks") << viewClass << false; + } +} + +// inject the ability to record which indexes get dragged into any QAbstractItemView class +struct DragRecorder +{ + virtual ~DragRecorder() = default; + bool dragStarted = false; + QModelIndexList draggedIndexes; + QAbstractItemView *view; +}; + +template +class DragRecorderView : public ViewClass, public DragRecorder +{ +public: + DragRecorderView() + { view = this; } +protected: + void startDrag(Qt::DropActions) override + { + draggedIndexes = ViewClass::selectedIndexes(); + dragStarted = true; + } +}; + +void tst_QAbstractItemView::dragWithSecondClick() +{ + QFETCH(QString, viewClass); + QFETCH(bool, doubleClick); + + QStandardItemModel model; + QStandardItem *parentItem = model.invisibleRootItem(); + for (int i = 0; i < 10; ++i) { + QStandardItem *item = new QStandardItem(QString("item %0").arg(i)); + item->setDragEnabled(true); + item->setEditable(false); + parentItem->appendRow(item); + } + + std::unique_ptr dragRecorder; + if (viewClass == "QTreeView") + dragRecorder.reset(new DragRecorderView); + else if (viewClass == "QListView") + dragRecorder.reset(new DragRecorderView); + + QAbstractItemView *view = dragRecorder->view; + view->setModel(&model); + view->setFixedSize(160, 650); // Minimum width for windows with frame on Windows 8 + view->setSelectionMode(QAbstractItemView::MultiSelection); + view->setDragDropMode(QAbstractItemView::InternalMove); + centerOnScreen(view); + moveCursorAway(view); + view->show(); + QVERIFY(QTest::qWaitForWindowExposed(view)); + + QModelIndex index0 = model.index(0, 0); + QModelIndex index1 = model.index(1, 0); + // Select item 0 using a single click + QTest::mouseClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, + view->visualRect(index0).center()); + QCOMPARE(view->currentIndex(), index0); + + if (doubleClick) { + // press on same item within the double click interval + QTest::mouseDClick(view->viewport(), Qt::LeftButton, Qt::NoModifier, + view->visualRect(index0).center()); + } else { + // or on different item with a slow second press + QTest::mousePress(view->viewport(), Qt::LeftButton, Qt::NoModifier, + view->visualRect(index1).center()); + } + // then drag far enough with left button held + const QPoint dragTo = view->visualRect(index1).center() + + QPoint(2 * QApplication::startDragDistance(), + 2 * QApplication::startDragDistance()); + QMouseEvent mouseMoveEvent(QEvent::MouseMove, dragTo, + Qt::NoButton, Qt::LeftButton, Qt::NoModifier); + QVERIFY(QApplication::sendEvent(view->viewport(), &mouseMoveEvent)); + // twice since the view will first enter dragging state, then start the drag + // (not necessary to actually move the mouse) + QVERIFY(QApplication::sendEvent(view->viewport(), &mouseMoveEvent)); + QVERIFY(dragRecorder->dragStarted); + QTest::mouseRelease(view->viewport(), Qt::LeftButton, Qt::NoModifier, dragTo); +} + void tst_QAbstractItemView::selectionCommand_data() { QTest::addColumn("selectionMode");