From 94c85b4e0b449c8a29e17c5aadea243d3d35da77 Mon Sep 17 00:00:00 2001 From: Andrew den Exter Date: Fri, 27 Jan 2012 14:56:07 +1000 Subject: [PATCH] Guard against event recursion in QQuickDrag. Do not emit drag events recursively as this will send events out of order and corrupt the grab stack. Change-Id: Ieede7123c96304e23c809ac895318ed76c8c95c1 Reviewed-by: Martin Jones --- src/quick/items/qquickdrag.cpp | 47 ++++- src/quick/items/qquickdrag_p.h | 2 + tests/auto/qtquick2/qquickdrag/tst_qquickdrag.cpp | 207 ++++++++++++++++++++++ 3 files changed, 247 insertions(+), 9 deletions(-) diff --git a/src/quick/items/qquickdrag.cpp b/src/quick/items/qquickdrag.cpp index a906af3..c44e1ac 100644 --- a/src/quick/items/qquickdrag.cpp +++ b/src/quick/items/qquickdrag.cpp @@ -45,7 +45,7 @@ #include #include #include - +#include #include QT_BEGIN_NAMESPACE @@ -64,10 +64,12 @@ public: , supportedActions(Qt::MoveAction | Qt::CopyAction | Qt::LinkAction) , active(false) , listening(false) + , inEvent(false) { } void itemGeometryChanged(QQuickItem *, const QRectF &, const QRectF &); + void deliverEvent(QQuickCanvas *canvas, QEvent *event); void start() { start(supportedActions); } void start(Qt::DropActions supportedActions); void setTarget(QQuickItem *item); @@ -82,6 +84,7 @@ public: Qt::DropActions supportedActions; bool active : 1; bool listening : 1; + bool inEvent : 1; QPointF hotSpot; QStringList keys; }; @@ -116,14 +119,14 @@ public: void QQuickDragAttachedPrivate::itemGeometryChanged(QQuickItem *, const QRectF &newGeometry, const QRectF &oldGeometry) { Q_Q(QQuickDragAttached); - if (newGeometry.topLeft() == oldGeometry.topLeft() || !active) + if (newGeometry.topLeft() == oldGeometry.topLeft() || !active || inEvent) return; if (QQuickCanvas *canvas = attachedItem->canvas()) { QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint(); QDragMoveEvent event(scenePos, mimeData->m_supportedActions, mimeData, Qt::NoButton, Qt::NoModifier); QQuickDropEventEx::setProposedAction(&event, proposedAction); - QQuickCanvasPrivate::get(canvas)->deliverDragEvent(&dragGrabber, &event); + deliverEvent(canvas, &event); if (target != dragGrabber.target()) { target = dragGrabber.target(); emit q->targetChanged(); @@ -131,6 +134,14 @@ void QQuickDragAttachedPrivate::itemGeometryChanged(QQuickItem *, const QRectF & } } +void QQuickDragAttachedPrivate::deliverEvent(QQuickCanvas *canvas, QEvent *event) +{ + Q_ASSERT(!inEvent); + inEvent = true; + QQuickCanvasPrivate::get(canvas)->deliverDragEvent(&dragGrabber, event); + inEvent = false; +} + QQuickDragAttached::QQuickDragAttached(QObject *parent) : QObject(*new QQuickDragAttachedPrivate, parent) { @@ -168,7 +179,9 @@ void QQuickDragAttached::setActive(bool active) { Q_D(QQuickDragAttached); if (d->active != active) { - if (active) + if (d->inEvent) + qmlInfo(this) << "active cannot be changed from within a drag event handler"; + else if (active) d->start(d->supportedActions); else cancel(); @@ -345,7 +358,7 @@ void QQuickDragAttachedPrivate::start(Qt::DropActions supportedActions) QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint(); QDragEnterEvent event(scenePos, supportedActions, mimeData, Qt::NoButton, Qt::NoModifier); QQuickDropEventEx::setProposedAction(&event, proposedAction); - QQuickCanvasPrivate::get(canvas)->deliverDragEvent(&dragGrabber, &event); + deliverEvent(canvas, &event); emit q->activeChanged(); if (target != dragGrabber.target()) { @@ -367,6 +380,11 @@ void QQuickDragAttachedPrivate::start(Qt::DropActions supportedActions) void QQuickDragAttached::start(QDeclarativeV8Function *args) { Q_D(QQuickDragAttached); + if (d->inEvent) { + qmlInfo(this) << "start() cannot be called from within a drag event handler"; + return; + } + if (d->active) cancel(); @@ -405,8 +423,14 @@ int QQuickDragAttached::drop() Q_D(QQuickDragAttached); Qt::DropAction acceptedAction = Qt::IgnoreAction; + if (d->inEvent) { + qmlInfo(this) << "drop() cannot be called from within a drag event handler"; + return acceptedAction; + } + if (!d->active) return acceptedAction; + d->active = false; QObject *target = 0; @@ -416,7 +440,7 @@ int QQuickDragAttached::drop() QDropEvent event( scenePos, d->mimeData->m_supportedActions, d->mimeData, Qt::NoButton, Qt::NoModifier); QQuickDropEventEx::setProposedAction(&event, d->proposedAction); - QQuickCanvasPrivate::get(canvas)->deliverDragEvent(&d->dragGrabber, &event); + d->deliverEvent(canvas, &event); if (event.isAccepted()) { acceptedAction = event.dropAction(); @@ -424,7 +448,6 @@ int QQuickDragAttached::drop() } } - d->active = false; if (d->target != target) { d->target = target; emit targetChanged(); @@ -443,15 +466,21 @@ int QQuickDragAttached::drop() void QQuickDragAttached::cancel() { Q_D(QQuickDragAttached); + + if (d->inEvent) { + qmlInfo(this) << "cancel() cannot be called from within a drag event handler"; + return; + } + if (!d->active) return; + d->active = false; if (QQuickCanvas *canvas = d->attachedItem->canvas()) { QDragLeaveEvent event; - QQuickCanvasPrivate::get(canvas)->deliverDragEvent(&d->dragGrabber, &event); + d->deliverEvent(canvas, &event); } - d->active = false; if (d->target) { d->target = 0; emit targetChanged(); diff --git a/src/quick/items/qquickdrag_p.h b/src/quick/items/qquickdrag_p.h index 7b568ec..9390401 100644 --- a/src/quick/items/qquickdrag_p.h +++ b/src/quick/items/qquickdrag_p.h @@ -90,6 +90,8 @@ public: void setTarget(QObject *target) { m_target = target; } void resetTarget() { m_target = 0; } + bool isEmpty() const { return m_items.isEmpty(); } + typedef ItemList::iterator iterator; iterator begin() { return m_items.begin(); } iterator end() { return m_items.end(); } diff --git a/tests/auto/qtquick2/qquickdrag/tst_qquickdrag.cpp b/tests/auto/qtquick2/qquickdrag/tst_qquickdrag.cpp index c2e195e..91edf8c 100644 --- a/tests/auto/qtquick2/qquickdrag/tst_qquickdrag.cpp +++ b/tests/auto/qtquick2/qquickdrag/tst_qquickdrag.cpp @@ -159,6 +159,8 @@ private slots: void proposedAction(); void keys(); void source(); + void recursion_data(); + void recursion(); private: QDeclarativeEngine engine; @@ -822,6 +824,211 @@ void tst_QQuickDrag::source() QCOMPARE(evaluate(item, "source"), static_cast(item)); } +class RecursingDropTarget : public TestDropTarget +{ +public: + RecursingDropTarget(const QString &script, int type, QQuickItem *parent) + : TestDropTarget(parent), script(script), type(type), item(0) {} + + void setItem(QQuickItem *i) { item = i; } + +protected: + void dragEnterEvent(QDragEnterEvent *event) + { + TestDropTarget::dragEnterEvent(event); + if (type == QEvent::DragEnter && enterEvents < 2) + evaluate(item, script); + } + + void dragMoveEvent(QDragMoveEvent *event) + { + TestDropTarget::dragMoveEvent(event); + if (type == QEvent::DragMove && moveEvents < 2) + evaluate(item, script); + } + + void dragLeaveEvent(QDragLeaveEvent *event) + { + TestDropTarget::dragLeaveEvent(event); + if (type == QEvent::DragLeave && leaveEvents < 2) + evaluate(item, script); + } + + void dropEvent(QDropEvent *event) + { + TestDropTarget::dropEvent(event); + if (type == QEvent::Drop && dropEvents < 2) + evaluate(item, script); + } + +private: + QString script; + int type; + QQuickItem *item; + +}; + +void tst_QQuickDrag::recursion_data() +{ + QTest::addColumn("script"); + QTest::addColumn("type"); + QTest::addColumn("warning"); + + QTest::newRow("Drag.start() in Enter") + << QString("Drag.start()") + << int(QEvent::DragEnter) + << QByteArray(": QML QQuickDragAttached: start() cannot be called from within a drag event handler"); + QTest::newRow("Drag.cancel() in Enter") + << QString("Drag.cancel()") + << int(QEvent::DragEnter) + << QByteArray(": QML QQuickDragAttached: cancel() cannot be called from within a drag event handler"); + QTest::newRow("Drag.drop() in Enter") + << QString("Drag.drop()") + << int(QEvent::DragEnter) + << QByteArray(": QML QQuickDragAttached: drop() cannot be called from within a drag event handler"); + QTest::newRow("Drag.active = true in Enter") + << QString("Drag.active = true") + << int(QEvent::DragEnter) + << QByteArray(); + QTest::newRow("Drag.active = false in Enter") + << QString("Drag.active = false") + << int(QEvent::DragEnter) + << QByteArray(": QML QQuickDragAttached: active cannot be changed from within a drag event handler"); + QTest::newRow("move in Enter") + << QString("x = 23") + << int(QEvent::DragEnter) + << QByteArray(); + + QTest::newRow("Drag.start() in Move") + << QString("Drag.start()") + << int(QEvent::DragMove) + << QByteArray(": QML QQuickDragAttached: start() cannot be called from within a drag event handler"); + QTest::newRow("Drag.cancel() in Move") + << QString("Drag.cancel()") + << int(QEvent::DragMove) + << QByteArray(": QML QQuickDragAttached: cancel() cannot be called from within a drag event handler"); + QTest::newRow("Drag.drop() in Move") + << QString("Drag.drop()") + << int(QEvent::DragMove) + << QByteArray(": QML QQuickDragAttached: drop() cannot be called from within a drag event handler"); + QTest::newRow("Drag.active = true in Move") + << QString("Drag.active = true") + << int(QEvent::DragMove) + << QByteArray(); + QTest::newRow("Drag.active = false in Move") + << QString("Drag.active = false") + << int(QEvent::DragMove) + << QByteArray(": QML QQuickDragAttached: active cannot be changed from within a drag event handler"); + QTest::newRow("move in Move") + << QString("x = 23") + << int(QEvent::DragMove) + << QByteArray(); + + QTest::newRow("Drag.start() in Leave") + << QString("Drag.start()") + << int(QEvent::DragLeave) + << QByteArray(": QML QQuickDragAttached: start() cannot be called from within a drag event handler"); + QTest::newRow("Drag.cancel() in Leave") + << QString("Drag.cancel()") + << int(QEvent::DragLeave) + << QByteArray(": QML QQuickDragAttached: cancel() cannot be called from within a drag event handler"); + QTest::newRow("Drag.drop() in Leave") + << QString("Drag.drop()") + << int(QEvent::DragLeave) + << QByteArray(": QML QQuickDragAttached: drop() cannot be called from within a drag event handler"); + QTest::newRow("Drag.active = true in Leave") + << QString("Drag.active = true") + << int(QEvent::DragLeave) + << QByteArray(": QML QQuickDragAttached: active cannot be changed from within a drag event handler"); + QTest::newRow("Drag.active = false in Leave") + << QString("Drag.active = false") + << int(QEvent::DragLeave) + << QByteArray(); + QTest::newRow("move in Leave") + << QString("x = 23") + << int(QEvent::DragLeave) + << QByteArray(); + + QTest::newRow("Drag.start() in Drop") + << QString("Drag.start()") + << int(QEvent::Drop) + << QByteArray(": QML QQuickDragAttached: start() cannot be called from within a drag event handler"); + QTest::newRow("Drag.cancel() in Drop") + << QString("Drag.cancel()") + << int(QEvent::Drop) + << QByteArray(": QML QQuickDragAttached: cancel() cannot be called from within a drag event handler"); + QTest::newRow("Drag.drop() in Drop") + << QString("Drag.drop()") + << int(QEvent::Drop) + << QByteArray(": QML QQuickDragAttached: drop() cannot be called from within a drag event handler"); + QTest::newRow("Drag.active = true in Drop") + << QString("Drag.active = true") + << int(QEvent::Drop) + << QByteArray(": QML QQuickDragAttached: active cannot be changed from within a drag event handler"); + QTest::newRow("Drag.active = false in Drop") + << QString("Drag.active = false") + << int(QEvent::Drop) + << QByteArray(); + QTest::newRow("move in Drop") + << QString("x = 23") + << int(QEvent::Drop) + << QByteArray(); +} + +void tst_QQuickDrag::recursion() +{ + QFETCH(QString, script); + QFETCH(int, type); + QFETCH(QByteArray, warning); + + if (!warning.isEmpty()) + QTest::ignoreMessage(QtWarningMsg, warning.constData()); + + QQuickCanvas canvas; + RecursingDropTarget dropTarget(script, type, canvas.rootItem()); + dropTarget.setSize(QSizeF(100, 100)); + QDeclarativeComponent component(&engine); + component.setData( + "import QtQuick 2.0\n" + "Item {\n" + "x: 50; y: 50\n" + "width: 10; height: 10\n" + "}", QUrl()); + QScopedPointer object(component.create()); + QQuickItem *item = qobject_cast(object.data()); + QVERIFY(item); + item->setParentItem(canvas.rootItem()); + + dropTarget.setItem(item); + + evaluate(item, "Drag.start()"); + QCOMPARE(dropTarget.enterEvents, 1); + QCOMPARE(dropTarget.moveEvents, 0); + QCOMPARE(dropTarget.dropEvents, 0); + QCOMPARE(dropTarget.leaveEvents, 0); + + evaluate(item, "y = 15"); + QCOMPARE(dropTarget.enterEvents, 1); + QCOMPARE(dropTarget.moveEvents, 1); + QCOMPARE(dropTarget.dropEvents, 0); + QCOMPARE(dropTarget.leaveEvents, 0); + + if (type == QEvent::Drop) { + QCOMPARE(evaluate(item, "Drag.drop() == Qt.MoveAction"), true); + QCOMPARE(dropTarget.enterEvents, 1); + QCOMPARE(dropTarget.moveEvents, 1); + QCOMPARE(dropTarget.dropEvents, 1); + QCOMPARE(dropTarget.leaveEvents, 0); + } else { + evaluate(item, "Drag.cancel()"); + QCOMPARE(dropTarget.enterEvents, 1); + QCOMPARE(dropTarget.moveEvents, 1); + QCOMPARE(dropTarget.dropEvents, 0); + QCOMPARE(dropTarget.leaveEvents, 1); + } +} + + QTEST_MAIN(tst_QQuickDrag) #include "tst_qquickdrag.moc" -- 2.7.4