← Back to team overview

touch-packages team mailing list archive

[Bug 1528651] Re: Code and functional issues with new download in browser feature

 

** Description changed:

  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"

-- 
You received this bug notification because you are a member of Ubuntu
Touch seeded packages, which is subscribed to webbrowser-app in Ubuntu.
https://bugs.launchpad.net/bugs/1528651

Title:
  Code and functional issues with new download in browser feature

Status in webbrowser-app package in Ubuntu:
  Confirmed

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"

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/webbrowser-app/+bug/1528651/+subscriptions


References