Unity 8

Merge lp:~aacid/unity8/fallbackImage into lp:unity8

  1. fallbackImage
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Andrea Cimitan
Approved revision: 1757
Merged at revision: 1822
Proposed branch: lp:~aacid/unity8/fallbackImage
Merge into: lp:unity8
Diff against target: 360 lines (+151/-8)
9 files modified
plugins/Dash/CardCreator.js (+8/-0)
qml/Components/ZoomableImage.qml (+1/-0)
qml/Dash/Previews/PreviewHeader.qml (+6/-0)
qml/Dash/Previews/PreviewImageGallery.qml (+14/-0)
qml/Dash/Previews/PreviewZoomableImage.qml (+13/-0)
tests/qmltests/Dash/Previews/tst_PreviewHeader.qml (+30/-0)
tests/qmltests/Dash/Previews/tst_PreviewImageGallery.qml (+22/-2)
tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml (+27/-6)
tests/qmltests/Dash/tst_Card.qml (+30/-0)
To merge this branch: bzr merge lp:~aacid/unity8/fallbackImage
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Michi Henning (community) Approve
Review via email: mp+258399@code.launchpad.net

Commit message

Support fallback images for dash card and preview widgets

Description of the change

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

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

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

 * 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

To post a comment you must log in.
Revision history for this message
Michi Henning (michihenning) wrote :

This looks good to me, but someone who actually knows QML should review this too :-)

Once this merges, we'll remove the fallback image support from the thumbnailer.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1757
http://jenkins.qa.ubuntu.com/job/unity8-ci/5679/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2644/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/844
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/844
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/844
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2286/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2642
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2642/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20223

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michi Henning (michihenning) wrote :

Note that providing a fallback image is optional for the scope. So, if the scope hasn't specified a fallback, the shell should probably provide some generic fallback image as a last resort.

Revision history for this message
Albert Astals Cid (aacid) wrote :

> Note that providing a fallback image is optional for the scope. So, if the
> scope hasn't specified a fallback, the shell should probably provide some
> generic fallback image as a last resort.

I didn't see that in the bug, that means involving design to give us a fits-all-cases "broken" image that i can see being hard to design.

Honestly i thought by reading the bug that if no fallback is used we'd just get what we have now, i.e. no image at all.

Revision history for this message
Michi Henning (michihenning) wrote :

Ah, sorry, I thought this might not be clear, which is why I mentioned it :-)

You are right: if the image can't be found and the scope hasn't specified a fallback, you'll just get an empty string. What the shell does with that is really up to the shell. I don't mind really. My only suggestion would be to not display nothing (which is what happens now, I believe). Even showing an empty grey rectangle would probably be better as a fallback than nothing at all?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1757
http://jenkins.qa.ubuntu.com/job/unity8-ci/5687/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2673
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/852/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/852
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/853
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2313
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2671
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2671/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20277

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

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:1757
http://jenkins.qa.ubuntu.com/job/unity8-ci/5690/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2687
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/855
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/855
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/856
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/2324
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2685
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2685/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/20309

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

