Merge lp:~cimi/unity8/preview-sharing-fix-broken-binding into lp:unity8

[Andrea Cimitan]

Fix a broken binding

 * Are there any related MPs required for this MP to build/function as expected? Please list.
from the prerequisite
 * Did you perform an exploratory manual test run of your code change and any related functionality?
y
 * Did you make sure that your branch does not contain spurious tags?
y
 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
n/a
 * If you changed the UI, has there been a design review?
n/a

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2082
http://jenkins.qa.ubuntu.com/job/unity8-ci/6974/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5851
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/389/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1684
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/387
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/386
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/385
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4510
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5862
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5862/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26167
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/150/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/387
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/387/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26168

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Is the problem here is that the visible of PreviewSharing got overwritten when some of the parents changed visibility?

Maybe you can write a test that shows it works now?

review: Needs Information

[Albert Astals Cid (aacid)]

"Maybe you can write a test that shows it works now?" -> i.e. a test that fails with this patch and works with it

[lp:~cimi/unity8/preview-sharing-fix-broken-binding]

2083. By Andrea Cimitan on 2016-01-04

Added test

[Andrea Cimitan (cimi)]

> "Maybe you can write a test that shows it works now?" -> i.e. a test that
> fails with this patch and works with it
added

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2083
http://jenkins.qa.ubuntu.com/job/unity8-ci/7009/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5918/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/424/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1714/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/417/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1609/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1609/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/416/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/415/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5929/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/422/console

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

23/23 Test #22: Whitespace .......................***Failed 1.04 sec
/tmp/buildd/unity8-8.11+16.04.20151208.1bzr2084pkg0vivid1714/tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml: multiple new lines at end of file

review: Needs Fixing

[lp:~cimi/unity8/preview-sharing-fix-broken-binding]

2084. By Andrea Cimitan on 2016-01-05

fix whitespace

[Andrea Cimitan (cimi)]

> 23/23 Test #22: Whitespace .......................***Failed 1.04 sec
> /tmp/buildd/unity8-8.11+16.04.20151208.1bzr2084pkg0vivid1714/tests/qmltests/Da
> sh/Previews/tst_PreviewMediaToolbar.qml: multiple new lines at end of file
fixed

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2084
http://jenkins.qa.ubuntu.com/job/unity8-ci/7014/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5926
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/429/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1719
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/422
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1614
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1614
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/421
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/420
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4574
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5937
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5937/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26411
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/183/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/427
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/427/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26412

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

review: Needs Fixing (continuous-integration)

[Albert Astals Cid (aacid)]

Code looks good and tests pass.

Is there a way this can be tested in real life or no scope uses this yet?

review: Needs Information

