Restart a drag when the item canvas changes.
authorAndrew den Exter <andrew.den-exter@nokia.com>
Thu, 10 May 2012 04:01:15 +0000 (14:01 +1000)
committerQt by Nokia <qt-info@nokia.com>
Wed, 6 Jun 2012 06:00:12 +0000 (08:00 +0200)
Likewise when a property that affects whether a drag event is likely to
be accepted.  Properties were locked in when a drag was started, but
this means that if a drag was started by setting the active property
in a state change or similar bulk change the properties that are
locked in is undefined. And if an item changed canvases during a drag
or the active property was set before a parent enter and leave events
weren't sent as they should have been.

Change-Id: Iefd22dcab4c5469904d8d8c5eaf91ec339540d9c
Reviewed-by: Martin Jones <martin.jones@nokia.com>
src/quick/items/qquickdrag.cpp
tests/auto/quick/qquickapplication/qquickapplication.pro
tests/auto/quick/qquickdrag/tst_qquickdrag.cpp
tests/auto/quick/quick.pro

index 6f0e677..a6fe87d 100644 (file)
@@ -66,13 +66,20 @@ public:
         , active(false)
         , listening(false)
         , inEvent(false)
+        , dragRestarted(false)
         , itemMoved(false)
         , eventQueued(false)
+        , overrideActions(false)
     {
     }
 
     void itemGeometryChanged(QQuickItem *, const QRectF &, const QRectF &);
+    void itemParentChanged(QQuickItem *, QQuickItem *parent);
+    void updatePosition();
+    void restartDrag();
+    void deliverEnterEvent();
     void deliverMoveEvent();
+    void deliverLeaveEvent();
     void deliverEvent(QQuickCanvas *canvas, QEvent *event);
     void start() { start(supportedActions); }
     void start(Qt::DropActions supportedActions);
@@ -82,6 +89,7 @@ public:
 
     QQmlGuard<QObject> source;
     QQmlGuard<QObject> target;
+    QQmlGuard<QQuickCanvas> canvas;
     QQuickItem *attachedItem;
     QQuickDragMimeData *mimeData;
     Qt::DropAction proposedAction;
@@ -89,8 +97,10 @@ public:
     bool active : 1;
     bool listening : 1;
     bool inEvent : 1;
+    bool dragRestarted : 1;
     bool itemMoved : 1;
     bool eventQueued : 1;
+    bool overrideActions : 1;
     QPointF hotSpot;
     QStringList keys;
 };
@@ -128,21 +138,69 @@ void QQuickDragAttachedPrivate::itemGeometryChanged(QQuickItem *, const QRectF &
     Q_Q(QQuickDragAttached);
     if (newGeometry.topLeft() == oldGeometry.topLeft() || !active || itemMoved)
         return;
+    updatePosition();
+}
+
+void QQuickDragAttachedPrivate::itemParentChanged(QQuickItem *, QQuickItem *)
+{
+    Q_Q(QQuickDragAttached);
+    if (!active || dragRestarted)
+        return;
+
+    QQuickCanvas *newCanvas = attachedItem->canvas();
+
+    if (canvas != newCanvas)
+        restartDrag();
+    else if (canvas)
+        updatePosition();
+}
 
+void QQuickDragAttachedPrivate::updatePosition()
+{
+    Q_Q(QQuickDragAttached);
     itemMoved = true;
+    if (!eventQueued) {
+        eventQueued = true;
+        QCoreApplication::postEvent(q, new QEvent(QEvent::User));
+    }
+}
 
+void QQuickDragAttachedPrivate::restartDrag()
+{
+    Q_Q(QQuickDragAttached);
+    dragRestarted = true;
     if (!eventQueued) {
         eventQueued = true;
         QCoreApplication::postEvent(q, new QEvent(QEvent::User));
     }
 }
 
