diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4a0812ba1..0362f7691 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -39,6 +39,7 @@ Fixed - `:enter-mode` now refuses to enter modes which can't be entered manually (which caused crashes). - `:record-macro` (`q`) now doesn't try to record macros for special keys without a text. - Fixed PAC (proxy autoconfig) not working with QtWebKit +- `:download --mhtml` now uses the new file dialog v0.9.1 ------ diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 63080af4f..adcd2e9e2 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -1302,54 +1302,50 @@ 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) + self._download_mhtml(target) 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): + def _download_mhtml(self, target=None): """Download the current page as an MHTML file, including all assets. Args: - dest: The file path to write the download to. + target: The download target for the file. """ tab = self._current_widget() if tab.backend == usertypes.Backend.QtWebEngine: raise cmdexc.CommandError("Download --mhtml is not implemented " "with QtWebEngine yet") + if target is not None: + mhtml.start_download_checked(target, tab=tab) + return - if dest is None: - suggested_fn = self._current_title() + ".mht" - suggested_fn = utils.sanitize_filename(suggested_fn) + 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) + filename = downloads.immediate_download_path() + if filename is not None: + target = downloads.FileDownloadTarget(filename) + mhtml.start_download_checked(target, tab=tab) else: - mhtml.start_download_checked(dest, tab=tab) + 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): diff --git a/qutebrowser/browser/downloads.py b/qutebrowser/browser/downloads.py index 548313b93..378fe9a19 100644 --- a/qutebrowser/browser/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -20,7 +20,6 @@ """Shared QtWebKit/QtWebEngine code for downloads.""" import sys -import shlex import html import os.path import collections @@ -28,15 +27,13 @@ import functools import tempfile import sip -from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QObject, QUrl, QModelIndex, +from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QObject, QModelIndex, QTimer, QAbstractListModel) -from PyQt5.QtGui import QDesktopServices from qutebrowser.commands import cmdexc, cmdutils from qutebrowser.config import config from qutebrowser.utils import (usertypes, standarddir, utils, message, log, qtutils) -from qutebrowser.misc import guiprocess ModelRole = usertypes.enum('ModelRole', ['item'], start=Qt.UserRole, @@ -158,7 +155,7 @@ def get_filename_question(*, suggested_filename, url, parent=None): q.title = "Save file to:" q.text = "Please enter a location for {}".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 +194,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 +216,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 +240,9 @@ class OpenFileDownloadTarget(_DownloadTarget): def suggested_filename(self): raise NoFilenameError + def __str__(self): + return 'temporary file' + class DownloadItemStats(QObject): @@ -520,31 +529,7 @@ class AbstractDownloadItem(QObject): if filename is None: # pragma: no cover log.downloads.error("No filename to open the download!") return - - # 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') - - # 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) + utils.open_file(filename, cmdline) def _ensure_can_set_filename(self, filename): """Make sure we can still set a filename.""" @@ -780,7 +765,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) diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index fbfeafef4..97da148a0 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -242,7 +242,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 +252,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 +462,34 @@ 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: {!r}" + .format(self.target)) + try: - with open(self.dest, 'wb') as file_output: - self.writer.write_to(file_output) + with fobj: + 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): + utils.open_file(fobj.name, self.target.cmdline) def _collect_zombies(self): """Collect done downloads and add their data to the MHTML file. @@ -501,26 +521,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 +551,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 +572,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 +584,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) diff --git a/qutebrowser/utils/utils.py b/qutebrowser/utils/utils.py index 2e5142511..fbed4ede3 100644 --- a/qutebrowser/utils/utils.py +++ b/qutebrowser/utils/utils.py @@ -29,9 +29,10 @@ import functools import contextlib import itertools import socket +import shlex -from PyQt5.QtCore import Qt -from PyQt5.QtGui import QKeySequence, QColor, QClipboard +from PyQt5.QtCore import Qt, QUrl +from PyQt5.QtGui import QKeySequence, QColor, QClipboard, QDesktopServices from PyQt5.QtWidgets import QApplication import pkg_resources @@ -825,3 +826,48 @@ def random_port(): port = sock.getsockname()[1] sock.close() return port + + +def open_file(filename, cmdline=None): + """Open the given file. + + If cmdline is not given, general->default-open-dispatcher is used. + If default-open-dispatcher is unset, the system's default application is + used. + + Args: + filename: The filename to open. + cmdline: The command to use as string. A `{}` is expanded to the + filename. None means to use the system's default application + or `default-open-dispatcher` if set. If no `{}` is found, the + filename is appended to the cmdline. + """ + # Import late to avoid circular imports: + # utils -> config -> configdata -> configtypes -> cmdutils -> command -> + # utils + from qutebrowser.misc import guiprocess + from qutebrowser.config import config + # 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') + + # precedence order: cmdline > default-open-dispatcher > openUrl + + if cmdline is None and not override: + log.misc.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.misc.debug("Opening {} with {}" + .format(filename, [cmd] + args)) + proc = guiprocess.GUIProcess(what='open-file') + proc.start_detached(cmd, args) diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 8552ca3e0..5f9aa649a 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -216,17 +216,26 @@ Feature: Downloading things from a website. When I set storage -> prompt-download-directory to true And I open html And I run :download --mhtml - And I wait for "Asking question text='Please enter a location for http://localhost:*/html' title='Save file to:'>, *" in the log + And I wait for "Asking question text='Please enter a location for http://localhost:*/html' title='Save file to:'>, *" in the log And I run :prompt-accept And I wait for "File successfully written." in the log And I run :download --mhtml - And I wait for "Asking question text='Please enter a location for http://localhost:*/html' title='Save file to:'>, *" in the log + And I wait for "Asking question text='Please enter a location for http://localhost:*/html' title='Save file to:'>, *" in the log And I run :prompt-accept And I wait for "Asking question text='* already exists. Overwrite?' title='Overwrite existing file?'>, *" in the log And I run :prompt-accept yes And I wait for "File successfully written." in the log Then no crash should happen + @qtwebengine_todo: :download --mhtml is not implemented yet + Scenario: Opening a mhtml download directly + When I set storage -> prompt-download-directory to true + And I open html + And I run :download --mhtml + And I wait for the download prompt for "*" + And I directly open the download + Then "Opening *.mht* with [*python*]" should be logged + ## :download-cancel Scenario: Cancelling a download diff --git a/tests/end2end/test_invocations.py b/tests/end2end/test_invocations.py index d85e2a467..6d1bd6a27 100644 --- a/tests/end2end/test_invocations.py +++ b/tests/end2end/test_invocations.py @@ -96,7 +96,7 @@ def test_ascii_locale(request, httpbin, tmpdir, quteproc_new): .format(sys.executable)) quteproc_new.wait_for(category='downloads', message='Download รค-issue908.bin finished') - quteproc_new.wait_for(category='downloads', + quteproc_new.wait_for(category='misc', message='Opening * with [*python*]') assert len(tmpdir.listdir()) == 1 diff --git a/tests/unit/utils/test_utils.py b/tests/unit/utils/test_utils.py index 73252ddc4..c790e7a31 100644 --- a/tests/unit/utils/test_utils.py +++ b/tests/unit/utils/test_utils.py @@ -27,8 +27,10 @@ import logging import functools import collections import socket +import re +import shlex -from PyQt5.QtCore import Qt +from PyQt5.QtCore import Qt, QUrl from PyQt5.QtGui import QColor, QClipboard import pytest @@ -984,3 +986,46 @@ def test_random_port(): sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) sock.bind(('localhost', port)) sock.close() + + +class TestOpenFile: + + @pytest.mark.not_frozen + def test_cmdline_without_argument(self, caplog, config_stub): + config_stub.data = {'general': {'default-open-dispatcher': ''}} + executable = shlex.quote(sys.executable) + cmdline = '{} -c pass'.format(executable) + utils.open_file('/foo/bar', cmdline) + result = caplog.records[0].message + assert re.match( + r'Opening /foo/bar with \[.*python.*/foo/bar.*\]', result) + + @pytest.mark.not_frozen + def test_cmdline_with_argument(self, caplog, config_stub): + config_stub.data = {'general': {'default-open-dispatcher': ''}} + executable = shlex.quote(sys.executable) + cmdline = '{} -c pass {{}} raboof'.format(executable) + utils.open_file('/foo/bar', cmdline) + result = caplog.records[0].message + assert re.match( + r"Opening /foo/bar with \[.*python.*/foo/bar.*'raboof'\]", result) + + @pytest.mark.not_frozen + def test_setting_override(self, caplog, config_stub): + executable = shlex.quote(sys.executable) + cmdline = '{} -c pass'.format(executable) + config_stub.data = {'general': {'default-open-dispatcher': cmdline}} + utils.open_file('/foo/bar') + result = caplog.records[0].message + assert re.match( + r"Opening /foo/bar with \[.*python.*/foo/bar.*\]", result) + + def test_system_default_application(self, caplog, config_stub, mocker): + config_stub.data = {'general': {'default-open-dispatcher': ''}} + m = mocker.patch('PyQt5.QtGui.QDesktopServices.openUrl', spec={}, + new_callable=mocker.Mock) + utils.open_file('/foo/bar') + result = caplog.records[0].message + assert re.match( + r"Opening /foo/bar with the system application", result) + m.assert_called_with(QUrl('file:///foo/bar'))