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 <martin.jones@nokia.com>

diff --git a/src/quick/items/qquickdrag.cpp b/src/quick/items/qquickdrag.cpp
index a906af3..c44e1ac 100644 (file)
--- a/src/quick/items/qquickdrag.cpp
+++ b/src/quick/items/qquickdrag.cpp
@@ -45,7 +45,7 @@
 #include <QtQuick/private/qquickevents_p_p.h>
 #include <private/qquickitemchangelistener_p.h>
 #include <private/qv8engine_p.h>
-
+#include <QtDeclarative/qdeclarativeinfo.h>
 #include <QtGui/qevent.h>
 
 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 (file)
--- 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 (file)
--- 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<QObject *>(item, "source"), static_cast<QObject *>(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<void>(item, script);
+    }
+
+    void dragMoveEvent(QDragMoveEvent *event)
+    {
+        TestDropTarget::dragMoveEvent(event);
+        if (type == QEvent::DragMove && moveEvents < 2)
+            evaluate<void>(item, script);
+    }
+
+    void dragLeaveEvent(QDragLeaveEvent *event)
+    {
+        TestDropTarget::dragLeaveEvent(event);
+        if (type == QEvent::DragLeave && leaveEvents < 2)
+            evaluate<void>(item, script);
+    }
+
+    void dropEvent(QDropEvent *event)
+    {
+        TestDropTarget::dropEvent(event);
+        if (type == QEvent::Drop && dropEvents < 2)
+            evaluate<void>(item, script);
+    }
+
+private:
+    QString script;
+    int type;
+    QQuickItem *item;
+
+};
+
+void tst_QQuickDrag::recursion_data()
+{
+    QTest::addColumn<QString>("script");
+    QTest::addColumn<int>("type");
+    QTest::addColumn<QByteArray>("warning");
+
+    QTest::newRow("Drag.start() in Enter")
+            << QString("Drag.start()")
+            << int(QEvent::DragEnter)
+            << QByteArray("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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("<Unknown File>: 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<QObject> object(component.create());
+    QQuickItem *item = qobject_cast<QQuickItem *>(object.data());
+    QVERIFY(item);
+    item->setParentItem(canvas.rootItem());
+
+    dropTarget.setItem(item);
+
+    evaluate<void>(item, "Drag.start()");
+    QCOMPARE(dropTarget.enterEvents, 1);
+    QCOMPARE(dropTarget.moveEvents, 0);
+    QCOMPARE(dropTarget.dropEvents, 0);
+    QCOMPARE(dropTarget.leaveEvents, 0);
+
+    evaluate<void>(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<bool>(item, "Drag.drop() == Qt.MoveAction"), true);
+        QCOMPARE(dropTarget.enterEvents, 1);
+        QCOMPARE(dropTarget.moveEvents, 1);
+        QCOMPARE(dropTarget.dropEvents, 1);
+        QCOMPARE(dropTarget.leaveEvents, 0);
+    } else {
+        evaluate<void>(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"