+void QQuickDragAttachedPrivate::deliverEnterEvent()
+{
+    dragRestarted = false;
+    itemMoved = false;
+
+    canvas = attachedItem->canvas();
+
+    mimeData->m_source = source;
+    if (!overrideActions)
+        mimeData->m_supportedActions = supportedActions;
+    mimeData->m_keys = keys;
+
+    if (canvas) {
+        QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint();
+        QDragEnterEvent event(scenePos, mimeData->m_supportedActions, mimeData, Qt::NoButton, Qt::NoModifier);
+        QQuickDropEventEx::setProposedAction(&event, proposedAction);
+        deliverEvent(canvas, &event);
+    }
+}
+
 void QQuickDragAttachedPrivate::deliverMoveEvent()
 {
     Q_Q(QQuickDragAttached);
 
     itemMoved = false;
-    if (QQuickCanvas *canvas = attachedItem->canvas()) {
+    if (canvas) {
         QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint();
         QDragMoveEvent event(scenePos, mimeData->m_supportedActions, mimeData, Qt::NoButton, Qt::NoModifier);
         QQuickDropEventEx::setProposedAction(&event, proposedAction);
@@ -154,6 +212,15 @@ void QQuickDragAttachedPrivate::deliverMoveEvent()
     }
 }
 
+void QQuickDragAttachedPrivate::deliverLeaveEvent()
+{
+    if (canvas) {
+        QDragLeaveEvent event;
+        deliverEvent(canvas, &event);
+        canvas = 0;
+    }
+}
+
 void QQuickDragAttachedPrivate::deliverEvent(QQuickCanvas *canvas, QEvent *event)
 {
     Q_ASSERT(!inEvent);
@@ -168,8 +235,17 @@ bool QQuickDragAttached::event(QEvent *event)
 
     if (event->type() == QEvent::User) {
         d->eventQueued = false;
-        if (d->itemMoved)
+        if (d->dragRestarted) {
+            d->deliverLeaveEvent();
+            d->deliverEnterEvent();
+
+            if (d->target != d->dragGrabber.target()) {
+                d->target = d->dragGrabber.target();
+                emit targetChanged();
+            }
+        } else if (d->itemMoved) {
             d->deliverMoveEvent();
+        }
         return true;
     } else {
         return QObject::event(event);
@@ -228,7 +304,8 @@ void QQuickDragAttached::setActive(bool active)
     This property holds an object that is identified to recipients of drag events as
     the source of the events.  By default this is the item Drag property is attached to.
 
-    Changes to source while a Drag is active don't take effect until a new drag is started.
+    Changing the source while a drag is active will reset the sequence of drag events by
+    sending a drag leave event followed by a drag enter event with the new source.
 */
 
 QObject *QQuickDragAttached::source() const
@@ -242,6 +319,8 @@ void QQuickDragAttached::setSource(QObject *item)
     Q_D(QQuickDragAttached);
     if (d->source != item) {
         d->source = item;
+        if (d->active)
+            d->restartDrag();
         emit sourceChanged();
     }
 }
@@ -251,6 +330,8 @@ void QQuickDragAttached::resetSource()
     Q_D(QQuickDragAttached);
     if (d->source != d->attachedItem) {
         d->source = d->attachedItem;
+        if (d->active)
+            d->restartDrag();
         emit sourceChanged();
     }
 }
@@ -280,7 +361,7 @@ QObject *QQuickDragAttached::target() const
 
     By default this is (0, 0).
 
-    Changes to hotSpot will take effect when the next event is sent.
+    Changes to hotSpot trigger a new drag move with the updated position.
 */
 
 QPointF QQuickDragAttached::hotSpot() const
@@ -295,14 +376,8 @@ void QQuickDragAttached::setHotSpot(const QPointF &hotSpot)
     if (d->hotSpot != hotSpot) {
         d->hotSpot = hotSpot;
 
-        if (d->active) {
-            d->itemMoved = true;
-
-            if (!d->eventQueued) {
-                d->eventQueued = true;
-                QCoreApplication::postEvent(this, new QEvent(QEvent::User));
-            }
-        }
+        if (d->active)
+            d->updatePosition();
 
         emit hotSpotChanged();
     }
@@ -313,7 +388,8 @@ void QQuickDragAttached::setHotSpot(const QPointF &hotSpot)
 
     This property holds a list of keys that can be used by a DropArea to filter drag events.
 
-    Changes to keys while a Drag is active don't take effect until a new drag is started.
+    Changing the keys while a drag is active will reset the sequence of drag events by
+    sending a drag leave event followed by a drag enter event with the new source.
 */
 
 QStringList QQuickDragAttached::keys() const
@@ -327,6 +403,8 @@ void QQuickDragAttached::setKeys(const QStringList &keys)
     Q_D(QQuickDragAttached);
     if (d->keys != keys) {
         d->keys = keys;
+        if (d->active)
+            d->restartDrag();
         emit keysChanged();
     }
 }
