Code and functional issues with new download in browser feature

Bug #1528651 reported by Olivier Tilloy
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Canonical System Image
Fix Released
High
Bill Filler
webbrowser-app (Ubuntu)
Fix Released
High
Olivier Tilloy

Bug Description

The new "download in browser" feature just landed. This works nicely, but there are a few remaining (mostly minor) issues that need to be addressed now. I’m filing this bug to track them all, we may want to break it up in several bugs if/where relevant.

Code issues:
∘ encapsulation broken in DownloadHandler.qml with references to 'downloadsModel'
∘ encapsulation broken in DownloadDelegate.qml with references to 'downloadManager'
∘ missing autopilot tests for file upload, picking webbrowser-app (including multiple files)
∘ showDownloadsPage() doesn’t make much sense on WebViewImpl, should be an internal function
∘ contentHandlerLoader should have "asynchronous: true"
∘ use default PageHeader from UITK instead of custom BrowserPageHeader?
∘ increase test coverage for the DownloadsModel C++ class
∘ for consistency, downloadsContainer should be made a Loader, as done in https://code.launchpad.net/~osomon/webbrowser-app/keyboard-shortcuts-focus-fixes/+merge/280083
∘ in Downloader.qml and Browser.qml, calls to Component.createObject() should pass the properties as second parameter, instead of setting them afterwards
∘ DownloadsModel should probably be made a singleton type, like HistoryModel and BookmarksModel
∘ includes are not ordered alphabetically in downloads-model.cpp
∘ in DownloadsModel::moveToDownloads(), the [if (QFile::exists(destination))] statement is useless, the do…while loop is enough

Functional issues:
∘ the "No Downloads available" message is confusing in picker mode, it would be clearer if it made it explicit that no download matching the expected file type is available
∘ edit mode in the downloads page should be disabled if the list is empty
∘ while in edit mode in the downloads page, the 'delete' icon should probably be disabled if no download is selected (it currently is enabled and exits select mode)
∘ save link from context menu doesn’t work great with links to other pages: no mime type is displayed (and sometimes even no title), and then the page is saved without an html extension, it can be viewed with the document viewer, but this displays the source of the page, it doesn’t render it (we may need special casing for HTML files in the download folder to open them directly in the browser)
∘ (possibly a bug in the UITK?) upload form that accepts only one file, can end up selecting two: select first, then second, then first again, then second again, and both are selected
∘ when downloading an image to the gallery, I’m seeing this message in the log: Failed moving file from "/home/phablet/.cache/com.ubuntu.gallery/HubIncoming/2/capture2.png" to "/home/phablet/Downloads/capture2.png"
∘ on desktop if I uninstall qtdeclarative5-ubuntu-download-manager0.1, the "Downloads" entry is not visible in the drawer menu, as expected, however the Ctrl+J shortcut still opens the downloads view; the shortcut should be enabled conditionally
∘ click/touch events go through the Downloads page header to items below it, so in narrow mode if one clicks/taps in the right place, they can focus the address bar (thus summoning the OSK) or open the drawer menu

Related branches

Changed in webbrowser-app (Ubuntu):
status: New → Confirmed
Olivier Tilloy (osomon)
description: updated
Olivier Tilloy (osomon)
description: updated
Olivier Tilloy (osomon)
Changed in webbrowser-app (Ubuntu):
assignee: Michael Sheldon (michael-sheldon) → Olivier Tilloy (osomon)
status: Confirmed → In Progress
Olivier Tilloy (osomon)
description: updated
Olivier Tilloy (osomon)
Changed in canonical-devices-system-image:
status: New → In Progress
Changed in canonical-devices-system-image:
status: In Progress → Fix Committed
importance: Undecided → High
assignee: nobody → Bill Filler (bfiller)
milestone: none → ww02-2016
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package webbrowser-app - 0.23+16.04.20160112.1-0ubuntu1

---------------
webbrowser-app (0.23+16.04.20160112.1-0ubuntu1) xenial; urgency=medium

  [ CI Train Bot ]
  * Resync trunk. added: po/et.po

  [ Michael Sheldon ]
  * Dynamically load the thumbnailer image provider if it's available,
    otherwise fallback to mimetype images (LP: #1531179)

  [ Olivier Tilloy ]
  * Address a number of minor functional and code issues in the new
    downloads-in-browser functionality. (LP: #1528651)
  * Restore active focus on the webview after closing the context menu.
    (LP: #1526884)

 -- Olivier Tilloy <email address hidden> Tue, 12 Jan 2016 17:44:22 +0000

Changed in webbrowser-app (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Olivier Tilloy (osomon) wrote :

I filed the following bugs to follow up on the issues that haven’t been addressed:

 bug #1534099
 bug #1534102
 bug #1534108
 bug #1534109
 bug #1534112

Changed in canonical-devices-system-image:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.