Merge lp:~dandrader/unity8/draghandle_lp1269022 into lp:unity8

[Daniel d'Andrada]

DragHandle: Never restart hinting animation while still pressed

Are there any related MPs required for this MP to build/function as expected? Please list.
No

Did you perform an exploratory manual test run of your code change and any related functionality?
On the desktop, using "make tryDragHandle".

If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Didn't change

If you changed the UI, has there been a design review?
Didn't change

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:649
http://jenkins.qa.ubuntu.com/job/unity8-ci/2102/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2498
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2326
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/969
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/624
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/626
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/626/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/624
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2177
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2500
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2500/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2326
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2326/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4770
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3229

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2102/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:649
http://jenkins.qa.ubuntu.com/job/unity8-ci/2104/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2502/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2328
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/971
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/626
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/628
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/628/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/626
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2181/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2504
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2504/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2328
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2328/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4772
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3232

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2104/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:649
http://jenkins.qa.ubuntu.com/job/unity8-ci/2107/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2517
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2334
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/974
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/631
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/631/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/629
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2195
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2519
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2519/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2334
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2334/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4777
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3244

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2107/rebuild

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Code looks sane, new test fails without the code and passes with it :-)

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/957/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3951
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2529
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2346/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/980
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/343
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/343
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/343/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/343
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2207
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2531
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2531/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2346
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2346/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4789/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3262

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/959/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3957
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2546
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2351
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/983
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/345
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/345
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/345/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/345
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2223
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2548
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2548/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2352
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2352/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4794
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3280

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/960/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3961
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2550
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2353
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/984
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/346
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/346
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/346/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/346
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2226
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2552
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2552/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2354
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2354/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/4796
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3284

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

 * you perform an exploratory manual test run of the code change and any related functionality?
   Yes, run on the phone and the bug was gone

 * Did CI run pass? If not, please explain why.
   No, notification tests are broken

=== modified file 'qml/Components/DragHandle.qml'
--- qml/Components/DragHandle.qml	2013-11-08 08:48:11 +0000
+++ qml/Components/DragHandle.qml	2014-01-21 15:34:18 +0000
@@ -97,7 +97,7 @@
     // Private stuff
     QtObject {
         id: d
-        property var previousStatus: undefined
+        property var previousStatus: DirectionalDragArea.WaitingForTouch
         property real startValue
         property real minValue: Direction.isPositive(direction) ? startValue
                                                                 : startValue - maxTotalDragDistance
@@ -193,14 +193,14 @@
                 d.rollbackDrag();
             }
         } else /* Undecided || Recognized */ {
-            if (d.previousStatus === DirectionalDragArea.WaitingForTouch ||
-                    d.previousStatus === undefined) {
+            if (d.previousStatus === DirectionalDragArea.WaitingForTouch) {
                 dragEvaluator.reset();
                 d.startValue = parent[d.targetProp];
-            }
-            if (hintDisplacement > 0) {
-                hintingAnimation.targetValue = d.startValue;
-                hintingAnimation.start();
+
+                if (hintDisplacement > 0) {
+                    hintingAnimation.targetValue = d.startValue;
+                    hintingAnimation.start();
+                }
             }
         }
 

=== modified file 'tests/qmltests/Components/tst_DragHandle.cpp'
--- tests/qmltests/Components/tst_DragHandle.cpp	2013-10-22 15:56:37 +0000
+++ tests/qmltests/Components/tst_DragHandle.cpp	2014-01-21 15:34:18 +0000
@@ -62,9 +62,13 @@
     void stretch_horizontal();
     void stretch_vertical();
     void hintingAnimation();
+    void hintingAnimation_dontRestartAfterFinishedAndStillPressed();
+
 
 private:
     void flickAndHold(DirectionalDragArea *dragHandle, qreal distance);
+    void drag(QPointF &touchPoint, const QPointF& direction, qreal distance,
+              int numSteps, qint64 timeMs = 500);
     DirectionalDragArea *fetchAndSetupDragHandle(const char *objectName);
     qreal fetchDragThreshold(DirectionalDragArea *dragHandle);
     void tryCompare(std::function<qreal ()> actualFunc, qreal expectedValue);
@@ -161,25 +165,29 @@
     QPointF initialTouchPos = dragHandle->mapToScene(
         QPointF(dragHandle->width() / 2.0, dragHandle->height() / 2.0));
     QPointF touchPoint = initialTouchPos;
+
+    QTest::touchEvent(m_view, m_device).press(0, touchPoint.toPoint());
+
     int numSteps = 10;
-    qint64 flickTimeMs = 500;
-    qint64 timeStep = flickTimeMs / numSteps;
-
     QPointF dragDirectionVector = calculateDirectionVector(dragHandle);
-    QPointF touchMovement = dragDirectionVector * (distance / (qreal)numSteps);
-
-    QTest::touchEvent(m_view, m_device).press(0, touchPoint.toPoint());
-
+    drag(touchPoint, dragDirectionVector, distance, numSteps);
+
+    // Wait for quite a bit before finally releasing to make a very low flick/release
+    // speed.
+    m_fakeTimeSource->m_msecsSinceReference += 5000;
+    QTest::touchEvent(m_view, m_device).release(0, touchPoint.toPoint());
+}
+
+void tst_DragHandle::drag(QPointF &touchPoint, const QPointF& direction, qreal distance,
+                          int numSteps, qint64 timeMs)
+{
+    qint64 timeStep = timeMs / numSteps;
+    QPointF touchMovement = direction * (distance / (qreal)numSteps);
     for (int i = 0; i < numSteps; ++i) {
         touchPoint += touchMovement;
         m_fakeTimeSource->m_msecsSinceReference += timeStep;
         QTest::touchEvent(m_view, m_device).move(0, touchPoint.toPoint());
     }
-
-    // Wait for quite a bit before finally releasing to make a very low flick/release
-    // speed.
-    m_fakeTimeSource->m_msecsSinceReference += 5000;
-    QTest::touchEvent(m_view, m_device).release(0, touchPoint.toPoint());
 }
 
 DirectionalDragArea *tst_DragHandle::fetchAndSetupDragHandle(const char *objectName)