[Albert Astals Cid (aacid)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass?
Didn't regress

 * Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

=== modified file 'qml/Dash/Previews/PreviewMediaToolbar.qml'
--- qml/Dash/Previews/PreviewMediaToolbar.qml	2015-10-26 18:32:19 +0000
+++ qml/Dash/Previews/PreviewMediaToolbar.qml	2016-01-05 09:52:40 +0000
@@ -23,10 +23,11 @@
 
     implicitHeight: units.gu(6)
     color: Qt.rgba(0, 0, 0, 0.8)
-    visible: sharingWidget.visible
+    visible: sharingWidget.active
 
     PreviewSharing {
         id: sharingWidget
+        objectName: "sharingWidget"
         anchors {
             left: parent.left
             verticalCenter: parent.verticalCenter

=== modified file 'qml/Dash/Previews/PreviewSharing.qml'
--- qml/Dash/Previews/PreviewSharing.qml	2016-01-05 09:52:40 +0000
+++ qml/Dash/Previews/PreviewSharing.qml	2016-01-05 09:52:40 +0000
@@ -24,13 +24,12 @@
     implicitWidth: button.width
 
     property var shareData
+    property alias active: peerPicker.active
     readonly property bool isUrlExternal: url && url.indexOf("file:///") != 0 && url.indexOf("/") != 0
     readonly property string contentType: shareData ? shareData["content-type"] : ""
     readonly property var url: shareData ? shareData["uri"] : ""
     readonly property Item rootItem: QuickUtils.rootItem(root)
 
-    visible: url != ""
-
     AbstractButton {
         id: button
         height: units.gu(4)
@@ -98,7 +97,7 @@
         parent: rootItem
         anchors.fill: parent
         visible: false
-        active: root.visible
+        active: root.url != ""
 
         sourceComponent: contentPeerComponent
     }

=== modified file 'tests/qmltests/CMakeLists.txt'
--- tests/qmltests/CMakeLists.txt	2015-11-26 16:36:53 +0000
+++ tests/qmltests/CMakeLists.txt	2016-01-05 09:52:40 +0000
@@ -38,6 +38,7 @@
 add_unity8_qmltest(Dash/Previews PreviewHeader)
 add_unity8_qmltest(Dash/Previews PreviewIconActions)
 add_unity8_qmltest(Dash/Previews PreviewImageGallery)
+add_unity8_qmltest(Dash/Previews PreviewMediaToolbar)
 add_unity8_qmltest(Dash/Previews PreviewPayments)
 add_unity8_qmltest(Dash/Previews PreviewProgress)
 add_unity8_qmltest(Dash/Previews PreviewRatingDisplay)

=== added file 'tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml'
--- tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml	1970-01-01 00:00:00 +0000
+++ tests/qmltests/Dash/Previews/tst_PreviewMediaToolbar.qml	2016-01-05 09:52:40 +0000
@@ -0,0 +1,69 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.4
+import QtTest 1.0
+import Ubuntu.Components 1.3
+import "../../../../qml/Dash/Previews"
+import Unity.Test 0.1 as UT
+
+Rectangle {
+    id: root
+    width: units.gu(40)
+    height: units.gu(80)
+    color: "black"
+
+    property var shareData: {
+        "uri": [
+                    "Text here",
+                    "text here 2",
+                    "text here 3"
+                ],
+        "content-type": "text"
+    }
+
+    property var shareDataNoUri: {
+        "uri": "",
+        "content-type": "text"
+    }
+
+
+    PreviewMediaToolbar {
+        id: previewMediaToolbar
+        anchors { left: parent.left; bottom: parent.bottom; }
+        shareData: root.shareData
+    }
+
+    UT.UnityTestCase {
+        name: "PreviewMediaToolbarTest"
+        when: windowShown
+
+        property Item sharingWidget: findChild(previewMediaToolbar, "sharingWidget")
+
+        function cleanup() {
+            previewMediaToolbar.shareData = root.shareData;
+        }
+
+        function test_visible() {
+            previewMediaToolbar.shareData = shareDataNoUri;
+            compare(previewMediaToolbar.visible, false);
+            compare(sharingWidget.visible, false);
+            previewMediaToolbar.shareData = shareData;
+            compare(previewMediaToolbar.visible, true);
+            compare(sharingWidget.visible, true);
+        }
+    }
+}

=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewSharing.qml'
--- tests/qmltests/Dash/Previews/tst_PreviewSharing.qml	2016-01-05 09:52:40 +0000
+++ tests/qmltests/Dash/Previews/tst_PreviewSharing.qml	2016-01-05 09:52:40 +0000
@@ -35,11 +35,6 @@
         "content-type": "text"
     }
 
-    property var shareDataNoUri: {
-        "uri": "",
-        "content-type": "text"
-    }
-
     PreviewSharing {
         id: previewSharing
         anchors { left: parent.left; bottom: parent.bottom; }
@@ -54,13 +49,6 @@
 
         function cleanup() {
             peerPicker.visible = false;
-            previewSharing.shareData = root.shareData;
-        }
-
-        function test_visible() {
-            compare(previewSharing.visible, true);
-            previewSharing.shareData = shareDataNoUri;
-            compare(previewSharing.visible, false);
         }
 
         function test_open_picker() {