From 8f3dca45556f536be36e943b6f1f0e65ef9eb93b Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Mon, 7 Jun 2021 22:51:02 +0200
Subject: [PATCH] QAbstractItemView: block autoScroll from interfering with
 QScroller

When pressing an index in order to start a flick gesture, QAIV sets
the current index. When QScroller changes state to Dragging, then
QAIV restores the current index to what it was before the press, as
the user is clearly scrolling the view. With autoScroll enabled, this
will produce an ugly jump if the old current index is no longer in
the viewport.

To prevent this, disable autoScroll before restoring the currentIndex.

Fixes: QTBUG-64543
Change-Id: I3e0a18a6a179d80b9d810fce5aa658f0cfff9a29
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit b1fdcc8c0fefe0660302494618032342b623e199)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
 src/widgets/itemviews/qabstractitemview.cpp   |  4 ++
 .../tst_qabstractitemview.cpp                 | 63 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp
index 8a6c197b5d5..120509fddd5 100644
--- a/src/widgets/itemviews/qabstractitemview.cpp
+++ b/src/widgets/itemviews/qabstractitemview.cpp
@@ -224,7 +224,11 @@ void QAbstractItemViewPrivate::_q_scrollerStateChanged()
             // restore the old selection if we really start scrolling
             if (q->selectionModel()) {
                 q->selectionModel()->select(oldSelection, QItemSelectionModel::ClearAndSelect);
+                // block autoScroll logic while we are already handling scrolling
+                const bool wasAutoScroll = autoScroll;
+                autoScroll = false;
                 q->selectionModel()->setCurrentIndex(oldCurrent, QItemSelectionModel::NoUpdate);
+                autoScroll = wasAutoScroll;
             }
             Q_FALLTHROUGH();
 
diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
index 3d851dc10cb..9dcef394e0f 100644
--- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
+++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
@@ -35,11 +35,13 @@
 #include <QHeaderView>
 #include <QIdentityProxyModel>
 #include <QItemDelegate>
+#include <QLabel>
 #include <QLineEdit>
 #include <QListWidget>
 #include <QProxyStyle>
 #include <QPushButton>
 #include <QScrollBar>
+#include <QScroller>
 #include <QSignalSpy>
 #include <QSortFilterProxyModel>
 #include <QSpinBox>
@@ -158,6 +160,8 @@ private slots:
     void dragWithSecondClick();
     void selectionCommand_data();
     void selectionCommand();
+    void scrollerSmoothScroll();
+
 private:
     static QAbstractItemView *viewFromString(const QByteArray &viewType, QWidget *parent = nullptr)
     {
@@ -2739,5 +2743,64 @@ void tst_QAbstractItemView::selectionCommand()
     QCOMPARE(selectionFlag, view.selectionCommand(QModelIndex(), nullptr));
 }
 
+/*!
+    Verify that scrolling an autoScroll enabled itemview with a QScroller
+    produces a continuous, smooth scroll without any jumping around due to
+    the currentItem negotiation between QAbstractItemView and QScroller.
+    QTBUG-64543.
+*/
+void tst_QAbstractItemView::scrollerSmoothScroll()
+{
+    QListWidget view;
+    view.setAutoScroll(true);
+    view.setVerticalScrollMode(QListView::ScrollPerPixel);
+
+    QScroller::grabGesture(view.viewport(), QScroller::TouchGesture);
+    QScroller::grabGesture(view.viewport(), QScroller::LeftMouseButtonGesture);
+
+    for (int i = 0; i < 50; i++) {
+        QListWidgetItem* item = new QListWidgetItem("item " + QString::number(i), &view);
+        // gives items a touch friendly size so that only a few fit into the viewport
+        item->setSizeHint(QSize(100,50));
+    }
+
+    // make sure we have space for only a few items
+    view.setFixedSize(120, 200);
+    view.show();
+    QVERIFY(QTest::qWaitForWindowActive(&view));
+
+    // we flick up, so we should never scroll back
+    int lastScrollPosition = 0;
+    bool scrollBack = false;
+    connect(view.verticalScrollBar(), &QScrollBar::valueChanged, [&](int value){
+        scrollBack |= (value < lastScrollPosition);
+        lastScrollPosition = value;
+    });
+
+    // start in the middle
+    view.scrollToItem(view.item(25));
+    QCOMPARE(view.currentItem(), view.item(0));
+    QListWidgetItem *pressItem = view.item(23);
+    QPoint dragPosition = view.visualRect(view.indexFromItem(pressItem)).center();
+    // the mouse press changes the current item temporarily, but the press is delayed
+    // by the gesture machinery
+    QTest::mousePress(view.viewport(), Qt::LeftButton, Qt::NoModifier, dragPosition);
+    QTRY_COMPARE(view.currentItem(), pressItem);
+
+    // QAIV will reset the current item when the scroller changes state to Dragging
+    for (int y = 0; y < QApplication::startDragDistance() * 2; ++y) {
+        // gesture recognizer needs some throttling
+        QTest::qWait(10);
+        dragPosition -= QPoint(0, 10);
+        const QPoint globalPos = view.viewport()->mapToGlobal(dragPosition);
+        QMouseEvent mouseMoveEvent(QEvent::MouseMove, dragPosition, dragPosition, globalPos,
+                                    Qt::NoButton, Qt::LeftButton, Qt::NoModifier);
+        QApplication::sendEvent(view.viewport(), &mouseMoveEvent);
+        QVERIFY(!scrollBack);
+    }
+
+    QTest::mouseRelease(view.viewport(), Qt::LeftButton, Qt::NoModifier, dragPosition);
+}
+
 QTEST_MAIN(tst_QAbstractItemView)
 #include "tst_qabstractitemview.moc"