use download prompt for mhtml downloads

Fixes #2204

We didn't previously use PromptMode.download for MHTML download prompts
to avoid dealing with thinks like "Open download", but the new download
prompt is just way better than the old, which justifies the extra work.

This means that MHTML downloads can now also be opened directly.

qutebrowser/browser/commands.py

@@ -1302,26 +1302,21 @@ class CommandDispatcher:
         # FIXME:qtwebengine do this with the QtWebEngine download manager?
         download_manager = objreg.get('qtnetwork-download-manager',
                                       scope='window', window=self._win_id)
+        target = None
+        if dest is not None:
+            target = downloads.FileDownloadTarget(dest)
+
         if url:
             if mhtml_:
                 raise cmdexc.CommandError("Can only download the current page"
                                           " as mhtml.")
             url = urlutils.qurl_from_user_input(url)
             urlutils.raise_cmdexc_if_invalid(url)
-            if dest is None:
-                target = None
-            else:
-                target = downloads.FileDownloadTarget(dest)
             download_manager.get(url, target=target)
         elif mhtml_:
             self._download_mhtml(dest)
         else:
             qnam = self._current_widget().networkaccessmanager()
-
-            if dest is None:
-                target = None
-            else:
-                target = downloads.FileDownloadTarget(dest)
             download_manager.get(self._current_url(), qnam=qnam, target=target)
 
     def _download_mhtml(self, dest=None):
@@ -1334,22 +1329,23 @@ class CommandDispatcher:
         if tab.backend == usertypes.Backend.QtWebEngine:
             raise cmdexc.CommandError("Download --mhtml is not implemented "
                                       "with QtWebEngine yet")
-
+        if dest is not None:
-        if dest is None:
-            suggested_fn = self._current_title() + ".mht"
-            suggested_fn = utils.sanitize_filename(suggested_fn)
-
-            filename = downloads.immediate_download_path()
-            if filename is not None:
-                mhtml.start_download_checked(filename, tab=tab)
-            else:
-                question = downloads.get_filename_question(
-                    suggested_filename=suggested_fn, url=tab.url(), parent=tab)
-                question.answered.connect(functools.partial(
-                    mhtml.start_download_checked, tab=tab))
-                message.global_bridge.ask(question, blocking=False)
-        else:
             mhtml.start_download_checked(dest, tab=tab)
+            return
+
+        suggested_fn = self._current_title() + ".mht"
+        suggested_fn = utils.sanitize_filename(suggested_fn)
+
+        filename = downloads.immediate_download_path()
+        if filename is not None:
+            target = downloads.FileDownloadTarget(filename)
+            mhtml.start_download_checked(target, tab=tab)
+        else:
+            question = downloads.get_filename_question(
+                suggested_filename=suggested_fn, url=tab.url(), parent=tab)
+            question.answered.connect(functools.partial(
+                mhtml.start_download_checked, tab=tab))
+            message.global_bridge.ask(question, blocking=False)
 
     @cmdutils.register(instance='command-dispatcher', scope='window')
     def view_source(self):

qutebrowser/browser/downloads.py

@@ -158,7 +158,7 @@ def get_filename_question(*, suggested_filename, url, parent=None):
     q.title = "Save file to:"
     q.text = "Please enter a location for <b>{}</b>".format(
         html.escape(url.toDisplayString()))
-    q.mode = usertypes.PromptMode.text
+    q.mode = usertypes.PromptMode.download
     q.completed.connect(q.deleteLater)
     q.default = _path_suggestion(suggested_filename)
     return q
@@ -197,6 +197,9 @@ class FileDownloadTarget(_DownloadTarget):
     def suggested_filename(self):
         return os.path.basename(self.filename)
 
+    def __str__(self):
+        return self.filename
+
 
 class FileObjDownloadTarget(_DownloadTarget):
 
@@ -216,6 +219,12 @@ class FileObjDownloadTarget(_DownloadTarget):
         except AttributeError:
             raise NoFilenameError
 
+    def __str__(self):
+        try:
+            return 'file object at {}'.format(self.fileobj.name)
+        except AttributeError:
+            return 'anonymous file object'
+
 
 class OpenFileDownloadTarget(_DownloadTarget):
 
@@ -234,6 +243,9 @@ class OpenFileDownloadTarget(_DownloadTarget):
     def suggested_filename(self):
         raise NoFilenameError
 
+    def __str__(self):
+        return 'temporary file'
+
 
 class DownloadItemStats(QObject):
 
@@ -780,7 +792,6 @@ class AbstractDownloadManager(QObject):
 
     def _init_filename_question(self, question, download):
         """Set up an existing filename question with a download."""
-        question.mode = usertypes.PromptMode.download
         question.answered.connect(download.set_target)
         question.cancelled.connect(download.cancel)
         download.cancelled.connect(question.abort)

qutebrowser/browser/webkit/mhtml.py

@@ -35,10 +35,12 @@ import email.message
 import quopri
 
 from PyQt5.QtCore import QUrl
+from PyQt5.QtGui import QDesktopServices
 
 from qutebrowser.browser import downloads
 from qutebrowser.browser.webkit import webkitelem
 from qutebrowser.utils import log, objreg, message, usertypes, utils, urlutils
