webbrowser-app

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

  1. fix-1531179
  2. Merge into trunk
Proposed by Michael Sheldon
Status: Merged
Approved by: Olivier Tilloy
Approved revision: 1318
Merged at revision: 1325
Proposed branch: lp:~michael-sheldon/webbrowser-app/fix-1531179
Merge into: lp:webbrowser-app
Diff against target: 181 lines (+73/-31)
3 files modified
src/app/webbrowser/Browser.qml (+38/-29)
src/app/webbrowser/DownloadsPage.qml (+9/-2)
src/app/webbrowser/Thumbnailer.qml (+26/-0)
To merge this branch: bzr merge lp:~michael-sheldon/webbrowser-app/fix-1531179
Reviewer Review Type Date Requested Status
Olivier Tilloy Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+281650@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Olivier Tilloy (osomon) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Olivier Tilloy (osomon) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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 updated
1316. By Michael Sheldon

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

1317. By Michael Sheldon

Merge changes

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

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)
Revision history for this message
Olivier Tilloy (osomon) wrote :

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 updated
1318. By Michael Sheldon

Convert downloadsContainer FocusScope into a Loader for consistency with other views

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

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)
Revision history for this message
Olivier Tilloy (osomon) wrote :

LGTM, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
172172
173 FocusScope {173 FocusScope {
174 anchors.fill: parent174 anchors.fill: parent
175 visible: !settingsViewLoader.active && !historyViewLoader.active && !bookmarksViewLoader.active && !downloadsContainer.visible175 visible: !settingsViewLoader.active && !historyViewLoader.active && !bookmarksViewLoader.active && !downloadsViewLoader.active
176176
177 FocusScope {177 FocusScope {
178 id: tabContainer178 id: tabContainer
@@ -544,7 +544,7 @@
544 objectName: "downloads"544 objectName: "downloads"
545 text: i18n.tr("Downloads")545 text: i18n.tr("Downloads")
546 iconName: "save"546 iconName: "save"
547 enabled: downloadHandlerLoader.status == Loader.Ready547 enabled: downloadHandlerLoader.status == Loader.Ready && contentHandlerLoader.status == Loader.Ready
548 onTriggered: {548 onTriggered: {
549 currentWebview.showDownloadsPage()549 currentWebview.showDownloadsPage()
550 }550 }
@@ -924,26 +924,36 @@
924 }924 }
925 }925 }
926926
927 FocusScope {927 Loader {
928 id: downloadsContainer928 id: downloadsViewLoader
929929
930 visible: children.length > 0
931 anchors.fill: parent930 anchors.fill: parent
932931 active: false
933 Component {932 source: "DownloadsPage.qml"
934 id: downloadsComponent933
935934 onStatusChanged: {
936 DownloadsPage {935 if (status == Loader.Ready) {
937 anchors.fill: parent936 item.downloadsModel = browser.downloadsModel
938 focus: true937 item.forceActiveFocus()
939 downloadsModel: browser.downloadsModel938 } else {
940 onDone: destroy()939 internal.resetFocus()
941 Keys.onEscapePressed: {940 }
942 destroy()941 }
943 internal.resetFocus()942
944 }943 Keys.onEscapePressed: active = false
945 }944
946 }945 onActiveChanged: {
946 if (active) {
947 forceActiveFocus()
948 }
949 }
950
951 Connections {
952 target: downloadsViewLoader.item
953
954 onDone: downloadsViewLoader.active = false
955 }
956
947 }957 }
948958
949 TabsModel {959 TabsModel {
@@ -1307,8 +1317,8 @@
1307 }1317 }
13081318
1309 function showDownloadsPage() {1319 function showDownloadsPage() {
1310 downloadsContainer.focus = true1320 downloadsViewLoader.active = true
1311 return downloadsComponent.createObject(downloadsContainer)1321 return downloadsViewLoader.item
1312 }1322 }
13131323
1314 function startDownload(downloadId, download, mimeType) {1324 function startDownload(downloadId, download, mimeType) {
@@ -1989,7 +1999,7 @@
1989 KeyboardShortcut {1999 KeyboardShortcut {
1990 modifiers: Qt.ControlModifier2000 modifiers: Qt.ControlModifier
1991 key: Qt.Key_J2001 key: Qt.Key_J
1992 enabled: chrome.visible && !downloadsContainer.visible2002 enabled: chrome.visible && !downloadsViewLoader.active
1993 onTriggered: currentWebview.showDownloadsPage()2003 onTriggered: currentWebview.showDownloadsPage()
1994 }2004 }
19952005
@@ -2019,12 +2029,11 @@
2019 target: contentHandlerLoader.item2029 target: contentHandlerLoader.item
2020 onExportFromDownloads: {2030 onExportFromDownloads: {
2021 if (downloadHandlerLoader.status == Loader.Ready) {2031 if (downloadHandlerLoader.status == Loader.Ready) {
2022 downloadsContainer.focus = true2032 downloadsViewLoader.active = true
2023 var downloadPage = downloadsComponent.createObject(downloadsContainer)2033 downloadsViewLoader.item.mimetypeFilter = mimetypeFilter
2024 downloadPage.mimetypeFilter = mimetypeFilter2034 downloadsViewLoader.item.activeTransfer = transfer
2025 downloadPage.activeTransfer = transfer2035 downloadsViewLoader.item.multiSelect = multiSelect
2026 downloadPage.multiSelect = multiSelect2036 downloadsViewLoader.item.pickingMode = true
2027 downloadPage.pickingMode = true
2028 }2037 }
2029 }2038 }
2030 }2039 }
20312040
=== 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 @@
20import Qt.labs.settings 1.020import Qt.labs.settings 1.0
21import Ubuntu.Components 1.321import Ubuntu.Components 1.3
22import Ubuntu.Components.Popups 1.022import Ubuntu.Components.Popups 1.0
23import Ubuntu.Thumbnailer 0.1
24import Ubuntu.Content 1.323import Ubuntu.Content 1.3
25import Ubuntu.Web 0.224import Ubuntu.Web 0.2
26import webbrowserapp.private 0.125import webbrowserapp.private 0.1
@@ -46,6 +45,11 @@
4645
47 signal done()46 signal done()
4847
48 Loader {
49 id: thumbnailLoader
50 source: "Thumbnailer.qml"
51 }
52
49 Rectangle {53 Rectangle {
50 anchors.fill: parent54 anchors.fill: parent
51 color: "#fbfbfb"55 color: "#fbfbfb"
@@ -166,7 +170,10 @@
166 downloadId: model.downloadId170 downloadId: model.downloadId
167 title: model.filename ? model.filename : model.url.toString().split('/').pop().split('?').shift()171 title: model.filename ? model.filename : model.url.toString().split('/').pop().split('?').shift()
168 url: model.url172 url: model.url
169 image: model.complete && (model.mimetype.indexOf("image") === 0 || model.mimetype.indexOf("video") === 0) ? "image://thumbnailer/file://" + model.path : ""173 image: model.complete && thumbnailLoader.status == Loader.Ready
174 && (model.mimetype.indexOf("image") === 0
175 || model.mimetype.indexOf("video") === 0)
176 ? "image://thumbnailer/file://" + model.path : ""
170 icon: MimeDatabase.iconForMimetype(model.mimetype)177 icon: MimeDatabase.iconForMimetype(model.mimetype)
171 incomplete: !model.complete178 incomplete: !model.complete
172 selectMode: downloadsItem.selectMode || downloadsItem.pickingMode179 selectMode: downloadsItem.selectMode || downloadsItem.pickingMode
173180
=== 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 @@
1/*
2 * Copyright 2016 Canonical Ltd.
3 *
4 * This file is part of webbrowser-app.
5 *
6 * webbrowser-app is free software; you can redistribute it and/or modify
7 * it under the terms of the GNU General Public License as published by
8 * the Free Software Foundation; version 3.
9 *
10 * webbrowser-app is distributed in the hope that it will be useful,
11 * but WITHOUT ANY WARRANTY; without even the implied warranty of
12 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13 * GNU General Public License for more details.
14 *
15 * You should have received a copy of the GNU General Public License
16 * along with this program. If not, see <http://www.gnu.org/licenses/>.
17 */
18
19import QtQml 2.0
20import Ubuntu.Thumbnailer 0.1
21
22/* Because Thumbnailer isn't in the main repository the webbrowser can't
23 depend on it. We use this file to dynamically load the thumbnailer image
24 provider if it's installed. */
25
26QtObject { }

Subscribers

People subscribed via source and target branches

to status/vote changes: