QQuickFlickable: avoid processing the same event twice

If Flickable has an exclusive grab (e.g if it's being dragged), and
at the same time, a child has a passive grab (e.g a TapHandler inside
a child of the content item), Flickable ends up getting the same pointer
events twice.

The reason this happens is because Flickable has a childMouseEventFilter.
So the flickable will first get all the pointer events since it has an
exclusive grab, just to see that the filter will receive the same
events once more, as they next are delivered to the passive grabbers.

The result is that Flickable will handle all pointer events
(move, release etc) twice when it has en exclusive grab, which will
even cause the flickable from stop flicking prematurely if the mouse
release ends up outside the bounds of the flickable (because of a double
call to handleReleaseEvent(), which will set stealMouse to false too
early).

To fix this, this patch will make sure that we don't handle any pointer
events in the childMouseEventFilter if we already have an exclusive grab.
After all, having an exclusive grab means that we're already getting the
events the "normal" way, and shouldn't handle the same events once more.

Fixes: QTBUG-104987
Change-Id: Iaed49cb860cf50ea38a70a6e546d9dcf25cce444
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Oliver Eftevaag <oliver.eftevaag@qt.io>
(cherry picked from commit 1bac9de1136c8d52650199f9defefae2f1d6a1a5)

3 files changed, 73 insertions, 0 deletions

diff --git a/src/quick/items/qquickflickable.cpp b/src/quick/items/qquickflickable.cpp
index 14b97e35da..fdb294cdfc 100644
--- a/src/quick/items/qquickflickable.cpp
+++ b/src/quick/items/qquickflickable.cpp
@@ -2537,6 +2537,18 @@ bool QQuickFlickable::filterPointerEvent(QQuickItem *receiver, QPointerEvent *ev
     if (isTouch && static_cast<QTouchEvent *>(event)->touchPointStates().testFlag(QEventPoint::State::Pressed))
         d->stealMouse = false;
     const auto &firstPoint = event->points().first();
+
+    if (event->pointCount() == 1 && event->exclusiveGrabber(firstPoint) == this) {
+        // We have an exclusive grab (since we're e.g dragging), but at the same time, we have
+        // a child with a passive grab (which is why this filter is being called). And because
+        // of that, we end up getting the same pointer events twice; First in our own event
+        // handlers (because of the grab), then once more in here, since we filter the child.
+        // To avoid processing the event twice (e.g avoid calling handleReleaseEvent once more
+        // from below), we mark the event as filtered, and simply return.
+        event->setAccepted(true);
+        return true;
+    }
+
     QPointF localPos = mapFromScene(firstPoint.scenePosition());
     bool receiverDisabled = receiver && !receiver->isEnabled();
     bool stealThisEvent = d->stealMouse;
diff --git a/tests/auto/quick/qquickflickable/data/flickableWithTapHandler.qml b/tests/auto/quick/qquickflickable/data/flickableWithTapHandler.qml
new file mode 100644
index 0000000000..91b81059ab
--- /dev/null
+++ b/tests/auto/quick/qquickflickable/data/flickableWithTapHandler.qml
@@ -0,0 +1,24 @@
+import QtQuick
+
+Item {
+    width: 300
+    height: 300
+
+    Flickable {
+        anchors.fill: parent
+        anchors.topMargin: 100
+        contentWidth: 1000
+        contentHeight: 1000
+
+        Rectangle {
+            objectName: "childItem"
+            x: 20
+            y: 50
+            width: 20
+            height: 20
+            color: "red"
+            TapHandler {
+            }
+        }
+    }
+}
diff --git a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
index b911b2e22c..7d5c113e87 100644
--- a/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
+++ b/tests/auto/quick/qquickflickable/tst_qquickflickable.cpp
@@ -217,6 +217,7 @@ private slots:
     void receiveTapOutsideContentItem();
     void flickWhenRotated_data();
     void flickWhenRotated();
+    void flickAndReleaseOutsideBounds();
     void scrollingWithFractionalExtentSize_data();
     void scrollingWithFractionalExtentSize();
     void setContentPositionWhileDragging_data();
@@ -2934,6 +2935,42 @@ void tst_qquickflickable::flickWhenRotated() // QTBUG-99639
     QVERIFY(!flickable->isAtYBeginning());
 }
 
+void tst_qquickflickable::flickAndReleaseOutsideBounds() // QTBUG-104987
+{
+    // Check that flicking works when the mouse release happens
+    // outside the bounds of the flickable (and the flick started on top
+    // of a TapHandler that has a passive grab).
+    QQuickView window;
+    QVERIFY(QQuickTest::showView(window, testFileUrl("flickableWithTapHandler.qml")));
+    QQuickItem *rootItem = window.rootObject();
+    QVERIFY(rootItem);
+    QQuickFlickable *flickable = rootItem->findChild<QQuickFlickable*>();
+    QVERIFY(flickable);
+    QQuickItem *childItem = flickable->findChild<QQuickItem*>("childItem");
+    QVERIFY(childItem);
+
+    QVERIFY(flickable->isAtYBeginning());
+
+    // Startpoint is on top of the tapHandler, while the endpoint is outside the flickable
+    const QPointF startPos = childItem->mapToGlobal(QPoint(10, 10));
+    const QPointF endPos = flickable->mapToGlobal(QPoint(10, -10));
+    const QPoint globalStartPos = window.mapFromGlobal(startPos).toPoint();
+    const QPoint globalEndPos = window.mapFromGlobal(endPos).toPoint();
+    const qreal dragDistance = 20;
+
+    // Note: here we need to initiate a flick using raw events, rather than
+    // flickable.flick(), since we're testing if the mouse events takes the
+    // correct path to starts a flick (among passive and exclusive grabbers, combined
+    // with childMouseEventFilter()).
+    QTest::mousePress(&window, Qt::LeftButton, Qt::NoModifier, globalStartPos);
+    QTest::mouseMove(&window, globalStartPos - QPoint(0, dragDistance / 2));
+    QTest::mouseMove(&window, globalStartPos - QPoint(0, dragDistance));
+    QTest::mouseMove(&window, globalEndPos);
+    QTest::mouseRelease(&window, Qt::LeftButton, Qt::NoModifier, globalEndPos);
+
+    // Ensure that the content item ends up being moved more than what we dragged
+    QTRY_VERIFY(flickable->contentY() > dragDistance * 2);
+}
 
 void tst_qquickflickable::scrollingWithFractionalExtentSize_data()
 {