+from qutebrowser.config import config
 
 _File = collections.namedtuple('_File',
                                ['content', 'content_type', 'content_location',
@@ -242,7 +244,7 @@ class _Downloader:
 
     Attributes:
         tab: The AbstractTab which contains the website that will be saved.
-        dest: Destination filename.
+        target: DownloadTarget where the file should be downloaded to.
         writer: The MHTMLWriter object which is used to save the page.
         loaded_urls: A set of QUrls of finished asset downloads.
         pending_downloads: A set of unfinished (url, DownloadItem) tuples.
@@ -252,9 +254,9 @@ class _Downloader:
         _win_id: The window this downloader belongs to.
     """
 
-    def __init__(self, tab, dest):
+    def __init__(self, tab, target):
         self.tab = tab
-        self.dest = dest
+        self.target = target
         self.writer = None
         self.loaded_urls = {tab.url()}
         self.pending_downloads = set()
@@ -462,14 +464,57 @@ class _Downloader:
             return
         self._finished_file = True
         log.downloads.debug("All assets downloaded, ready to finish off!")
+
+        if isinstance(self.target, downloads.FileDownloadTarget):
+            fobj = open(self.target.filename, 'wb')
+        elif isinstance(self.target, downloads.FileObjDownloadTarget):
+            fobj = self.target.fileobj
+        elif isinstance(self.target, downloads.OpenFileDownloadTarget):
+            try:
+                fobj = downloads.temp_download_manager.get_tmpfile(
+                    self.tab.title() + '.mht')
+            except OSError as exc:
+                msg = "Download error: {}".format(exc)
+                message.error(msg)
+                return
+        else:
+            raise ValueError("Invalid DownloadTarget given")
+
         try:
-            with open(self.dest, 'wb') as file_output:
+            with fobj:
-                self.writer.write_to(file_output)
+                self.writer.write_to(fobj)
         except OSError as error:
             message.error("Could not save file: {}".format(error))
             return
         log.downloads.debug("File successfully written.")
-        message.info("Page saved as {}".format(self.dest))
+        message.info("Page saved as {}".format(self.target))
+
+        if isinstance(self.target, downloads.OpenFileDownloadTarget):
+            filename = fobj.name
+            # the default program to open downloads with - will be empty string
+            # if we want to use the default
+            override = config.get('general', 'default-open-dispatcher')
+            cmdline = self.target.cmdline
+
+            # precedence order: cmdline > default-open-dispatcher > openUrl
+            if cmdline is None and not override:
+                log.downloads.debug("Opening {} with the system application"
+                                    .format(filename))
+                url = QUrl.fromLocalFile(filename)
+                QDesktopServices.openUrl(url)
+                return
+
+            if cmdline is None and override:
+                cmdline = override
+
+            cmd, *args = shlex.split(cmdline)
+            args = [arg.replace('{}', filename) for arg in args]
+            if '{}' not in cmdline:
+                args.append(filename)
+            log.downloads.debug("Opening {} with {}"
+                                .format(filename, [cmd] + args))
+            proc = guiprocess.GUIProcess(what='download')
+            proc.start_detached(cmd, args)
 
     def _collect_zombies(self):
         """Collect done downloads and add their data to the MHTML file.
@@ -501,26 +546,29 @@ class _NoCloseBytesIO(io.BytesIO):
         super().close()
 
 
-def _start_download(dest, tab):
+def _start_download(target, tab):
     """Start downloading the current page and all assets to an MHTML file.
 
     This will overwrite dest if it already exists.
 
     Args:
-        dest: The filename where the resulting file should be saved.
+        target: The DownloadTarget where the resulting file should be saved.
         tab: Specify the tab whose page should be loaded.
     """
-    loader = _Downloader(tab, dest)
+    loader = _Downloader(tab, target)
     loader.run()
 
 
-def start_download_checked(dest, tab):
+def start_download_checked(target, tab):
     """First check if dest is already a file, then start the download.
 
     Args:
-        dest: The filename where the resulting file should be saved.
+        target: The DownloadTarget where the resulting file should be saved.
         tab: Specify the tab whose page should be loaded.
     """
+    if not isinstance(target, downloads.FileDownloadTarget):
+        _start_download(target, tab)
+        return
     # The default name is 'page title.mht'
     title = tab.title()
     default_name = utils.sanitize_filename(title + '.mht')
@@ -528,7 +576,7 @@ def start_download_checked(dest, tab):
     # Remove characters which cannot be expressed in the file system encoding
     encoding = sys.getfilesystemencoding()
     default_name = utils.force_encoding(default_name, encoding)
-    dest = utils.force_encoding(dest, encoding)
+    dest = utils.force_encoding(target.filename, encoding)
 
     dest = os.path.expanduser(dest)
 
@@ -549,8 +597,9 @@ def start_download_checked(dest, tab):
         message.error("Directory {} does not exist.".format(folder))
         return
 
+    target = downloads.FileDownloadTarget(path)
     if not os.path.isfile(path):
-        _start_download(path, tab=tab)
+        _start_download(target, tab=tab)
         return
 
     q = usertypes.Question()
@@ -560,5 +609,5 @@ def start_download_checked(dest, tab):
         html.escape(path))
     q.completed.connect(q.deleteLater)
     q.answered_yes.connect(functools.partial(
-        _start_download, path, tab=tab))
+        _start_download, target, tab=tab))
     message.global_bridge.ask(q, blocking=False)