diff options
author | Shawn Rutledge <shawn.rutledge@qt.io> | 2020-11-19 16:29:20 +0100 |
---|---|---|
committer | Shawn Rutledge <shawn.rutledge@qt.io> | 2020-11-19 20:52:43 +0100 |
commit | fd9e4920b3d8235061d2bde19bca7ded566f6331 (patch) | |
tree | 9aa40a037d18ed1a01e47548d21b1448fa8a40d0 | |
parent | d68419faf424df2492425baca789742f1a239af3 (diff) |
Fix tst_QQuickMouseArea::notPressedAfterStolenGrab again
Most of the time, QQuickWindowPrivate::deliverMatchingPointsToItem()
doesn't need to call item->mouseUngrabEvent() because all grab changes
are notified via the connection from signal QPointingDevice::grabChanged
to slot QQuickWindowPrivate::onGrabChanged(). But in this case,
MouseArea only accepts the event, rather than taking the grab itself.
Therefore at the time the grab is "stolen", there was not yet any
grabber, because grabbing is done after delivery. But we still need to
inform MouseArea that it's not getting the grab it expects to get, so
that it can reset its pressed state. But we don't want it to be
redundant (other tests are counting events, and we don't want repeated
ungrabs to show up in those); so now we have to track whether the item
on which we're about to call mouseUngrabEvent() has already gotten it.
This illustrates another problem with the tradition of accepting events
and being unclear about what it means. Grabbing is one thing, ending
delivery is another.
Amends a97759a336c597327cb82eebc9f45c793aec32c9
Task-number: QTBUG-55325
Task-number: QTBUG-86729
Change-Id: I8150f901e00e7a71499fc98ab54f0ba75370f3ec
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
-rw-r--r-- | src/quick/items/qquickwindow.cpp | 15 | ||||
-rw-r--r-- | src/quick/items/qquickwindow_p.h | 1 | ||||
-rw-r--r-- | tests/auto/quick/qquickmousearea/BLACKLIST | 3 | ||||
-rw-r--r-- | tests/auto/quick/qquickmousearea/data/rejectEvent.qml | 2 | ||||
-rw-r--r-- | tests/auto/quick/qquickmousearea/tst_qquickmousearea.cpp | 6 |
5 files changed, 20 insertions, 7 deletions
diff --git a/src/quick/items/qquickwindow.cpp b/src/quick/items/qquickwindow.cpp index 0515a62b1e..54320d1e4b 100644 --- a/src/quick/items/qquickwindow.cpp +++ b/src/quick/items/qquickwindow.cpp @@ -2636,8 +2636,10 @@ void QQuickWindowPrivate::onGrabChanged(QObject *grabber, QPointingDevice::GrabT QMutableSinglePointEvent e(QEvent::UngrabMouse, point.device(), point); hasFiltered.clear(); filtered = sendFilteredMouseEvent(&e, item, item->parentItem()); - if (!filtered) + if (!filtered) { + lastUngrabbed = item; item->mouseUngrabEvent(); + } } if (point.device()->type() == QInputDevice::DeviceType::TouchScreen) { bool allReleasedOrCancelled = true; @@ -2713,6 +2715,7 @@ void QQuickWindowPrivate::deliverPointerEvent(QPointerEvent *event) eventsInDelivery.pop(); --pointerEventRecursionGuard; + lastUngrabbed = nullptr; } // check if item or any of its child items contain the point, or if any pointer handler "wants" the point @@ -2959,7 +2962,15 @@ void QQuickWindowPrivate::deliverMatchingPointsToItem(QQuickItem *item, bool isG auto &point = pointerEvent->point(0); auto mouseGrabber = pointerEvent->exclusiveGrabber(point); if (mouseGrabber && mouseGrabber != item && mouseGrabber != oldMouseGrabber) { - // we don't need item->mouseUngrabEvent() because QQuickWindowPrivate::onGrabChanged does it + // Normally we don't need item->mouseUngrabEvent() here, because QQuickWindowPrivate::onGrabChanged does it. + // However, if one item accepted the mouse event, it expects to have the grab and be in "pressed" state, + // because accepting implies grabbing. But before it actually gets the grab, another item could steal it. + // In that case, onGrabChanged() does NOT notify the item that accepted the event that it's not getting the grab after all. + // So after ensuring that it's not redundant, we send a notification here, for that case (QTBUG-55325). + if (item != lastUngrabbed) { + item->mouseUngrabEvent(); + lastUngrabbed = item; + } } else if (item->isEnabled() && item->isVisible() && point.state() != QEventPoint::State::Released) { pointerEvent->setExclusiveGrabber(point, item); } diff --git a/src/quick/items/qquickwindow_p.h b/src/quick/items/qquickwindow_p.h index 6e4d24da85..67bce9f02c 100644 --- a/src/quick/items/qquickwindow_p.h +++ b/src/quick/items/qquickwindow_p.h @@ -154,6 +154,7 @@ public: #if QT_CONFIG(quick_draganddrop) QQuickDragGrabber *dragGrabber; #endif + QQuickItem *lastUngrabbed = nullptr; QStack<QPointerEvent *> eventsInDelivery; int touchMouseId; // only for obsolete stuff like QQuickItem::grabMouse() diff --git a/tests/auto/quick/qquickmousearea/BLACKLIST b/tests/auto/quick/qquickmousearea/BLACKLIST index 41608725da..7522f6f92f 100644 --- a/tests/auto/quick/qquickmousearea/BLACKLIST +++ b/tests/auto/quick/qquickmousearea/BLACKLIST @@ -12,9 +12,6 @@ opensuse-leap [nestedEventDelivery] * # QTBUG-86729 -[notPressedAfterStolenGrab] -* # QTBUG-86729 - [ignoreBySource] * # QTBUG-86729 diff --git a/tests/auto/quick/qquickmousearea/data/rejectEvent.qml b/tests/auto/quick/qquickmousearea/data/rejectEvent.qml index 816fc76fac..48b68ee845 100644 --- a/tests/auto/quick/qquickmousearea/data/rejectEvent.qml +++ b/tests/auto/quick/qquickmousearea/data/rejectEvent.qml @@ -13,6 +13,7 @@ Rectangle { MouseArea { id: mouseRegion1 + objectName: "mouseRegion1" anchors.fill: parent onPressed: { root.mr1_pressed = true } onReleased: { root.mr1_released = true } @@ -20,6 +21,7 @@ Rectangle { } MouseArea { id: mouseRegion2 + objectName: "mouseRegion2" width: 120; height: 120 onPressed: { root.mr2_pressed = true; mouse.accepted = false } onReleased: { root.mr2_released = true } diff --git a/tests/auto/quick/qquickmousearea/tst_qquickmousearea.cpp b/tests/auto/quick/qquickmousearea/tst_qquickmousearea.cpp index 569c68447e..40d0b11a86 100644 --- a/tests/auto/quick/qquickmousearea/tst_qquickmousearea.cpp +++ b/tests/auto/quick/qquickmousearea/tst_qquickmousearea.cpp @@ -43,6 +43,8 @@ #include <QtGui/QScreen> #include <qpa/qwindowsysteminterface.h> +Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests") + class CircleMask : public QObject { Q_OBJECT @@ -2228,7 +2230,7 @@ void tst_QQuickMouseArea::ignoreBySource() QCOMPARE(flickable->contentY(), 0.); } -void tst_QQuickMouseArea::notPressedAfterStolenGrab() +void tst_QQuickMouseArea::notPressedAfterStolenGrab() // QTBUG-55325 { QQuickWindow window; window.resize(200, 200); @@ -2239,7 +2241,7 @@ void tst_QQuickMouseArea::notPressedAfterStolenGrab() ma->setSize(window.size()); QObject::connect(ma, static_cast<void (QQuickMouseArea::*)(QQuickMouseEvent*)>(&QQuickMouseArea::pressed), - [&]() { window.contentItem()->grabMouse(); }); + [&]() { qCDebug(lcTests) << "stealing grab now"; window.contentItem()->grabMouse(); }); QTest::mouseClick(&window, Qt::LeftButton); QVERIFY(!ma->pressed()); |