webbrowser-app

Merge lp:~michael-sheldon/webbrowser-app/general-download-support into lp:webbrowser-app

general-download-support
Merge into trunk

Proposed by Michael Sheldon on 2014-07-01

Status:

Merged

Approved by:

Olivier Tilloy on 2014-07-09

Approved revision:

593

Merged at revision:

617

Proposed branch:

lp:~michael-sheldon/webbrowser-app/general-download-support

Merge into:

lp:webbrowser-app

Diff against target:

444 lines (+276/-23)

9 files modified

debian/control (+2/-2)
src/app/ContentDownloadDialog.qml (+2/-2)
src/app/ContentPickerDialog.qml (+6/-1)
src/app/ContentShareDialog.qml (+2/-2)
src/app/Downloader.qml (+26/-11)
src/app/FileExtensionMapper.js (+211/-0)
src/app/MimeTypeMapper.js (+3/-1)
src/app/Share.qml (+5/-4)
src/app/WebViewImpl.qml (+19/-0)

To merge this branch:

bzr merge lp:~michael-sheldon/webbrowser-app/general-download-support

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Olivier Tilloy		2014-07-01	Approve on 2014-07-09
PS Jenkins bot	continuous-integration		Approve on 2014-07-09
Review via email: mp+225143@code.launchpad.net

Commit message

Add support for downloads via content-hub on non-desktop devices when it's either not possible to render a file within the browser itself or when a download is explicitly requested via the Content-Disposition header.

Description of the change

Adds support for downloads via content-hub on non-desktop devices when it's either not possible to render a file within the browser itself or when a download is explicitly requested via the Content-Disposition header.

lp:~michael-sheldon/webbrowser-app/general-download-support updated on 2014-07-01

585. By Michael Sheldon on 2014-07-01: Merge from trunk
586. By Michael Sheldon on 2014-07-01: Even out bracket spacing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-01:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

Testing with the latest oxide built from trunk (revision 634) and ubuntu-download-manager built from revision 320 of https://code.launchpad.net/~mandel/ubuntu-download-manager/expose-property/+merge/223874, and downloading image attachments from gmail works. Trying to download an mp4 video attachment results in a message that tells me there is no app installed that can handle this type of content. Is that expected? The same happens for PDF, but I know this is expected.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-01:

315 + headers["Referer"] = request.referrer

It seems "referer" is a common mispelling of "referrer", would that be a bug/typo in UDM?

317 + headers["User-Agent"] = webview.context.userAgent

This won’t get us the overridden UA if any, it’s probably good enough in most cases but I would bet some corner cases will fail because the download is not requested with the same UA as the one that was used to request the parent page.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-01:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Sheldon (michael-sheldon) wrote on 2014-07-01:

It looks like Ken's branch for adding video as one of gallery's accepted types was never merged: https://code.launchpad.net/~ken-vandine/gallery-app/register_for_videos

I'll propose something new for that tomorrow.

The "Referer" spelling is pretty interesting and something I only found out about whilst implementing this; it's actually a misspelling carried forward from the original HTTP spec, so the correct header for HTTP requests is "Referer" instead of "Referrer" (http://en.wikipedia.org/wiki/HTTP_referer#Origin_of_the_term_referer has a bit of detail on that).

Is there a mechanism by which we could be getting the overridden user agent?

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-02:

> The "Referer" spelling is pretty interesting and something I only found out
> about whilst implementing this; it's actually a misspelling carried forward
> from the original HTTP spec, so the correct header for HTTP requests is
> "Referer" instead of "Referrer"
> (http://en.wikipedia.org/wiki/HTTP_referer#Origin_of_the_term_referer has a
> bit of detail on that).

Interesting. Thanks for the pointer, I learnt some history today :)

> Is there a mechanism by which we could be getting the overridden user agent?

Not without modifying the UA override mechanism. I see a few options:

- In UbuntuWebContext, in the message handler for the networkRequestDelegate, we could store the url and corresponding override in a dictionary that would be exposed as a property (or a method) of the context. We would then query this dictionary to get the overridden UA for the URL of the current page, and if not found, use the default.

