Merge lp:~michael-sheldon/webbrowser-app/fix-1531179 into lp:webbrowser-app

[Michael Sheldon]

Dynamically load the thumbnailer image provider if it's available, otherwise fallback to mimetype images

Dynamically load the thumbnailer image provider if it's available, otherwise fallback to mimetype images

[Olivier Tilloy (osomon)]

LGTM, thanks for working promptly on this.

One minor suggestion: in Thumbnailer.qml, instead of importing QtQuick and instantiating a visual item, you could import QtQml and instantiate a more lightweight QtObject.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1315
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2552/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5936
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1305
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1305
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1305/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1305
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4584
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5947
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5947/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26429

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2552/rebuild

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

Just tested on a fresh xenial desktop install, and this is not sufficient to make webbrowser-app start. See https://bugs.launchpad.net/ubuntu/+source/webbrowser-app/+bug/1531179/comments/3.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1316
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2553/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5944
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1306
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1306
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1306/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1306
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4592
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5955
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5955/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26442

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2553/rebuild

review: Needs Fixing (continuous-integration)

[lp:~michael-sheldon/webbrowser-app/fix-1531179]

1316. By Michael Sheldon on 2016-01-06

Only display the downloads page if both UDM and content-hub bindings are available

1317. By Michael Sheldon on 2016-01-06

Merge changes

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1317
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2554/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5946
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1307
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1307
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1307/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1307
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4594
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5957
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5957/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26445

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2554/rebuild

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

Looks good to me. A couple of minor suggestions:

  - "id: downloadsLoader" is unnecessary, in onLoaded you can directly refer to item

  - a Loader is a FocusScope, so "downloadsLoader.item.focus = true" should not be needed

In fact for consistency it would be better if the FocusScope with id 'downloadsContainer' was made a Loader (like bookmarksViewLoader, historyViewLoader, and settingsViewLoader).

[lp:~michael-sheldon/webbrowser-app/fix-1531179]

1318. By Michael Sheldon on 2016-01-07

Convert downloadsContainer FocusScope into a Loader for consistency with other views

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1318
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/2557/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5964
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/1310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1310
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/1310/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/1310
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4610
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5975
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5975/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26477

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/2557/rebuild

review: Needs Fixing (continuous-integration)

[Olivier Tilloy (osomon)]

LGTM, thanks!

review: Approve

=== modified file 'src/app/webbrowser/Browser.qml'
--- src/app/webbrowser/Browser.qml	2015-12-22 18:15:49 +0000
+++ src/app/webbrowser/Browser.qml	2016-01-07 13:43:20 +0000
@@ -172,7 +172,7 @@
 
     FocusScope {
         anchors.fill: parent
-        visible: !settingsViewLoader.active && !historyViewLoader.active && !bookmarksViewLoader.active && !downloadsContainer.visible
+        visible: !settingsViewLoader.active && !historyViewLoader.active && !bookmarksViewLoader.active && !downloadsViewLoader.active
 
         FocusScope {
             id: tabContainer
@@ -544,7 +544,7 @@
                 objectName: "downloads"
                 text: i18n.tr("Downloads")
                 iconName: "save"
-                enabled: downloadHandlerLoader.status == Loader.Ready
+                enabled: downloadHandlerLoader.status == Loader.Ready && contentHandlerLoader.status == Loader.Ready
                 onTriggered: {
                     currentWebview.showDownloadsPage()
                 }
@@ -924,26 +924,36 @@
         }
     }
 