@@ -336,8 +414,8 @@ void QQuickDragAttached::setKeys(const QStringList &keys)
 
     This property holds return values of Drag.drop() supported by the drag source.
 
-    Changes to supportedActions while a Drag is active don't take effect
-    until a new drag is started.
+    Changing the supportedActions while a drag is active will reset the sequence of drag
+    events by sending a drag leave event followed by a drag enter event with the new source.
 */
 
 Qt::DropActions QQuickDragAttached::supportedActions() const
@@ -351,6 +429,8 @@ void QQuickDragAttached::setSupportedActions(Qt::DropActions actions)
     Q_D(QQuickDragAttached);
     if (d->supportedActions != actions) {
         d->supportedActions = actions;
+        if (d->active)
+            d->restartDrag();
         emit supportedActionsChanged();
     }
 }
@@ -361,7 +441,7 @@ void QQuickDragAttached::setSupportedActions(Qt::DropActions actions)
     This property holds an action that is recommended by the drag source as a
     return value from Drag.drop().
 
-    Changes to proposedAction will take effect when the next event is sent.
+    Changes to proposedAction will trigger a move event with the updated proposal.
 */
 
 Qt::DropAction QQuickDragAttached::proposedAction() const
@@ -375,8 +455,12 @@ void QQuickDragAttached::setProposedAction(Qt::DropAction action)
     Q_D(QQuickDragAttached);
     if (d->proposedAction != action) {
         d->proposedAction = action;
+        // The proposed action shouldn't affect whether a drag is accepted
+        // so leave/enter events are excessive, but the target should still
+        // updated.
+        if (d->active)
+            d->updatePosition();
         emit proposedActionChanged();
-        // send a move event with the new default action if active?
     }
 }
 
@@ -385,30 +469,27 @@ void QQuickDragAttachedPrivate::start(Qt::DropActions supportedActions)
     Q_Q(QQuickDragAttached);
     Q_ASSERT(!active);
 