- Alternatively, we could expose the UA override mechanism as a method of the context, but that’s not necessarily trivial as the mechanism involves user scripts which would need to be refactored to be exposed as generic JS libs.

- Finally, another option would be to add an API to oxide itself to expose the HTTP headers for the current page, that might be useful for other use cases.

What do you think? In case you reckon option #1 is reasonable, feel free to go ahead with it (it shouldn’t be too much work). Other options look more involved, so I think it’s OK to go with the current implementation for now (but in that case please file a bug report and add a comment that points to it).

lp:~michael-sheldon/webbrowser-app/general-download-support updated on 2014-07-02

587. By Michael Sheldon on 2014-07-02: Merge from trunk

Revision history for this message

Michael Sheldon (michael-sheldon) wrote on 2014-07-02:

Are there any related MPs required for this MP to build/function as expected? Please list.

* Requires oxide trunk release
* https://code.launchpad.net/~mandel/ubuntu-download-manager/expose-property/+merge/223874

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

* Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

* Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/webbrowser-app) on device or emulator?

* Yes, except for youtube videos, which I assume is due to my local oxide build being built with default free ffmpeg codecs (I seem to remember this has to be rebuilt in some way to enable non-free codecs?) - Could someone confirm this?

If you changed the UI, was the change specified/approved by design?

* No change

If you changed the packaging (debian), did you add a core-dev as a reviewer to this MP?

* No change

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-02:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-05:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2014-07-09:

In debian/control, can you bump the required version of liboxideqt-qmlplugin to >= 1.1.0 ?

Note that the build-dep probably doesn’t need to be bumped, and neither does the runtime dep for qtdeclarative5-ubuntu-web-plugin, however runtime deps for webbrowser-app and webapp-container do.

lp:~michael-sheldon/webbrowser-app/general-download-support updated on 2014-07-09

588. By Michael Sheldon on 2014-07-09: Set required version of liboxideqt-qmlplugin to >= 1.1.0 for webbrowser-app and webapp-container
589. By Michael Sheldon on 2014-07-09: Add 'm4v' to file extension mapper
590. By Michael Sheldon on 2014-07-09: Merge from trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-09:

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

review: Approve (continuous-integration)

lp:~michael-sheldon/webbrowser-app/general-download-support updated on 2014-07-09

591. By Michael Sheldon on 2014-07-09: Use PopupUtils open and close methods instead of experimental show/hide
592. By Michael Sheldon on 2014-07-09: Pass content picker popup parameters as part of PopupUtils.open call
593. By Michael Sheldon on 2014-07-09: Change liboxideqt-qmlplugin version requirement to >= 1.1 for webbrowser-app and webapp-container

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-09:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-09:

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

review: Approve (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) on 2014-07-09:

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Michael Sheldon

Ubuntu Phablet Team

to status/vote changes:

Rachel Liu

 === modified file 'debian/control'
 --- debian/control	2014-07-04 15:05:13 +0000
 +++ debian/control	2014-07-09 13:31:51 +0000
@@ -33,7 +33,7 @@
  Depends: ${misc:Depends},
           ${shlibs:Depends},
           fonts-liberation,
--         liboxideqt-qmlplugin (>= 1.0.2),
++         liboxideqt-qmlplugin (>= 1.1),
           libqt5sql5-sqlite,
           qtdeclarative5-dialogs-plugin,
           qtdeclarative5-qtquick2-plugin,
@@ -56,7 +56,7 @@
  Depends: ${misc:Depends},
           ${shlibs:Depends},
           fonts-liberation,
--         liboxideqt-qmlplugin (>= 1.0.2),
++         liboxideqt-qmlplugin (>= 1.1),
           libqt5sql5-sqlite,
           libqt5webkit5-qmlwebkitplugin,
           qtdeclarative5-accounts-plugin,
 === modified file 'src/app/ContentDownloadDialog.qml'
 --- src/app/ContentDownloadDialog.qml	2014-05-05 15:31:21 +0000
 +++ src/app/ContentDownloadDialog.qml	2014-07-09 13:31:51 +0000