review: Needs Fixing (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

I realise that this is only likely to happen with an incorrectly configured scope, but what happens if there is an error loading the fallback image? Does it go into an infinite loop, or is the second time setting the "source" property effectively a no-op?

Revision history for this message
Albert Astals Cid (aacid) wrote :

Setting the source to the same source is a no-op

void QQuickImageBase::setSource(const QUrl &url)
{
    Q_D(QQuickImageBase);

    if (url == d->url)
        return;

    ....
}

Revision history for this message
Andrea Cimitan (cimi) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
ap
 * Did you make sure that the branch does not contain spurious tags?
yes

There are few things we need to adjust in order to update to new uitk, but I will take care of them

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/CardCreator.js'
2--- plugins/Dash/CardCreator.js 2015-02-24 10:16:09 +0000
3+++ plugins/Dash/CardCreator.js 2015-05-06 15:06:58 +0000
4@@ -362,6 +362,10 @@
5 }
6
7 code += kArtShapeHolderCode.arg(artAnchors).arg(widthCode).arg(heightCode);
8+ var fallback = components["art"] && components["art"]["fallback"] || "";
9+ if (fallback !== "") {
10+ code += 'Connections { target: artShapeLoader.item ? artShapeLoader.item.image : null; onStatusChanged: if (artShapeLoader.item.image.status === Image.Error) artShapeLoader.item.image.source = "%1"; } \n'.arg(fallback);
11+ }
12 } else {
13 code += 'readonly property size artShapeSize: Qt.size(-1, -1);\n'
14 }
15@@ -441,6 +445,10 @@
16
17 var mascotImageVisible = useMascotShape ? 'false' : 'showHeader';
18 mascotCode = kMascotImageCode.arg(mascotAnchors).arg(mascotImageVisible);
19+ var fallback = components["mascot"] && components["mascot"]["fallback"] || "";
20+ if (fallback !== "") {
21+ code += 'Connections { target: mascotImage.image; onStatusChanged: if (mascotImage.image.status === Image.Error) mascotImage.source = "%1"; } \n'.arg(fallback);
22+ }
23 }
24
25 var summaryColorWithBackground = 'backgroundLoader.active && backgroundLoader.item && root.scopeStyle ? root.scopeStyle.getTextColor(backgroundLoader.item.luminance) : (backgroundLoader.item && backgroundLoader.item.luminance > 0.7 ? Theme.palette.normal.baseText : "white")';
26
27=== modified file 'qml/Components/ZoomableImage.qml'
28--- qml/Components/ZoomableImage.qml 2015-03-18 14:34:18 +0000
29+++ qml/Components/ZoomableImage.qml 2015-05-06 15:06:58 +0000
30@@ -30,6 +30,7 @@
31 property var zoomable: false
32 property alias imageStatus: imageRenderer.status
33 property alias asynchronous: imageRenderer.asynchronous
34+ readonly property alias status: imageRenderer.status
35
36 Flickable {
37 id: flickable
38
39=== modified file 'qml/Dash/Previews/PreviewHeader.qml'
40--- qml/Dash/Previews/PreviewHeader.qml 2014-10-02 14:05:29 +0000
41+++ qml/Dash/Previews/PreviewHeader.qml 2015-05-06 15:06:58 +0000
42@@ -22,6 +22,7 @@
43 /*! This preview widget shows a header
44 * The title comes in widgetData["title"]
45 * The mascot comes in widgetData["mascot"]
46+ * The mascot fall back image comes in widgetData["fallback"]
47 * The subtitle comes in widgetData["subtitle"]
48 * The attributes comes in widgetData["attributes"]
49 */
50@@ -35,11 +36,15 @@
51 id: headerRoot
52 objectName: "innerPreviewHeader"
53 readonly property url mascot: root.widgetData["mascot"] || ""
54+ readonly property url fallback: root.widgetData["fallback"] || ""
55 readonly property string title: root.widgetData["title"] || ""
56 readonly property string subtitle: root.widgetData["subtitle"] || ""
57 readonly property var attributes: root.widgetData["attributes"] || null
58 readonly property color fontColor: root.scopeStyle ? root.scopeStyle.foreground : Theme.palette.normal.baseText
59
60+ // Rewire the source since we may have unwired it on onStatusChanged
61+ onMascotChanged: if (mascotShapeLoader.item) mascotShapeLoader.item.image.source = mascot;
62+
63 implicitHeight: row.height + row.margins * 2
64 width: parent.width
65
66@@ -82,6 +87,7 @@
67 fillMode: Image.PreserveAspectCrop
68 horizontalAlignment: Image.AlignHCenter
69 verticalAlignment: Image.AlignVCenter
70+ onStatusChanged: if (status === Image.Error) source = headerRoot.fallback;
71 }
72 }
73 }
74
75=== modified file 'qml/Dash/Previews/PreviewImageGallery.qml'
76--- qml/Dash/Previews/PreviewImageGallery.qml 2015-03-10 11:52:20 +0000
77+++ qml/Dash/Previews/PreviewImageGallery.qml 2015-05-06 15:06:58 +0000
78@@ -20,6 +20,7 @@
79
80 /*! This preview widget shows a horizontal list of images.
81 * The URIs for the images should be an array in widgetData["sources"].
82+ * Images fall back to widgetData["fallback"] if loading fails
83 */
84
85 PreviewWidget {
86@@ -72,6 +73,14 @@
87 overlay.show();
88 }
89 }
90+
91+ Connections {
92+ target: sourceImage
93+ // If modelData would change after failing to load it would not be
94+ // reloaded since the source binding is destroyed by the source = fallback
95+ // But at the moment the model never changes
96+ onStatusChanged: if (sourceImage.status === Image.Error) sourceImage.source = widgetData["fallback"];
97+ }
98 }
99 }
100
101@@ -122,6 +131,11 @@
102 source: modelData ? modelData : ""
103 fillMode: Image.PreserveAspectFit
104 sourceSize { width: screenshot.width; height: screenshot.height }
105+
106+ // If modelData would change after failing to load it would not be
107+ // reloaded since the source binding is destroyed by the source = fallback
108+ // But at the moment the model never changes
109+ onStatusChanged: if (status === Image.Error) source = widgetData["fallback"];
110 }
111
112 MouseArea {
113
114=== modified file 'qml/Dash/Previews/PreviewZoomableImage.qml'
115--- qml/Dash/Previews/PreviewZoomableImage.qml 2015-03-04 12:32:39 +0000
116+++ qml/Dash/Previews/PreviewZoomableImage.qml 2015-05-06 15:06:58 +0000
117@@ -21,6 +21,7 @@
118 /*! \brief Preview widget for image.
119
120 This widget shows image contained in widgetData["source"],
121+ and falls back to widgetData["fallback"] if loading fails
122 can be zoomable accordingly with widgetData["zoomable"].
123 */
124
125@@ -53,6 +54,14 @@
126 overlay.show();
127 }
128 }
129+
130+ Connections {
131+ target: lazyImage.sourceImage
132+ // If modelData would change after failing to load it would not be
133+ // reloaded since the source binding is destroyed by the source = fallback
134+ // But at the moment the model never changes
135+ onStatusChanged: if (lazyImage.sourceImage.status === Image.Error) lazyImage.sourceImage.source = widgetData["fallback"];
136+ }
137 }
138
139 PreviewOverlay {
140@@ -67,6 +76,10 @@
141 anchors.fill: parent
142 source: widgetData["source"]
143 zoomable: widgetData["zoomable"] ? widgetData["zoomable"] : false
144+ // If modelData would change after failing to load it would not be
145+ // reloaded since the source binding is destroyed by the source = fallback
146+ // But at the moment the model never changes
147+ onStatusChanged: if (status === Image.Error) source = widgetData["fallback"];
148 }
149 }
150 }
151
152=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewHeader.qml'
153--- tests/qmltests/Dash/Previews/tst_PreviewHeader.qml 2014-09-26 13:34:04 +0000
154+++ tests/qmltests/Dash/Previews/tst_PreviewHeader.qml 2015-05-06 15:06:58 +0000
155@@ -38,6 +38,22 @@
156 "attributes": [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"}]
157 }
158
159+ property var brokenheaderjson: {
160+ "title": "THE TITLE",
161+ "subtitle": "Something catchy",
162+ "mascot": "bad_path",
163+ "fallback": "",
164+ "attributes": [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"}]
165+ }
166+
167+ property var fallbackheaderjson: {
168+ "title": "THE TITLE",
169+ "subtitle": "Something catchy",
170+ "mascot": "bad_path2",
171+ "fallback": "../graphics/play_button.png",
172+ "attributes": [{"value":"text1","icon":"image://theme/ok"},{"value":"text2","icon":"image://theme/cancel"}]
173+ }
174+
175 PreviewHeader {
176 id: previewHeader
177 widgetData: headerjson
178@@ -121,5 +137,19 @@
179 compare(innerPreviewHeader.subtitle, "Something catchy");
180 compare(innerPreviewHeader.mascot.toString().slice(-24), "graphics/play_button.png");
181 }
182+
183+ function test_fallback() {
184+ previewHeader.widgetData = brokenheaderjson;
185+ tryCompareFunction(function() { return findChild(previewHeader, "mascotShape") != null }, true);
186+ var mascot = findChild(previewHeader, "mascotShape");
187+ compare(mascot.visible, false);
188+
189+ previewHeader.widgetData = {};
190+ previewHeader.widgetData = fallbackheaderjson;
191+ tryCompareFunction(function() { return findChild(previewHeader, "mascotShape") != null }, true);
192+ var mascot = findChild(previewHeader, "mascotShape");
193+ tryCompare(mascot, "visible", true);
194+ tryCompare(mascot.image, "status", Image.Ready);
195+ }
196 }
197 }
198
199=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewImageGallery.qml'
200--- tests/qmltests/Dash/Previews/tst_PreviewImageGallery.qml 2015-03-05 17:07:07 +0000
201+++ tests/qmltests/Dash/Previews/tst_PreviewImageGallery.qml 2015-05-06 15:06:58 +0000
202@@ -33,8 +33,19 @@
203 "sources": [
204 "../../graphics/phone_background.jpg",
205 "../../graphics/tablet_background.jpg",
206- "../../graphics/clock@18.png"
207- ]
208+ "../../graphics/clock@18.png",
209+ "../../graphics/borked"
210+ ]
211+ }
212+
213+ property var sourcesModel1WithFallback: {
214+ "sources": [
215+ "../../graphics/phone_background.jpg",
216+ "../../graphics/tablet_background.jpg",
217+ "../../graphics/clock@18.png",
218+ "../../graphics/borked"
219+ ]
220+ , "fallback": "../../graphics/clock@18.png"
221 }
222
223 PreviewImageGallery {
224@@ -103,5 +114,14 @@
225 tryCompare(overlayListView, "currentIndex", data.index);
226 verify(image.source === overlayListView.currentItem.source);
227 }
228+
229+ function test_fallback() {
230+ var image3 = findChild(imageGallery, "previewImage3");
231+ tryCompare(image3, "state", "error");
232+ imageGallery.widgetData = sourcesModel0;
233+ imageGallery.widgetData = sourcesModel1WithFallback;
234+ image3 = findChild(imageGallery, "previewImage3");
235+ tryCompare(image3, "state", "ready");
236+ }
237 }
238 }
239
240=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml'
241--- tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml 2015-01-09 09:15:45 +0000
242+++ tests/qmltests/Dash/Previews/tst_PreviewZoomableImage.qml 2015-05-06 15:06:58 +0000
243@@ -26,7 +26,7 @@
244 color: "lightgrey"
245
246 property var widgetData0: {
247- "source": Qt.resolvedUrl("../artwork/checkers.png"),
248+ "source": "../../graphics/phone_background.jpg",
249 "zoomable": false
250 }
251
252@@ -34,21 +34,35 @@
253 "source": ""
254 }
255
256- PreviewZoomableImage {
257- id: zoomableImage
258+ property var widgetData2: {
259+ "source": "fadsasf",
260+ "fallback": "../../graphics/phone_background.jpg"
261+ }
262+
263+ Loader {
264+ id: loader
265 anchors.centerIn: parent
266- widgetData: widgetData0
267+ sourceComponent: PreviewZoomableImage {
268+ widgetData: widgetData0
269+ }
270 }
271
272+ property alias zoomableImage: loader.item
273+
274 UT.UnityTestCase {
275 name: "PreviewZoomableImageTest"
276 when: windowShown
277
278- property Item lazyImage: findChild(zoomableImage, "lazyImage");
279- property Item overlay: findChild(zoomableImage.rootItem, "overlay");
280+ property Item lazyImage
281+ property Item overlay
282
283 function init() {
284+ // Use a loader so we start from scratch each time
285+ loader.active = false;
286+ loader.active = true;
287+ lazyImage = findChild(zoomableImage, "lazyImage");
288 waitForRendering(zoomableImage);
289+ overlay = findChild(zoomableImage.rootItem, "overlay");
290 waitForRendering(overlay);
291 }
292
293@@ -79,5 +93,12 @@
294 mouseClick(overlayCloseButton);
295 tryCompare(overlay, "visible", false);
296 }
297+
298+ function test_fallback() {
299+ zoomableImage.widgetData = widgetData2;
300+ waitForRendering(zoomableImage);
301+ waitForRendering(lazyImage);
302+ tryCompare(lazyImage, "state", "ready");
303+ }
304 }
305 }
306
307=== modified file 'tests/qmltests/Dash/tst_Card.qml'
308--- tests/qmltests/Dash/tst_Card.qml 2015-01-22 14:53:43 +0000
309+++ tests/qmltests/Dash/tst_Card.qml 2015-05-06 15:06:58 +0000
310@@ -224,6 +224,7 @@
311 property Item mascotImage: findChild(card, "mascotImage");
312
313 function init() {
314+ cardTool.components = Qt.binding(function() { return Helpers.update(JSON.parse(Helpers.defaultLayout), Helpers.tryParse(layoutArea.text, layoutError))['components']; });
315 loader.visible = true;
316 }
317
318@@ -424,6 +425,7 @@
319 ];
320 }
321
322+
323 function test_background(data) {
324 selector.selectedIndex = data.index;
325 waitForRendering(selector);
326@@ -454,6 +456,34 @@
327 }
328 }
329
330+ function test_fallback() {
331+ selector.selectedIndex = 0;
332+ waitForRendering(card);
333+
334+ card.cardData["art"] = "somethingbroken";
335+ card.cardDataChanged();
336+ waitForRendering(card);
337+ tryCompare(art, "visible", false);
338+
339+ cardTool.components["art"]["fallback"] = Qt.resolvedUrl("artwork/emblem.png");
340+ cardTool.componentsChanged();
341+ card.cardData["art"] = "somethingbroken";
342+ card.cardDataChanged();
343+ waitForRendering(card);
344+ tryCompare(art, "visible", true);
345+
346+ card.cardData["mascot"] = "somethingbroken2";
347+ card.cardDataChanged();
348+ compare(mascotImage.image.status, Image.Error);
349+
350+ cardTool.components["mascot"] = {"fallback": Qt.resolvedUrl("artwork/emblem.png")};
351+ cardTool.componentsChanged();
352+ card.cardData["mascot"] = "somethingbroken2";
353+ card.cardDataChanged();
354+ waitForRendering(card);
355+ tryCompare(mascotImage.image, "status", Image.Ready);
356+ }
357+
358 function test_font_weights_data() {
359 return [
360 { tag: "Title only", index: 8, weight: Font.Normal },

Subscribers

People subscribed via source and target branches