From 8fd398c9d2f5f54e446e0b402bc63a2edb50da6f Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Tue, 2 May 2017 16:25:37 +1000 Subject: Prevent PathView's parent stealing mouse grab during double-flick This commit fixes an issue where mouse events could be stolen by the parent of a PathView due to the PathView not correctly setting the keep-mouse-grab flag in handleMousePressEvent(). This commit ensures that the flag is correctly set, so that the second flick in a double flick is handled by the PathView rather than being stolen by the parent. Task-number: QTBUG-59620 Change-Id: Iccdfe16e7e80e6d1d31f95c3dba9c8839b20f30f Reviewed-by: Shawn Rutledge --- src/quick/items/qquickpathview.cpp | 2 + .../quick/qquickpathview/tst_qquickpathview.cpp | 55 ++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/quick/items/qquickpathview.cpp b/src/quick/items/qquickpathview.cpp index 879db6284e..e7e19b041e 100644 --- a/src/quick/items/qquickpathview.cpp +++ b/src/quick/items/qquickpathview.cpp @@ -1632,6 +1632,7 @@ void QQuickPathView::mousePressEvent(QMouseEvent *event) void QQuickPathViewPrivate::handleMousePressEvent(QMouseEvent *event) { + Q_Q(QQuickPathView); if (!interactive || !items.count() || !model || !modelCount) return; velocityBuffer.clear(); @@ -1657,6 +1658,7 @@ void QQuickPathViewPrivate::handleMousePressEvent(QMouseEvent *event) stealMouse = true; // If we've been flicked then steal the click. else stealMouse = false; + q->setKeepMouseGrab(stealMouse); timer.start(); lastPosTime = computeCurrentTime(event); diff --git a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp index e6c03d052c..badc6f44c8 100644 --- a/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp +++ b/tests/auto/quick/qquickpathview/tst_qquickpathview.cpp @@ -50,6 +50,8 @@ #include +Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests") + using namespace QQuickViewTestUtil; using namespace QQuickVisualTestUtil; @@ -2255,6 +2257,59 @@ void tst_QQuickPathView::nestedinFlickable() QCOMPARE(fflickingSpy.count(), 0); QCOMPARE(fflickStartedSpy.count(), 0); QCOMPARE(fflickEndedSpy.count(), 0); + + // now test that two quick flicks are both handled by the pathview + movingSpy.clear(); + moveStartedSpy.clear(); + moveEndedSpy.clear(); + fflickingSpy.clear(); + fflickStartedSpy.clear(); + fflickEndedSpy.clear(); + int shortInterval = 2; + QTest::mousePress(window.data(), Qt::LeftButton, 0, QPoint(23,216)); + QTest::mouseMove(window.data(), QPoint(48,216), shortInterval); + QTest::mouseRelease(window.data(), Qt::LeftButton, 0, QPoint(73,217)); + QVERIFY(pathview->isMoving()); + QTest::mousePress(window.data(), Qt::LeftButton, 0, QPoint(21,216)); + QTest::mouseMove(window.data(), QPoint(46,216), shortInterval); + QTest::mouseRelease(window.data(), Qt::LeftButton, 0, QPoint(71,217)); + QVERIFY(pathview->isMoving()); + // moveEndedSpy.count() and moveStartedSpy.count() should be exactly 1 + // but in CI we sometimes see a scheduling issue being hit which + // causes the main thread to be stalled while the animation thread + // continues, allowing the animation timer to finish after the first + // call to QVERIFY(pathview->isMoving()) in the code above, prior to + // the detected beginning of the second flick, which can cause both of + // those signal counts to be 2 rather than 1. + // Note that this is not a true problem (this scheduling quirk just + // means that the unit test is not testing the enforced behavior + // as strictly as it would otherwise); it is only a bug if it results + // in the Flickable handling one or more of the flicks, and that + // is unconditionally tested below. + // To avoid false positive test failure in the scheduling quirk case + // we allow the multiple signal count case, rather than simply: + // QTRY_COMPARE(moveEndedSpy.count(), 1); + // QCOMPARE(moveStartedSpy.count(), 1); + QTRY_VERIFY(moveEndedSpy.count() > 0); + qCDebug(lcTests) << "After receiving moveEnded signal:" + << "moveEndedSpy.count():" << moveEndedSpy.count() + << "moveStartedSpy.count():" << moveStartedSpy.count() + << "fflickingSpy.count():" << fflickingSpy.count() + << "fflickStartedSpy.count():" << fflickStartedSpy.count() + << "fflickEndedSpy.count():" << fflickEndedSpy.count(); + QTRY_COMPARE(moveStartedSpy.count(), moveEndedSpy.count()); + qCDebug(lcTests) << "After receiving matched moveEnded signal(s):" + << "moveEndedSpy.count():" << moveEndedSpy.count() + << "moveStartedSpy.count():" << moveStartedSpy.count() + << "fflickingSpy.count():" << fflickingSpy.count() + << "fflickStartedSpy.count():" << fflickStartedSpy.count() + << "fflickEndedSpy.count():" << fflickEndedSpy.count(); + QVERIFY(moveStartedSpy.count() <= 2); + // Flickable should not handle this + QCOMPARE(fflickingSpy.count(), 0); + QCOMPARE(fflickStartedSpy.count(), 0); + QCOMPARE(fflickEndedSpy.count(), 0); + } void tst_QQuickPathView::flickableDelegate() -- cgit v1.2.3