@@ -39,11 +39,11 @@
                  activeTransfer = peer.request()
                  activeTransfer.downloadId = downloadDialog.downloadId
                  activeTransfer.state = ContentTransfer.Downloading
--                downloadDialog.hide()
++                PopupUtils.close(downloadDialog)
+             }
              onCancelPressed: {
--                downloadDialog.hide()
++                PopupUtils.close(downloadDialog)
+             }
+         }
+     }
 === modified file 'src/app/ContentPickerDialog.qml'
 --- src/app/ContentPickerDialog.qml	2014-05-22 16:11:25 +0000
 +++ src/app/ContentPickerDialog.qml	2014-07-09 13:31:51 +0000
@@ -93,7 +93,12 @@
          Component.onCompleted: {
              if(acceptTypes.length === 1) {
--                peerPicker.contentType = MimeTypeMapper.mimeTypeToContentType(acceptTypes[0])
++                var contentType = MimeTypeMapper.mimeTypeToContentType(acceptTypes[0])
++                if(contentType == ContentType.Unknown) {
++                    // If we don't recognise the type, allow uploads from any app
++                    contentType = ContentType.All
++                }
++                peerPicker.contentType = contentType
              } else {
                  peerPicker.contentType = ContentType.All
+             }
 === modified file 'src/app/ContentShareDialog.qml'
 --- src/app/ContentShareDialog.qml	2014-06-17 14:32:58 +0000
 +++ src/app/ContentShareDialog.qml	2014-07-09 13:31:51 +0000
@@ -39,11 +39,11 @@
                  activeTransfer = peer.request()
                  activeTransfer.items = shareDialog.items
                  activeTransfer.state = ContentTransfer.Charged
--                shareDialog.hide()
++                PopupUtils.close(shareDialog)
+             }
              onCancelPressed: {
--                shareDialog.hide()
++                PopupUtils.close(shareDialog)
+             }
+         }
+     }
 === modified file 'src/app/Downloader.qml'
 --- src/app/Downloader.qml	2014-05-12 10:13:50 +0000
 +++ src/app/Downloader.qml	2014-07-09 13:31:51 +0000
@@ -18,34 +18,49 @@
  import QtQuick 2.0
  import Ubuntu.Components 0.1
++import Ubuntu.Components.Popups 0.1
  import Ubuntu.DownloadManager 0.1
  import Ubuntu.Content 0.1