-    if (QQuickCanvas *canvas = attachedItem ? attachedItem->canvas() : 0) {
-        if (!mimeData)
-            mimeData = new QQuickDragMimeData;
-        if (!listening) {
-            QQuickItemPrivate::get(attachedItem)->addItemChangeListener(this, QQuickItemPrivate::Geometry);
-            listening = true;
-        }
+    if (!mimeData)
+        mimeData = new QQuickDragMimeData;
+    if (!listening) {
+        QQuickItemPrivate::get(attachedItem)->addItemChangeListener(
+                this, QQuickItemPrivate::Geometry | QQuickItemPrivate::Parent);
+        listening = true;
+    }
 
-        mimeData->m_source = source;
-        mimeData->m_supportedActions = supportedActions;
-        mimeData->m_keys = keys;
-        active = true;
+    mimeData->m_supportedActions = supportedActions;
+    active = true;
+    itemMoved = false;
+    dragRestarted = false;
 
-        QPoint scenePos = attachedItem->mapToScene(hotSpot).toPoint();
-        QDragEnterEvent event(scenePos, supportedActions, mimeData, Qt::NoButton, Qt::NoModifier);
-        QQuickDropEventEx::setProposedAction(&event, proposedAction);
-        deliverEvent(canvas, &event);
+    deliverEnterEvent();
 
-        emit q->activeChanged();
-        if (target != dragGrabber.target()) {
-            target = dragGrabber.target();
-            emit q->targetChanged();
-        }
+    if (target != dragGrabber.target()) {
+        target = dragGrabber.target();
+        emit q->targetChanged();
     }
+
+    emit q->activeChanged();
 }
 
 /*!
@@ -431,12 +512,15 @@ void QQuickDragAttached::start(QQmlV8Function *args)
     if (d->active)
         cancel();
 
+    d->overrideActions = false;
     Qt::DropActions supportedActions = d->supportedActions;
     // check arguments for supportedActions, maybe data?
     if (args->Length() >= 1) {
         v8::Local<v8::Value> v = (*args)[0];
-        if (v->IsInt32())
+        if (v->IsInt32()) {
             supportedActions = Qt::DropActions(v->Int32Value());
+            d->overrideActions = true;
+        }
     }
 
     d->start(supportedActions);
@@ -480,13 +564,13 @@ int QQuickDragAttached::drop()
 
     QObject *target = 0;
 
-    if (QQuickCanvas *canvas = d->attachedItem->canvas()) {
+    if (d->canvas) {
         QPoint scenePos = d->attachedItem->mapToScene(d->hotSpot).toPoint();
 
         QDropEvent event(
                 scenePos, d->mimeData->m_supportedActions, d->mimeData, Qt::NoButton, Qt::NoModifier);
         QQuickDropEventEx::setProposedAction(&event, d->proposedAction);
-        d->deliverEvent(canvas, &event);
+        d->deliverEvent(d->canvas, &event);
 
         if (event.isAccepted()) {
             acceptedAction = event.dropAction();
@@ -521,17 +605,13 @@ void QQuickDragAttached::cancel()
     if (!d->active)
         return;
     d->active = false;
-    d->itemMoved = false;
-
-    if (QQuickCanvas *canvas = d->attachedItem->canvas()) {
-        QDragLeaveEvent event;
-        d->deliverEvent(canvas, &event);
-    }
+    d->deliverLeaveEvent();
 
     if (d->target) {
         d->target = 0;
         emit targetChanged();
     }
+
     emit activeChanged();
 }
 
index 4545912..b59e69d 100644 (file)
@@ -4,5 +4,5 @@ TARGET = tst_qquickapplication
 macx:CONFIG -= app_bundle
 
 SOURCES += tst_qquickapplication.cpp
-QT += core-private gui-private qml-private quick-private testlib
+QT += core-private gui-private qml quick qml-private quick-private testlib
 
index 0f8449f..0ebfa81 100644 (file)
@@ -154,6 +154,7 @@ private slots:
     void active();
     void drop();
     void move();
+    void parentChange();
     void hotSpot();
     void supportedActions();
     void proposedAction();
@@ -676,6 +677,93 @@ void tst_QQuickDrag::move()
     QCOMPARE(outerTarget.position.x(), qreal(25)); QCOMPARE(outerTarget.position.y(), qreal(50));
 }
 
+void tst_QQuickDrag::parentChange()
+{
+    QQuickCanvas canvas1;
+    TestDropTarget dropTarget1(canvas1.rootItem());
+    dropTarget1.setSize(QSizeF(100, 100));
+
+    QQuickCanvas canvas2;
+    TestDropTarget dropTarget2(canvas2.rootItem());
+    dropTarget2.setSize(QSizeF(100, 100));
+
+    QQmlComponent component(&engine);
+    component.setData(
+            "import QtQuick 2.0\n"
+            "Item {\n"
+                "property real hotSpotX: Drag.hotSpot.x\n"
+                "property real hotSpotY: Drag.hotSpot.y\n"
+                "x: 50; y: 50\n"
+                "width: 10; height: 10\n"
+                "Drag.active: true\n"
+            "}", QUrl());
+    QScopedPointer<QObject> object(component.create());
+    QQuickItem *item = qobject_cast<QQuickItem *>(object.data());
+    QVERIFY(item);
+
+    QCOMPARE(evaluate<bool>(item, "Drag.active"), true);
+
+    // Verify setting a parent item for an item with an active drag sends an enter event.
+    item->setParentItem(canvas1.rootItem());
+    QCOMPARE(dropTarget1.enterEvents, 0);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget1.enterEvents, 1);
+
+    // Changing the parent within the same canvas should send a move event.
+    item->setParentItem(&dropTarget1);
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 0);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+
+    // Changing the parent to an item in another canvas sends a leave event in the old canvas
+    // and an enter on the new canvas.
+    item->setParentItem(canvas2.rootItem());
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+    QCOMPARE(dropTarget1.leaveEvents, 0);
+    QCOMPARE(dropTarget2.enterEvents, 0);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+    QCOMPARE(dropTarget1.leaveEvents, 1);
+    QCOMPARE(dropTarget2.enterEvents, 1);
+
+    // Removing then parent item sends a leave event.
+    item->setParentItem(0);
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+    QCOMPARE(dropTarget1.leaveEvents, 1);
+    QCOMPARE(dropTarget2.enterEvents, 1);
+    QCOMPARE(dropTarget2.leaveEvents, 0);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+    QCOMPARE(dropTarget1.leaveEvents, 1);
+    QCOMPARE(dropTarget2.enterEvents, 1);
+    QCOMPARE(dropTarget2.leaveEvents, 1);
+
+    // Go around again and verify no events if active is false.
+    evaluate<void>(item, "Drag.active = false");
+    item->setParentItem(canvas1.rootItem());
+    QCoreApplication::processEvents();
+
+    item->setParentItem(&dropTarget1);
+    QCoreApplication::processEvents();
+
+    item->setParentItem(canvas2.rootItem());
+    QCoreApplication::processEvents();
+
+    item->setParentItem(0);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget1.enterEvents, 1);
+    QCOMPARE(dropTarget1.moveEvents, 1);
+    QCOMPARE(dropTarget1.leaveEvents, 1);
+    QCOMPARE(dropTarget2.enterEvents, 1);
+    QCOMPARE(dropTarget2.leaveEvents, 1);
+}
+
 void tst_QQuickDrag::hotSpot()
 {
     QQuickCanvas canvas;
@@ -763,18 +851,28 @@ void tst_QQuickDrag::supportedActions()
     evaluate<void>(item, "{ Drag.start(); Drag.cancel() }");
     QCOMPARE(dropTarget.supportedActions, Qt::CopyAction | Qt::MoveAction | Qt::LinkAction);
 
+    dropTarget.reset();
     evaluate<void>(item, "Drag.supportedActions = Qt.CopyAction | Qt.MoveAction");
     QCOMPARE(evaluate<bool>(item, "Drag.supportedActions == Qt.CopyAction | Qt.MoveAction"), true);
     QCOMPARE(evaluate<bool>(item, "supportedActions == Qt.CopyAction | Qt.MoveAction"), true);
     evaluate<void>(item, "Drag.start()");
     QCOMPARE(dropTarget.supportedActions, Qt::CopyAction | Qt::MoveAction);
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
 
-    // Once a drag is started the proposed actions are locked in for future events.
+    // Changing the supported actions will restart the drag, after a delay to avoid any
+    // recursion.
     evaluate<void>(item, "Drag.supportedActions = Qt.MoveAction");
     QCOMPARE(evaluate<bool>(item, "Drag.supportedActions == Qt.MoveAction"), true);
     QCOMPARE(evaluate<bool>(item, "supportedActions == Qt.MoveAction"), true);
     item->setPos(QPointF(60, 60));
     QCOMPARE(dropTarget.supportedActions, Qt::CopyAction | Qt::MoveAction);
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget.supportedActions, Qt::MoveAction);
+    QCOMPARE(dropTarget.leaveEvents, 1);
+    QCOMPARE(dropTarget.enterEvents, 2);
 
     // Calling start with proposed actions will override the current actions for the next sequence.
     evaluate<void>(item, "Drag.start(Qt.CopyAction)");
@@ -806,7 +904,6 @@ void tst_QQuickDrag::proposedAction()
     QVERIFY(item);
     item->setParentItem(&dropTarget);
 
-
     QCOMPARE(evaluate<bool>(item, "Drag.proposedAction == Qt.MoveAction"), true);
     QCOMPARE(evaluate<bool>(item, "proposedAction == Qt.MoveAction"), true);
     evaluate<void>(item, "{ Drag.start(); Drag.cancel() }");
@@ -824,7 +921,6 @@ void tst_QQuickDrag::proposedAction()
     evaluate<void>(item, "Drag.proposedAction = Qt.MoveAction");
     QCOMPARE(evaluate<bool>(item, "Drag.proposedAction == Qt.MoveAction"), true);
     QCOMPARE(evaluate<bool>(item, "proposedAction == Qt.MoveAction"), true);
-    item->setPos(QPointF(60, 60));
     QCoreApplication::processEvents();
     QCOMPARE(dropTarget.defaultAction, Qt::MoveAction);
     QCOMPARE(dropTarget.proposedAction, Qt::MoveAction);
@@ -859,6 +955,23 @@ void tst_QQuickDrag::keys()
     QCOMPARE(evaluate<QStringList>(item, "Drag.keys"), QStringList() << "red" << "blue");
     QCOMPARE(evaluate<QStringList>(item, "keys"), QStringList() << "red" << "blue");
     QCOMPARE(item->property("keys").toStringList(), QStringList() << "red" << "blue");
+
+    // Test changing the keys restarts a drag.
+    QQuickCanvas canvas;
+    item->setParentItem(canvas.rootItem());
+    TestDropTarget dropTarget(canvas.rootItem());
+    dropTarget.setSize(QSizeF(100, 100));
+
+    evaluate<void>(item, "Drag.start()");
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
+
+    evaluate<void>(item, "Drag.keys = [\"green\"]");
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget.leaveEvents, 1);
+    QCOMPARE(dropTarget.enterEvents, 2);
 }
 
 void tst_QQuickDrag::source()
@@ -890,6 +1003,23 @@ void tst_QQuickDrag::source()
     evaluate<void>(item, "Drag.source = undefined");
     QCOMPARE(evaluate<QObject *>(item, "Drag.source"), static_cast<QObject *>(item));
     QCOMPARE(evaluate<QObject *>(item, "source"), static_cast<QObject *>(item));
+
+    // Test changing the source restarts a drag.
+    QQuickCanvas canvas;
+    item->setParentItem(canvas.rootItem());
+    TestDropTarget dropTarget(canvas.rootItem());
+    dropTarget.setSize(QSizeF(100, 100));
+
+    evaluate<void>(item, "Drag.start()");
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
+
+    evaluate<void>(item, "Drag.source = proxySource");
+    QCOMPARE(dropTarget.leaveEvents, 0);
+    QCOMPARE(dropTarget.enterEvents, 1);
+    QCoreApplication::processEvents();
+    QCOMPARE(dropTarget.leaveEvents, 1);
+    QCOMPARE(dropTarget.enterEvents, 2);
 }
 
 class RecursingDropTarget : public TestDropTarget
index 1c73a3b..2ef6dc0 100644 (file)
@@ -44,6 +44,7 @@ QUICKTESTS =  \
     qquickborderimage \
     qquickcanvas \
     qquickdrag \
+    qquickdroparea \
     qquickflickable \
     qquickflipable \
     qquickfocusscope \