@@ -389,6 +397,53 @@
     QCOMPARE(parentItem->property("shown").toBool(), false);
 }
 
+/*
+    Regression test for LP#1269022: https://bugs.launchpad.net/unity8/+bug/1269022
+
+    1) Click on handle.
+    2) wait for hint portion to appear
+    3) slowly drag handle, only a few pixels
+
+    Expected outcome:
+        Nothing happens
+
+    Actual outcome:
+        Handle will momentarily move back to zero position, then back down to the
+        hint displacement location.
+ */
+void tst_DragHandle::hintingAnimation_dontRestartAfterFinishedAndStillPressed()
+{
+    DirectionalDragArea *dragHandle = fetchAndSetupDragHandle("downwardsDragHandle");
+    QQuickItem *parentItem = dragHandle->parentItem();
+    qreal hintDisplacement = 100.0;
+
+    // enable hinting animations
+    m_view->rootObject()->setProperty("hintDisplacement", QVariant(hintDisplacement));
+    m_view->rootObject()->setProperty("stretch", QVariant(true));
+
+    QCOMPARE(parentItem->height(), 0.0);
+
+    QPointF initialTouchPos = dragHandle->mapToScene(
+        QPointF(dragHandle->width() / 2.0, dragHandle->height() / 2.0));
+    QPointF touchPoint = initialTouchPos;
+
+    // Pressing causes the Showable to be stretched by hintDisplacement pixels
+    const int touchId = 0;
+    QTest::touchEvent(m_view, m_device).press(touchId, touchPoint.toPoint());
+    tryCompare([&](){ return parentItem->height(); }, hintDisplacement);
+
+
+    QSignalSpy parentHeightChangedSpy(parentItem, SIGNAL(heightChanged()));
+
+    drag(touchPoint, QPointF(0.0, -1.0) /*dragDirectionVector*/, 15 /*distance*/, 3 /*numSteps*/);
+
+    // Give some time for animations to run, if any
+    QTest::qWait(300);
+
+    // parentItem height shouldn't have changed at all
+    QVERIFY(parentHeightChangedSpy.isEmpty());
+}
+
 QTEST_MAIN(tst_DragHandle)
 
 #include "tst_DragHandle.moc"