++import "MimeTypeMapper.js" as MimeTypeMapper
++import "FileExtensionMapper.js" as FileExtensionMapper
  Item {
++    id: downloadItem
--    ContentDownloadDialog {
++    Component {
          id: downloadDialog
++        ContentDownloadDialog { }
+     }
      SingleDownload {
          id: singleDownload
          autoStart: false
++        property var contentType
          onDownloadIdChanged: {
--            downloadDialog.downloadId = singleDownload.downloadId
--            downloadDialog.show()
++            PopupUtils.open(downloadDialog, downloadItem, {"contentType" : singleDownload.contentType, "downloadId" : singleDownload.downloadId})
+         }
+     }
--    function download(url, contentType) {
--        if(contentType) {
--            downloadDialog.contentType = contentType
--        } else {
--            downloaddialog.contentType = ContentType.All
++    function download(url, contentType, headers) {
++        singleDownload.contentType = contentType
++        if(headers) {
++            singleDownload.headers = headers
+         }
          singleDownload.download(url)
+     }
--    function downloadPicture(url) {
--        download(url, ContentType.Pictures)
--    }
++    function downloadPicture(url, headers) {
++        download(url, ContentType.Pictures, headers)
++    }
++
++    function downloadMimeType(url, mimeType, headers, filename) {
++        var contentType = MimeTypeMapper.mimeTypeToContentType(mimeType)
++        if (contentType == ContentType.Unknown && filename) {
++            // If we can't determine the content type from the mime-type
++            // attempt to discover it from the file extension
++            contentType = FileExtensionMapper.filenameToContentType(filename)
++        }
++        download(url, contentType, headers)
++    }
++
+ }
 === added file 'src/app/FileExtensionMapper.js'
 --- src/app/FileExtensionMapper.js	1970-01-01 00:00:00 +0000
 +++ src/app/FileExtensionMapper.js	2014-07-09 13:31:51 +0000
@@ -0,0 +1,211 @@
++/*
++ * Copyright 2014 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/>.
++*/
++
++// Constructed from /etc/mime.types
++function filenameToContentType(filename) {
++    var filenameParts = filename.split(".");
++    if(filenameParts.length === 1 || (filenameParts[0] === "" && filenameParts.length === 2)) {
++        return ContentType.Unknown;
++    }
++    var ext = filenameParts.pop().toLowerCase();
++    switch(ext) {
++        case "art":
++        case "bmp":
++        case "cdr":
++        case "cdt":
++        case "cpt":
++        case "cr2":
++        case "crw":
++        case "djv":
++        case "djvu":
++        case "erf":
++        case "gif":
++        case "ico":
++        case "ief":
++        case "jng":
++        case "jp2":
++        case "jpe":
++        case "jpeg":
++        case "jpf":
++        case "jpg":
++        case "jpg2":
++        case "jpm":
++        case "jpx":
++        case "nef":
++        case "orf":
++        case "pat":
++        case "pbm":
++        case "pcx":
++        case "pgm":
++        case "png":
++        case "pnm":
++        case "ppm":
++        case "psd":
++        case "ras":
++        case "rgb":
++        case "svg":
++        case "svgz":
++        case "tif":
++        case "tiff":
++        case "wbmp":
++        case "xbm":
++        case "xpm":
++        case "xwd":
++            return ContentType.Pictures;
++        case "3gp":
++        case "asf":
++        case "asx":
++        case "avi":
++        case "axv":
++        case "dif":
++        case "dl":
++        case "dv":
++        case "fli":
++        case "flv":
++        case "gl":
++        case "lsf":
++        case "lsx":
++        case "m4v":
++        case "mkv":
++        case "mng":
++        case "mov":
++        case "movie":
++        case "mp4":
++        case "mpe":
++        case "mpeg":
++        case "mpg":
++        case "mpv":
++        case "mxu":
++        case "ogv":
++        case "qt":
++        case "ts":
++        case "webm":
++        case "wm":
++        case "wmv":
++        case "wmx":
++        case "wvx":
++            return ContentType.Videos;
++        case "aif":
++        case "aifc":
++        case "aiff":
++        case "amr":
++        case "au":
++        case "awb":
++        case "axa":
++        case "csd":
++        case "flac":
++        case "gsm":
++        case "kar":
++        case "m3u":
++        case "m4a":
++        case "mid":
++        case "midi":
++        case "mp2":
++        case "mp3":
++        case "mpega":
++        case "mpga":
++        case "oga":
++        case "ogg":
++        case "opus":
++        case "orc":
++        case "pls":
++        case "ra":
++        case "ram":
++        case "rm":
++        case "sco":
++        case "sd2":
++        case "sid":
++        case "snd":
++        case "spx":
++        case "wav":
++        case "wax":
++        case "wma":
++            return ContentType.Music;
++        case "vcard":
++        case "vcf":
++            return ContentType.Contacts;
++        case "323":
++        case "appcache":
++        case "asc":
++        case "bib":
++        case "boo":
++        case "brf":
++        case "c":
++        case "c++":
++        case "cc":
++        case "cls":
++        case "cpp":
++        case "csh":
++        case "css":
++        case "csv":
++        case "cxx":
++        case "d":
++        case "diff":
++        case "etx":
++        case "gcd":
++        case "h":
++        case "hh":
++        case "h++":
++        case "hpp":
++        case "hs":
++        case "htc":
++        case "htm":
++        case "html":
++        case "hxx":
++        case "ics":
++        case "icz":
++        case "jad":
++        case "java":
++        case "lhs":
++        case "ltx":
++        case "ly":
++        case "mml":
++        case "moc":
++        case "p":
++        case "pas":
++        case "patch":
++        case "pl":
++        case "pm":
++        case "pot":
++        case "py":
++        case "rtx":
++        case "scala":
++        case "sct":
++        case "sfv":
++        case "sh":
++        case "shtml":
++        case "srt":
++        case "sty":
++        case "tcl":
++        case "tex":
++        case "text":
++        case "tk":
++        case "tm":
++        case "tsv":
++        case "ttl":
++        case "txt":
++        case "uls":
++        case "vcs":
++        case "wml":
++        case "wmls":
++        case "wsc":
++            return ContentType.Documents;
++        default:
++            return ContentType.Unknown;
++    }
++}
 === modified file 'src/app/MimeTypeMapper.js'
 --- src/app/MimeTypeMapper.js	2014-05-22 20:03:22 +0000
 +++ src/app/MimeTypeMapper.js	2014-07-09 13:31:51 +0000
@@ -22,11 +22,13 @@
          return ContentType.Pictures;
      } else if(mimeType.search("audio/") === 0) {
          return ContentType.Music;
++    } else if(mimeType.search("video/") === 0) {
++        return ContentType.Videos;
      } else if(mimeType.search("text/x-vcard") === 0) {
          return ContentType.Contacts;
      } else if(mimeType.search("text/") === 0) {
          return ContentType.Documents;
      } else {
--        return ContentType.All;
++        return ContentType.Unknown;
+     }
+ }
 === modified file 'src/app/Share.qml'
 --- src/app/Share.qml	2014-06-20 15:52:26 +0000
 +++ src/app/Share.qml	2014-07-09 13:31:51 +0000
@@ -18,14 +18,16 @@
  import QtQuick 2.0
  import Ubuntu.Components 0.1
++import Ubuntu.Components.Popups 0.1
  import Ubuntu.DownloadManager 0.1
  import Ubuntu.Content 0.1
  Item {
      id: shareItem
--    ContentShareDialog {
++    Component {
          id: shareDialog
++        ContentShareDialog { }
+     }
      Component {
@@ -34,9 +36,8 @@
+     }
      function share(url, name, contentType) {
--        shareDialog.contentType = contentType
--        shareDialog.items.push(contentItemComponent.createObject(shareItem, {"url" : url, "name" : name}))
--        shareDialog.show()
++        var sharePopup = PopupUtils.open(shareDialog, shareItem, {"contentType" : contentType})
++        sharePopup.items.push(contentItemComponent.createObject(shareItem, {"url" : url, "name" : name}))
+     }
      function shareLink(url, title) {
 === modified file 'src/app/WebViewImpl.qml'
 --- src/app/WebViewImpl.qml	2014-06-25 17:07:29 +0000
 +++ src/app/WebViewImpl.qml	2014-07-09 13:31:51 +0000
@@ -37,11 +37,30 @@
      beforeUnloadDialog: BeforeUnloadDialog {}
      filePicker: filePickerLoader.item
++    onDownloadRequested: {
++        if (downloadLoader.status == Loader.Ready) {
++            var headers = { }
++            if(request.cookies.length > 0) {
++                headers["Cookie"] = request.cookies.join(";")
++            }
++            if(request.referrer) {
++                headers["Referer"] = request.referrer
++            }
++            headers["User-Agent"] = webview.context.userAgent
++            downloadLoader.item.downloadMimeType(request.url, request.mimeType, headers, request.suggestedFilename)
++        }
++    }
++
      Loader {
          id: filePickerLoader
          source: formFactor == "desktop" ? "FilePickerDialog.qml" : "ContentPickerDialog.qml"
+     }
++    Loader {
++        id: downloadLoader
++        source: formFactor == "desktop" ? "" : "Downloader.qml"
++    }
++
      selectionActions: ActionList {
          Actions.Copy {
              onTriggered: copy()