-    FocusScope {
-        id: downloadsContainer
+    Loader {
+        id: downloadsViewLoader
 
-        visible: children.length > 0
         anchors.fill: parent
-
-        Component {
-            id: downloadsComponent
-
-            DownloadsPage {
-                anchors.fill: parent
-                focus: true
-                downloadsModel: browser.downloadsModel
-                onDone: destroy()
-                Keys.onEscapePressed: {
-                    destroy()
-                    internal.resetFocus()
-                }
-            }
-        }
+        active: false
+        source: "DownloadsPage.qml"
+
+        onStatusChanged: {
+            if (status == Loader.Ready) {
+                item.downloadsModel = browser.downloadsModel
+                item.forceActiveFocus()
+            } else {
+                internal.resetFocus()
+            }
+        }
+
+        Keys.onEscapePressed: active = false
+
+        onActiveChanged: {
+            if (active) {
+                forceActiveFocus()
+            }
+        }
+
+        Connections {
+            target: downloadsViewLoader.item
+
+            onDone: downloadsViewLoader.active = false
+        }
+
     }
 
     TabsModel {
@@ -1307,8 +1317,8 @@
                 }
 
                 function showDownloadsPage() {
-                    downloadsContainer.focus = true
-                    return downloadsComponent.createObject(downloadsContainer)
+                    downloadsViewLoader.active = true
+                    return downloadsViewLoader.item
                 }
 
                 function startDownload(downloadId, download, mimeType) {
@@ -1989,7 +1999,7 @@
         KeyboardShortcut {
             modifiers: Qt.ControlModifier
             key: Qt.Key_J
-            enabled: chrome.visible && !downloadsContainer.visible
+            enabled: chrome.visible && !downloadsViewLoader.active
             onTriggered: currentWebview.showDownloadsPage()
         }
 
@@ -2019,12 +2029,11 @@
         target: contentHandlerLoader.item
         onExportFromDownloads: {
             if (downloadHandlerLoader.status == Loader.Ready) {
-                downloadsContainer.focus = true
-                var downloadPage = downloadsComponent.createObject(downloadsContainer)
-                downloadPage.mimetypeFilter = mimetypeFilter
-                downloadPage.activeTransfer = transfer
-                downloadPage.multiSelect = multiSelect
-                downloadPage.pickingMode = true
+                downloadsViewLoader.active = true
+                downloadsViewLoader.item.mimetypeFilter = mimetypeFilter
+                downloadsViewLoader.item.activeTransfer = transfer
+                downloadsViewLoader.item.multiSelect = multiSelect
+                downloadsViewLoader.item.pickingMode = true
             }
         }
     }

=== modified file 'src/app/webbrowser/DownloadsPage.qml'
--- src/app/webbrowser/DownloadsPage.qml	2015-12-16 10:11:46 +0000
+++ src/app/webbrowser/DownloadsPage.qml	2016-01-07 13:43:20 +0000
@@ -20,7 +20,6 @@
 import Qt.labs.settings 1.0
 import Ubuntu.Components 1.3
 import Ubuntu.Components.Popups 1.0
-import Ubuntu.Thumbnailer 0.1
 import Ubuntu.Content 1.3
 import Ubuntu.Web 0.2
 import webbrowserapp.private 0.1
@@ -46,6 +45,11 @@
 
     signal done()
 
+    Loader {
+        id: thumbnailLoader
+        source: "Thumbnailer.qml"
+    }
+
     Rectangle {
         anchors.fill: parent
         color: "#fbfbfb"
@@ -166,7 +170,10 @@
             downloadId: model.downloadId
             title: model.filename ? model.filename : model.url.toString().split('/').pop().split('?').shift()
             url: model.url
-            image: model.complete && (model.mimetype.indexOf("image") === 0 || model.mimetype.indexOf("video") === 0) ? "image://thumbnailer/file://" + model.path : ""
+            image: model.complete && thumbnailLoader.status == Loader.Ready 
+                                  && (model.mimetype.indexOf("image") === 0 
+                                      || model.mimetype.indexOf("video") === 0)
+                                  ? "image://thumbnailer/file://" + model.path : ""
             icon: MimeDatabase.iconForMimetype(model.mimetype)
             incomplete: !model.complete
             selectMode: downloadsItem.selectMode || downloadsItem.pickingMode

=== added file 'src/app/webbrowser/Thumbnailer.qml'
--- src/app/webbrowser/Thumbnailer.qml	1970-01-01 00:00:00 +0000
+++ src/app/webbrowser/Thumbnailer.qml	2016-01-07 13:43:20 +0000
@@ -0,0 +1,26 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This file is part of webbrowser-app.
+ *
+ * webbrowser-app 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.
+ *
+ * webbrowser-app 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 QtQml 2.0
+import Ubuntu.Thumbnailer 0.1
+
+/* Because Thumbnailer isn't in the main repository the webbrowser can't
+ depend on it. We use this file to dynamically load the thumbnailer image
+ provider if it's installed. */
+
+QtObject { }