webbrowser-app

Merge lp://staging/~michael-sheldon/webbrowser-app/fix-1531179 into lp://staging/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://staging/~michael-sheldon/webbrowser-app/fix-1531179
Merge into: lp://staging/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://staging/~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.staging.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://staging/~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://staging/~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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: