diff --git a/.pylintrc b/.pylintrc index dda9e75ab..3f06facaf 100644 --- a/.pylintrc +++ b/.pylintrc @@ -75,4 +75,5 @@ ignored-modules=pytest # UnsetObject because pylint infers any objreg.get(...) as UnsetObject. ignored-classes=qutebrowser.utils.objreg.UnsetObject, qutebrowser.browser.webkit.webelem.WebElementWrapper, - scripts.dev.check_coverage.MsgType + scripts.dev.check_coverage.MsgType, + qutebrowser.browser.downloads.UnsupportedAttribute diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 8a796a2b2..256edb489 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -140,6 +140,8 @@ Changed - `qute:version` and `qutebrowser --version` now show various important paths - `:spawn`/userscripts now show a nicer error when a script wasn't found - Various functionality now works when javascript is disabled with QtWebKit +- Various commands/settings taking `left`/`right`/`previous` arguments now take + `prev`/`next`/`last-used` to remove ambiguity. Deprecated ~~~~~~~~~~ diff --git a/README.asciidoc b/README.asciidoc index b5ea0fb6d..22b21dd83 100644 --- a/README.asciidoc +++ b/README.asciidoc @@ -154,8 +154,8 @@ Contributors, sorted by the number of commits in descending order: * Bruno Oliveira * Alexander Cogneau * Felix Van der Jeugt -* Martin Tournoij * Daniel Karbach +* Martin Tournoij * Kevin Velghe * Raphael Pierzina * Joel Torstensson diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index 8c5471ab1..5d76b1eca 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -779,13 +779,13 @@ Duplicate the current tab. [[tab-close]] === tab-close -Syntax: +:tab-close [*--left*] [*--right*] [*--opposite*]+ +Syntax: +:tab-close [*--prev*] [*--next*] [*--opposite*]+ Close the current/[count]th tab. ==== optional arguments -* +*-l*+, +*--left*+: Force selecting the tab to the left of the current tab. -* +*-r*+, +*--right*+: Force selecting the tab to the right of the current tab. +* +*-p*+, +*--prev*+: Force selecting the tab before the current tab. +* +*-n*+, +*--next*+: Force selecting the tab after the current tab. * +*-o*+, +*--opposite*+: Force selecting the tab in the opposite direction of what's configured in 'tabs->select-on-remove'. @@ -841,13 +841,13 @@ How many tabs to switch forward. [[tab-only]] === tab-only -Syntax: +:tab-only [*--left*] [*--right*]+ +Syntax: +:tab-only [*--prev*] [*--next*]+ Close all tabs except for the current one. ==== optional arguments -* +*-l*+, +*--left*+: Keep tabs to the left of the current. -* +*-r*+, +*--right*+: Keep tabs to the right of the current. +* +*-p*+, +*--prev*+: Keep tabs before the current. +* +*-n*+, +*--next*+: Keep tabs after the current. [[tab-prev]] === tab-prev diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index f2dc26588..9341676f5 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -1038,11 +1038,11 @@ Which tab to select when the focused tab is removed. Valid values: - * +left+: Select the tab on the left. - * +right+: Select the tab on the right. - * +previous+: Select the previously selected tab. + * +prev+: Select the tab which came before the closed one (left in horizontal, above in vertical). + * +next+: Select the tab which came after the closed one (right in horizontal, below in vertical). + * +last-used+: Select the previously selected tab. -Default: +pass:[right]+ +Default: +pass:[next]+ [[tabs-new-tab-position]] === new-tab-position @@ -1050,12 +1050,12 @@ How new tabs are positioned. Valid values: - * +left+: On the left of the current tab. - * +right+: On the right of the current tab. - * +first+: At the left end. - * +last+: At the right end. + * +prev+: Before the current tab. + * +next+: After the current tab. + * +first+: At the beginning. + * +last+: At the end. -Default: +pass:[right]+ +Default: +pass:[next]+ [[tabs-new-tab-position-explicit]] === new-tab-position-explicit @@ -1063,10 +1063,10 @@ How new tabs opened explicitly are positioned. Valid values: - * +left+: On the left of the current tab. - * +right+: On the right of the current tab. - * +first+: At the left end. - * +last+: At the right end. + * +prev+: Before the current tab. + * +next+: After the current tab. + * +first+: At the beginning. + * +last+: At the end. Default: +pass:[last]+ diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 16f5083a7..04967bd64 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -45,8 +45,9 @@ import qutebrowser.resources from qutebrowser.completion.models import instances as completionmodels from qutebrowser.commands import cmdutils, runners, cmdexc from qutebrowser.config import style, config, websettings, configexc -from qutebrowser.browser import urlmarks, adblock, history, browsertab -from qutebrowser.browser.webkit import cookies, cache, downloads +from qutebrowser.browser import (urlmarks, adblock, history, browsertab, + downloads) +from qutebrowser.browser.webkit import cookies, cache from qutebrowser.browser.webkit.network import networkmanager from qutebrowser.mainwindow import mainwindow, prompt from qutebrowser.misc import (readline, ipc, savemanager, sessions, @@ -371,7 +372,6 @@ def _init_modules(args, crash_handler): args: The argparse namespace. crash_handler: The CrashHandler instance. """ - # pylint: disable=too-many-statements log.init.debug("Initializing prompts...") prompt.init() @@ -435,8 +435,6 @@ def _init_modules(args, crash_handler): os.environ['QT_WAYLAND_DISABLE_WINDOWDECORATION'] = '1' else: os.environ.pop('QT_WAYLAND_DISABLE_WINDOWDECORATION', None) - temp_downloads = downloads.TempDownloadManager(qApp) - objreg.register('temporary-downloads', temp_downloads) # Init backend-specific stuff browsertab.init(args) @@ -705,6 +703,7 @@ class Quitter: atexit.register(shutil.rmtree, self._args.basedir, ignore_errors=True) # Delete temp download dir + downloads.temp_download_manager.cleanup() # If we don't kill our custom handler here we might get segfaults log.destroy.debug("Deactivating message handler...") qInstallMessageHandler(None) diff --git a/qutebrowser/browser/adblock.py b/qutebrowser/browser/adblock.py index 0069fe7e3..84d630ab6 100644 --- a/qutebrowser/browser/adblock.py +++ b/qutebrowser/browser/adblock.py @@ -26,8 +26,9 @@ import posixpath import zipfile import fnmatch +from qutebrowser.browser import downloads from qutebrowser.config import config -from qutebrowser.utils import objreg, standarddir, log, message, usertypes +from qutebrowser.utils import objreg, standarddir, log, message from qutebrowser.commands import cmdutils, cmdexc @@ -190,8 +191,8 @@ class HostBlocker: self._blocked_hosts = set() self._done_count = 0 urls = config.get('content', 'host-block-lists') - download_manager = objreg.get('download-manager', scope='window', - window='last-focused') + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window='last-focused') if urls is None: return for url in urls: @@ -208,7 +209,7 @@ class HostBlocker: else: fobj = io.BytesIO() fobj.name = 'adblock: ' + url.host() - target = usertypes.FileObjDownloadTarget(fobj) + target = downloads.FileObjDownloadTarget(fobj) download = download_manager.get(url, target=target, auto_remove=True) self._in_progress.append(download) diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index 0e43cbf17..caf87a178 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -43,8 +43,7 @@ import pygments.formatters from qutebrowser.commands import userscripts, cmdexc, cmdutils, runners from qutebrowser.config import config, configexc from qutebrowser.browser import (urlmarks, browsertab, inspector, navigate, - webelem) -from qutebrowser.browser.webkit import downloads + webelem, downloads) try: from qutebrowser.browser.webkit import mhtml except ImportError: @@ -179,12 +178,12 @@ class CommandDispatcher: raise cmdexc.CommandError("Last focused tab vanished!") self._set_current_index(idx) - def _get_selection_override(self, left, right, opposite): + def _get_selection_override(self, prev, next_, opposite): """Helper function for tab_close to get the tab to select. Args: - left: Force selecting the tab to the left of the current tab. - right: Force selecting the tab to the right of the current tab. + prev: Force selecting the tab before the current tab. + next_: Force selecting the tab after the current tab. opposite: Force selecting the tab in the opposite direction of what's configured in 'tabs->select-on-remove'. @@ -192,10 +191,10 @@ class CommandDispatcher: QTabBar.SelectLeftTab, QTabBar.SelectRightTab, or None if no change should be made. """ - cmdutils.check_exclusive((left, right, opposite), 'lro') - if left: + cmdutils.check_exclusive((prev, next_, opposite), 'pno') + if prev: return QTabBar.SelectLeftTab - elif right: + elif next_: return QTabBar.SelectRightTab elif opposite: conf_selection = config.get('tabs', 'select-on-remove') @@ -206,25 +205,24 @@ class CommandDispatcher: elif conf_selection == QTabBar.SelectPreviousTab: raise cmdexc.CommandError( "-o is not supported with 'tabs->select-on-remove' set to " - "'previous'!") + "'last-used'!") else: # pragma: no cover raise ValueError("Invalid select-on-remove value " "{!r}!".format(conf_selection)) return None - def _tab_close(self, tab, left=False, right=False, opposite=False): + def _tab_close(self, tab, prev=False, next_=False, opposite=False): """Helper function for tab_close be able to handle message.async. - Args: tab: Tab select to be closed. - left: Force selecting the tab to the left of the current tab. - right: Force selecting the tab to the right of the current tab. + prev: Force selecting the tab before the current tab. + next_: Force selecting the tab after the current tab. opposite: Force selecting the tab in the opposite direction of what's configured in 'tabs->select-on-remove'. count: The tab index to close, or None """ tabbar = self._tabbed_browser.tabBar() - selection_override = self._get_selection_override(left, right, + selection_override = self._get_selection_override(prev, next_, opposite) if tab.data.pinned: @@ -240,12 +238,11 @@ class CommandDispatcher: @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('count', count=True) - def tab_close(self, left=False, right=False, opposite=False, count=None): + def tab_close(self, prev=False, next_=False, opposite=False, count=None): """Close the current/[count]th tab. - Args: - left: Force selecting the tab to the left of the current tab. - right: Force selecting the tab to the right of the current tab. + prev: Force selecting the tab before the current tab. + next_: Force selecting the tab after the current tab. opposite: Force selecting the tab in the opposite direction of what's configured in 'tabs->select-on-remove'. count: The tab index to close, or None @@ -253,9 +250,8 @@ class CommandDispatcher: tab = self._cntwidget(count) if tab is None: return - - close = functools.partial(self._tab_close, tab, left, - right, opposite) + close = functools.partial(self._tab_close, tab, prev, + next_, opposite) if tab.data.pinned: message.confirm_async(title='Pinned Tab', @@ -854,20 +850,20 @@ class CommandDispatcher: message.info("Zoom level: {}%".format(level)) @cmdutils.register(instance='command-dispatcher', scope='window') - def tab_only(self, left=False, right=False): + def tab_only(self, prev=False, next_=False): """Close all tabs except for the current one. Args: - left: Keep tabs to the left of the current. - right: Keep tabs to the right of the current. + prev: Keep tabs before the current. + next_: Keep tabs after the current. """ - cmdutils.check_exclusive((left, right), 'lr') + cmdutils.check_exclusive((prev, next_), 'pn') cur_idx = self._tabbed_browser.currentIndex() assert cur_idx != -1 for i, tab in enumerate(self._tabbed_browser.widgets()): - if (i == cur_idx or (left and i < cur_idx) or - (right and i > cur_idx)): + if (i == cur_idx or (prev and i < cur_idx) or + (next_ and i > cur_idx)): continue else: self._tabbed_browser.close_tab(tab) @@ -1348,8 +1344,7 @@ class CommandDispatcher: except inspector.WebInspectorError as e: raise cmdexc.CommandError(e) - @cmdutils.register(instance='command-dispatcher', scope='window', - backend=usertypes.Backend.QtWebKit) + @cmdutils.register(instance='command-dispatcher', scope='window') @cmdutils.argument('dest_old', hide=True) def download(self, url=None, dest_old=None, *, mhtml_=False, dest=None): """Download a given URL, or current page if no URL given. @@ -1371,8 +1366,9 @@ class CommandDispatcher: " download.") dest = dest_old - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) + # FIXME:qtwebengine do this with the QtWebEngine download manager? + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window=self._win_id) if url: if mhtml_: raise cmdexc.CommandError("Can only download the current page" @@ -1382,21 +1378,16 @@ class CommandDispatcher: if dest is None: target = None else: - target = usertypes.FileDownloadTarget(dest) + target = downloads.FileDownloadTarget(dest) download_manager.get(url, target=target) elif mhtml_: self._download_mhtml(dest) else: - tab = self._current_widget() - # FIXME:qtwebengine have a proper API for this - # pylint: disable=protected-access - qnam = tab._widget.page().networkAccessManager() - # pylint: enable=protected-access if dest is None: target = None else: - target = usertypes.FileDownloadTarget(dest) - download_manager.get(self._current_url(), qnam=qnam, target=target) + target = downloads.FileDownloadTarget(dest) + download_manager.get(self._current_url(), target=target) def _download_mhtml(self, dest=None): """Download the current page as an MHTML file, including all assets. @@ -1405,17 +1396,23 @@ class CommandDispatcher: dest: The file path to write the download to. """ tab = self._current_widget() + if tab.backend == usertypes.Backend.QtWebEngine: + raise cmdexc.CommandError("Download --mhtml is not implemented " + "with QtWebEngine yet") + if dest is None: suggested_fn = self._current_title() + ".mht" suggested_fn = utils.sanitize_filename(suggested_fn) - filename, q = downloads.ask_for_filename(suggested_fn, parent=tab, - url=tab.url()) + + filename = downloads.immediate_download_path() if filename is not None: mhtml.start_download_checked(filename, tab=tab) else: - q.answered.connect(functools.partial( + 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)) - q.ask() + message.global_bridge.ask(question, blocking=False) else: mhtml.start_download_checked(dest, tab=tab) diff --git a/qutebrowser/browser/webkit/downloads.py b/qutebrowser/browser/downloads.py similarity index 55% rename from qutebrowser/browser/webkit/downloads.py rename to qutebrowser/browser/downloads.py index d25c363ea..12e766ecb 100644 --- a/qutebrowser/browser/webkit/downloads.py +++ b/qutebrowser/browser/downloads.py @@ -17,43 +17,32 @@ # You should have received a copy of the GNU General Public License # along with qutebrowser. If not, see . -"""Download manager.""" +"""Shared QtWebKit/QtWebEngine code for downloads.""" -import io -import os import sys import shlex +import html import os.path -import shutil +import collections import functools import tempfile -import collections -import html import sip -from PyQt5.QtCore import (pyqtSlot, pyqtSignal, QObject, QTimer, - Qt, QAbstractListModel, QModelIndex, QUrl) +from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QObject, QUrl, QModelIndex, + QTimer, QAbstractListModel) from PyQt5.QtGui import QDesktopServices -from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply -from qutebrowser.config import config from qutebrowser.commands import cmdexc, cmdutils -from qutebrowser.utils import (message, usertypes, log, utils, urlutils, - objreg, standarddir, qtutils) +from qutebrowser.config import config +from qutebrowser.utils import (usertypes, standarddir, utils, message, log, + qtutils) from qutebrowser.misc import guiprocess -from qutebrowser.browser.webkit import http -from qutebrowser.browser.webkit.network import networkmanager ModelRole = usertypes.enum('ModelRole', ['item'], start=Qt.UserRole, is_int=True) -_RetryInfo = collections.namedtuple('_RetryInfo', ['request', 'manager']) - -_DownloadPath = collections.namedtuple('_DownloadPath', ['filename', - 'question']) - # Remember the last used directory last_used_directory = None @@ -63,6 +52,22 @@ last_used_directory = None _REFRESH_INTERVAL = 500 +class UnsupportedAttribute: + + """Class which is used to create attributes which are not supported. + + This is used for attributes like "fileobj" for downloads which are not + supported with QtWebengine. + """ + + pass + + +class UnsupportedOperationError(Exception): + + """Raised when an operation is not supported with the given backend.""" + + def download_dir(): """Get the download directory to use.""" directory = config.get('storage', 'download-directory') @@ -76,6 +81,24 @@ def download_dir(): return directory +def immediate_download_path(prompt_download_directory=None): + """Try to get an immediate download path without asking the user. + + If that's possible, we return a path immediately. If not, None is returned. + + Args: + prompt_download_directory: If this is something else than None, it + will overwrite the + storage->prompt-download-directory setting. + """ + if prompt_download_directory is None: + prompt_download_directory = config.get('storage', + 'prompt-download-directory') + + if not prompt_download_directory: + return download_dir() + + def _path_suggestion(filename): """Get the suggested file path. @@ -120,33 +143,14 @@ def create_full_filename(basename, filename): return None -def ask_for_filename(suggested_filename, *, url, parent=None, - prompt_download_directory=None): - """Prepare a question for a download-path. - - If a filename can be determined directly, it is returned instead. - - Returns a (filename, question)-namedtuple, in which one component is - None. filename is a string, question is a usertypes.Question. The - question has a special .ask() method that takes no arguments for - convenience, as this function does not yet ask the question, it - only prepares it. +def get_filename_question(*, suggested_filename, url, parent=None): + """Get a Question object for a download-path. Args: suggested_filename: The "default"-name that is pre-entered as path. url: The URL the download originated from. parent: The parent of the question (a QObject). - prompt_download_directory: If this is something else than None, it - will overwrite the - storage->prompt-download-directory setting. """ - if prompt_download_directory is None: - prompt_download_directory = config.get('storage', - 'prompt-download-directory') - - if not prompt_download_directory: - return _DownloadPath(filename=download_dir(), question=None) - encoding = sys.getfilesystemencoding() suggested_filename = utils.force_encoding(suggested_filename, encoding) @@ -157,9 +161,78 @@ def ask_for_filename(suggested_filename, *, url, parent=None, q.mode = usertypes.PromptMode.text q.completed.connect(q.deleteLater) q.default = _path_suggestion(suggested_filename) + return q - q.ask = lambda: message.global_bridge.ask(q, blocking=False) - return _DownloadPath(filename=None, question=q) + +class NoFilenameError(Exception): + + """Raised when we can't find out a filename in DownloadTarget.""" + + +# Where a download should be saved +class _DownloadTarget: + + """Abstract base class for different download targets.""" + + def __init__(self): + raise NotImplementedError + + def suggested_filename(self): + """Get the suggested filename for this download target.""" + raise NotImplementedError + + +class FileDownloadTarget(_DownloadTarget): + + """Save the download to the given file. + + Attributes: + filename: Filename where the download should be saved. + """ + + def __init__(self, filename): + # pylint: disable=super-init-not-called + self.filename = filename + + def suggested_filename(self): + return os.path.basename(self.filename) + + +class FileObjDownloadTarget(_DownloadTarget): + + """Save the download to the given file-like object. + + Attributes: + fileobj: File-like object where the download should be written to. + """ + + def __init__(self, fileobj): + # pylint: disable=super-init-not-called + self.fileobj = fileobj + + def suggested_filename(self): + try: + return self.fileobj.name + except AttributeError: + raise NoFilenameError + + +class OpenFileDownloadTarget(_DownloadTarget): + + """Save the download in a temp dir and directly open it. + + Attributes: + cmdline: The command to use as string. A `{}` is expanded to the + filename. None means to use the system's default application. + If no `{}` is found, the filename is appended to the cmdline. + """ + + def __init__(self, cmdline=None): + # pylint: disable=super-init-not-called + self.cmdline = cmdline + + def suggested_filename(self): + raise NoFilenameError class DownloadItemStats(QObject): @@ -238,31 +311,15 @@ class DownloadItemStats(QObject): bytes_done: How many bytes are downloaded. bytes_total: How many bytes there are to download in total. """ - if bytes_total == -1: + if bytes_total in [0, -1]: # QtWebEngine, QtWebKit bytes_total = None self.done = bytes_done self.total = bytes_total -class DownloadItem(QObject): +class AbstractDownloadItem(QObject): - """A single download currently running. - - There are multiple ways the data can flow from the QNetworkReply to the - disk. - - If the filename/file object is known immediately when starting the - download, QNetworkReply's readyRead writes to the target file directly. - - If not, readyRead is ignored and with self._read_timer we periodically read - into the self._buffer BytesIO slowly, so some broken servers don't close - our connection. - - As soon as we know the file object, we copy self._buffer over and the next - readyRead will write to the real file object. - - Class attributes: - _MAX_REDIRECTS: The maximum redirection count. + """Shared QtNetwork/QtWebEngine part of a download item. Attributes: done: Whether the download is finished. @@ -270,20 +327,10 @@ class DownloadItem(QObject): index: The index of the download in the view. successful: Whether the download has completed successfully. error_msg: The current error message, or None - autoclose: Whether to close the associated file if the download is - done. fileobj: The file object to download the file to. - retry_info: A _RetryInfo instance. raw_headers: The headers sent by the server. _filename: The filename of the download. - _redirects: How many time we were redirected already. - _buffer: A BytesIO object to buffer incoming data until we know the - target file. - _read_timer: A Timer which reads the QNetworkReply into self._buffer - periodically. - _win_id: The window ID the DownloadItem runs in. _dead: Whether the Download has _die()'d. - _reply: The QNetworkReply associated with this download. Signals: data_changed: The downloads metadata changed. @@ -291,50 +338,29 @@ class DownloadItem(QObject): cancelled: The download was cancelled. error: An error with the download occurred. arg: The error message as string. - redirected: Signal emitted when a download was redirected. - arg 0: The new QNetworkRequest. - arg 1: The old QNetworkReply. - do_retry: Emitted when a download is retried. - arg 0: The new DownloadItem remove_requested: Emitted when the removal of this download was requested. """ - _MAX_REDIRECTS = 10 data_changed = pyqtSignal() finished = pyqtSignal() error = pyqtSignal(str) cancelled = pyqtSignal() - redirected = pyqtSignal(QNetworkRequest, QNetworkReply) - do_retry = pyqtSignal(object) # DownloadItem remove_requested = pyqtSignal() - def __init__(self, reply, win_id, parent=None): - """Constructor. - - Args: - reply: The QNetworkReply to download. - """ + def __init__(self, parent=None): super().__init__(parent) - self.retry_info = None self.done = False self.stats = DownloadItemStats(self) self.index = 0 - self.autoclose = True - self._reply = None - self._buffer = io.BytesIO() - self._read_timer = usertypes.Timer(self, name='download-read-timer') - self._read_timer.setInterval(500) - self._read_timer.timeout.connect(self._on_read_timer_timeout) - self._redirects = 0 self.error_msg = None self.basename = '???' self.successful = False - self.fileobj = None + + self.fileobj = UnsupportedAttribute() + self.raw_headers = UnsupportedAttribute() + self._filename = None - self.init_reply(reply) - self._win_id = win_id - self.raw_headers = {} self._dead = False def __repr__(self): @@ -353,10 +379,12 @@ class DownloadItem(QObject): errmsg = "" else: errmsg = " - {}".format(self.error_msg) + if all(e is None for e in [perc, remaining, self.stats.total]): return ('{index}: {name} [{speed:>10}|{down}]{errmsg}'.format( index=self.index, name=self.basename, speed=speed, down=down, errmsg=errmsg)) + perc = round(perc) if remaining is None: remaining = '?' @@ -374,88 +402,39 @@ class DownloadItem(QObject): remaining=remaining, perc=perc, down=down, total=total, errmsg=errmsg)) - def _create_fileobj(self): - """Create a file object using the internal filename.""" - try: - fileobj = open(self._filename, 'wb') - except OSError as e: - self._die(e.strerror) - else: - self.set_fileobj(fileobj) - - def _ask_confirm_question(self, title, msg): - """Create a Question object to be asked.""" - no_action = functools.partial(self.cancel, remove_data=False) - message.confirm_async(title=title, text=msg, - yes_action=self._create_fileobj, - no_action=no_action, cancel_action=no_action, - abort_on=[self.cancelled, self.error]) + def _do_die(self): + """Do cleanup steps after a download has died.""" + raise NotImplementedError def _die(self, msg): """Abort the download and emit an error.""" assert not self.successful - # Prevent actions if calling _die() twice. This might happen if the - # error handler correctly connects, and the error occurs in init_reply - # between reply.error.connect and the reply.error() check. In this - # case, the connected error handlers will be called twice, once via the - # direct error.emit() and once here in _die(). The stacks look like - # this then: + # Prevent actions if calling _die() twice. + # + # For QtWebKit, this might happen if the error handler correctly + # connects, and the error occurs in _init_reply between + # reply.error.connect and the reply.error() check. In this case, the + # connected error handlers will be called twice, once via the direct + # error.emit() and once here in _die(). The stacks look like this then: + # # -> on_reply_error -> _die -> # self.error.emit() + # # and - # [init_reply -> ->] -> + # + # [_init_reply -> ->] -> # self.error.emit() + # # which may lead to duplicate error messages (and failing tests) if self._dead: return self._dead = True - self._read_timer.stop() - self._reply.downloadProgress.disconnect() - self._reply.finished.disconnect() - self._reply.error.disconnect() - self._reply.readyRead.disconnect() + self._do_die() self.error_msg = msg self.stats.finish() self.error.emit(msg) - with log.hide_qt_warning('QNetworkReplyImplPrivate::error: Internal ' - 'problem, this method must only be called ' - 'once.'): - # See https://codereview.qt-project.org/#/c/107863/ - self._reply.abort() - self._reply.deleteLater() - self._reply = None self.done = True self.data_changed.emit() - if self.fileobj is not None: - try: - self.fileobj.close() - except OSError: - log.downloads.exception("Error while closing file object") - - def init_reply(self, reply): - """Set a new reply and connect its signals. - - Args: - reply: The QNetworkReply to handle. - """ - self.done = False - self.successful = False - self._reply = reply - reply.setReadBufferSize(16 * 1024 * 1024) # 16 MB - reply.downloadProgress.connect(self.stats.on_download_progress) - reply.finished.connect(self._on_reply_finished) - reply.error.connect(self._on_reply_error) - reply.readyRead.connect(self._on_ready_read) - reply.metaDataChanged.connect(self._on_meta_data_changed) - self.retry_info = _RetryInfo(request=reply.request(), - manager=reply.manager()) - if not self.fileobj: - self._read_timer.start() - # We could have got signals before we connected slots to them. - # Here no signals are connected to the DownloadItem yet, so we use a - # singleShot QTimer to emit them after they are connected. - if reply.error() != QNetworkReply.NoError: - QTimer.singleShot(0, lambda: self._die(reply.errorString())) def get_status_color(self, position): """Choose an appropriate color for presenting the download's status. @@ -479,6 +458,10 @@ class DownloadItem(QObject): return utils.interpolate_color(start, stop, self.stats.percentage(), system) + def _do_cancel(self): + """Actual cancel implementation.""" + raise NotImplementedError + @pyqtSlot() def cancel(self, *, remove_data=True): """Cancel the download. @@ -486,16 +469,8 @@ class DownloadItem(QObject): Args: remove_data: Whether to remove the downloaded data. """ + self._do_cancel() log.downloads.debug("cancelled") - self._read_timer.stop() - self.cancelled.emit() - if self._reply is not None: - self._reply.finished.disconnect(self._on_reply_finished) - self._reply.abort() - self._reply.deleteLater() - self._reply = None - if self.fileobj is not None: - self.fileobj.close() if remove_data: self.delete() self.done = True @@ -521,15 +496,14 @@ class DownloadItem(QObject): @pyqtSlot() def retry(self): """Retry a failed download.""" - assert self.done - assert not self.successful - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) - new_reply = self.retry_info.manager.get(self.retry_info.request) - new_download = download_manager.fetch( - new_reply, suggested_filename=self.basename) - self.do_retry.emit(new_download) - self.cancel() + raise NotImplementedError + + def _get_open_filename(self): + """Get the filename to open a download. + + Returns None if no suitable filename was found. + """ + raise NotImplementedError @pyqtSlot() def open_file(self, cmdline=None): @@ -542,9 +516,7 @@ class DownloadItem(QObject): to the cmdline. """ assert self.successful - filename = self._filename - if filename is None: - filename = getattr(self.fileobj, 'name', None) + filename = self._get_open_filename() if filename is None: # pragma: no cover log.downloads.error("No filename to open the download!") return @@ -565,19 +537,45 @@ class DownloadItem(QObject): proc = guiprocess.GUIProcess(what='download') proc.start_detached(cmd, args) - def set_filename(self, filename): + def _ensure_can_set_filename(self, filename): + """Make sure we can still set a filename.""" + raise NotImplementedError + + def _after_set_filename(self): + """Finish initialization based on self._filename.""" + raise NotImplementedError + + def _ask_confirm_question(self, title, msg): + """Ask a confirmation question for the download.""" + raise NotImplementedError + + def _set_fileobj(self, fileobj, *, autoclose=True): + """Set a file object to save the download to. + + Not supported by QtWebEngine. + + Args: + fileobj: The file object to download to. + autoclose: Close the file object automatically when it's done. + """ + raise NotImplementedError + + def _set_tempfile(self, fileobj): + """Set a temporary file when opening the download.""" + raise NotImplementedError + + def _set_filename(self, filename, *, force_overwrite=False): """Set the filename to save the download to. Args: filename: The full filename to save the download to. None: special value to stop the download. + force_overwrite: Force overwriting existing files. """ global last_used_directory - if self.fileobj is not None: # pragma: no cover - raise ValueError("fileobj was already set! filename: {}, " - "existing: {}, fileobj {}".format( - filename, self._filename, self.fileobj)) filename = os.path.expanduser(filename) + self._ensure_can_set_filename(filename) + self._filename = create_full_filename(self.basename, filename) if self._filename is None: # We only got a filename (without directory) or a relative path @@ -605,7 +603,9 @@ class DownloadItem(QObject): last_used_directory = os.path.dirname(self._filename) log.downloads.debug("Setting filename to {}".format(filename)) - if os.path.isfile(self._filename): + if force_overwrite: + self._after_set_filename() + elif os.path.isfile(self._filename): # The file already exists, so ask the user if it should be # overwritten. txt = "{} already exists. Overwrite?".format( @@ -618,190 +618,71 @@ class DownloadItem(QObject): "it anyways?".format(html.escape(self._filename))) self._ask_confirm_question("Overwrite special file?", txt) else: - self._create_fileobj() + self._after_set_filename() - def set_fileobj(self, fileobj): - """"Set the file object to write the download to. + def _open_if_successful(self, cmdline): + """Open the downloaded file, but only if it was successful. Args: - fileobj: A file-like object. + cmdline: Passed to DownloadItem.open_file(). """ - if self.fileobj is not None: # pragma: no cover - raise ValueError("fileobj was already set! Old: {}, new: " - "{}".format(self.fileobj, fileobj)) - self.fileobj = fileobj - try: - self._read_timer.stop() - log.downloads.debug("buffer: {} bytes".format(self._buffer.tell())) - self._buffer.seek(0) - shutil.copyfileobj(self._buffer, fileobj) - self._buffer.close() - if self._reply.isFinished(): - # Downloading to the buffer in RAM has already finished so we - # write out the data and clean up now. - self._on_reply_finished() - else: - # Since the buffer already might be full, on_ready_read might - # not be called at all anymore, so we force it here to flush - # the buffer and continue receiving new data. - self._on_ready_read() - except OSError as e: - self._die(e.strerror) + if not self.successful: + log.downloads.debug("{} finished but not successful, not opening!" + .format(self)) + return + self.open_file(cmdline) - def _finish_download(self): - """Write buffered data to disk and finish the QNetworkReply.""" - log.downloads.debug("Finishing download...") - if self._reply.isOpen(): - self.fileobj.write(self._reply.readAll()) - if self.autoclose: - self.fileobj.close() - self.successful = self._reply.error() == QNetworkReply.NoError - self._reply.close() - self._reply.deleteLater() - self._reply = None - self.finished.emit() - self.done = True - log.downloads.debug("Download {} finished".format(self.basename)) - self.data_changed.emit() + def set_target(self, target): + """Set the target for a given download. - @pyqtSlot() - def _on_reply_finished(self): - """Clean up when the download was finished. - - Note when this gets called, only the QNetworkReply has finished. This - doesn't mean the download (i.e. writing data to the disk) is finished - as well. Therefore, we can't close() the QNetworkReply in here yet. + Args: + target: The DownloadTarget for this download. """ - if self._reply is None: - return - self._read_timer.stop() - self.stats.finish() - is_redirected = self._handle_redirect() - if is_redirected: - return - log.downloads.debug("Reply finished, fileobj {}".format(self.fileobj)) - if self.fileobj is not None: - # We can do a "delayed" write immediately to empty the buffer and - # clean up. - self._finish_download() - - @pyqtSlot() - def _on_ready_read(self): - """Read available data and save file when ready to read.""" - if self.fileobj is None or self._reply is None: - # No filename has been set yet (so we don't empty the buffer) or we - # got a readyRead after the reply was finished (which happens on - # qute:log for example). - return - if not self._reply.isOpen(): - raise OSError("Reply is closed!") - try: - self.fileobj.write(self._reply.readAll()) - except OSError as e: - self._die(e.strerror) - - @pyqtSlot('QNetworkReply::NetworkError') - def _on_reply_error(self, code): - """Handle QNetworkReply errors.""" - if code == QNetworkReply.OperationCanceledError: - return - else: - self._die(self._reply.errorString()) - - @pyqtSlot() - def _on_read_timer_timeout(self): - """Read some bytes from the QNetworkReply periodically.""" - if not self._reply.isOpen(): - raise OSError("Reply is closed!") - data = self._reply.read(1024) - if data is not None: - self._buffer.write(data) - - @pyqtSlot() - def _on_meta_data_changed(self): - """Update the download's metadata.""" - if self._reply is None: - return - self.raw_headers = {} - for key, value in self._reply.rawHeaderPairs(): - self.raw_headers[bytes(key)] = bytes(value) - - def _handle_redirect(self): - """Handle an HTTP redirect. - - Return: - True if the download was redirected, False otherwise. - """ - redirect = self._reply.attribute( - QNetworkRequest.RedirectionTargetAttribute) - if redirect is None or redirect.isEmpty(): - return False - new_url = self._reply.url().resolved(redirect) - request = self._reply.request() - if new_url == request.url(): - return False - - if self._redirects > self._MAX_REDIRECTS: - self._die("Maximum redirection count reached!") - self.delete() - return True # so on_reply_finished aborts - - log.downloads.debug("{}: Handling redirect".format(self)) - self._redirects += 1 - request.setUrl(new_url) - reply = self._reply - reply.finished.disconnect(self._on_reply_finished) - self._read_timer.stop() - self._reply = None - if self.fileobj is not None: - self.fileobj.seek(0) - self.redirected.emit(request, reply) # this will change self._reply! - reply.deleteLater() # the old one - return True - - def uses_nam(self, nam): - """Check if this download uses the given QNetworkAccessManager.""" - running_nam = self._reply is not None and self._reply.manager() is nam - # user could request retry after tab is closed. - retry_nam = (self.done and (not self.successful) and - self.retry_info.manager is nam) - return running_nam or retry_nam + if isinstance(target, FileObjDownloadTarget): + self._set_fileobj(target.fileobj, autoclose=False) + elif isinstance(target, FileDownloadTarget): + self._set_filename(target.filename) + elif isinstance(target, OpenFileDownloadTarget): + try: + fobj = temp_download_manager.get_tmpfile(self.basename) + except OSError as exc: + msg = "Download error: {}".format(exc) + message.error(msg) + self.cancel() + return + self.finished.connect( + functools.partial(self._open_if_successful, target.cmdline)) + self._set_tempfile(fobj) + else: # pragma: no cover + raise ValueError("Unsupported download target: {}".format(target)) -class DownloadManager(QObject): +class AbstractDownloadManager(QObject): - """Manager for currently running downloads. + """Backend-independent download manager code. Attributes: downloads: A list of active DownloadItems. - questions: A list of Question objects to not GC them. _networkmanager: A NetworkManager for generic downloads. - _win_id: The window ID the DownloadManager runs in. Signals: - begin_remove_rows: Emitted before downloads are removed. - end_remove_rows: Emitted after downloads are removed. - begin_insert_rows: Emitted before downloads are inserted. - end_insert_rows: Emitted after downloads are inserted. + begin_remove_row: Emitted before downloads are removed. + end_remove_row: Emitted after downloads are removed. + begin_insert_row: Emitted before downloads are inserted. + end_insert_row: Emitted after downloads are inserted. data_changed: Emitted when the data of the model changed. - The arguments are int indices to the downloads. + The argument is the index of the changed download """ - # parent, first, last - begin_remove_rows = pyqtSignal(QModelIndex, int, int) - end_remove_rows = pyqtSignal() - # parent, first, last - begin_insert_rows = pyqtSignal(QModelIndex, int, int) - end_insert_rows = pyqtSignal() - data_changed = pyqtSignal(int, int) # begin, end + begin_remove_row = pyqtSignal(int) + end_remove_row = pyqtSignal() + begin_insert_row = pyqtSignal(int) + end_insert_row = pyqtSignal() + data_changed = pyqtSignal(int) - def __init__(self, win_id, parent=None): + def __init__(self, parent=None): super().__init__(parent) - self._win_id = win_id self.downloads = [] - self.questions = [] - self._networkmanager = networkmanager.NetworkManager( - win_id, None, self) self._update_timer = usertypes.Timer(self, 'download-update') self._update_timer.timeout.connect(self._update_gui) self._update_timer.setInterval(_REFRESH_INTERVAL) @@ -809,123 +690,16 @@ class DownloadManager(QObject): def __repr__(self): return utils.get_repr(self, downloads=len(self.downloads)) - def _postprocess_question(self, q): - """Postprocess a Question object that is asked.""" - q.destroyed.connect(functools.partial(self.questions.remove, q)) - # We set the mode here so that other code that uses ask_for_filename - # doesn't need to handle the special download mode. - q.mode = usertypes.PromptMode.download - self.questions.append(q) - @pyqtSlot() def _update_gui(self): """Periodical GUI update of all items.""" assert self.downloads for dl in self.downloads: dl.stats.update_speed() - self.data_changed.emit(0, -1) + self.data_changed.emit(-1) - @pyqtSlot('QUrl') - def get(self, url, **kwargs): - """Start a download with a link URL. - - Args: - url: The URL to get, as QUrl - **kwargs: passed to get_request(). - - Return: - The created DownloadItem. - """ - if not url.isValid(): - urlutils.invalid_url_error(url, "start download") - return - req = QNetworkRequest(url) - return self.get_request(req, **kwargs) - - def get_request(self, request, *, target=None, **kwargs): - """Start a download with a QNetworkRequest. - - Args: - request: The QNetworkRequest to download. - target: Where to save the download as usertypes.DownloadTarget. - **kwargs: Passed to _fetch_request. - - Return: - The created DownloadItem. - """ - # WORKAROUND for Qt corrupting data loaded from cache: - # https://bugreports.qt.io/browse/QTBUG-42757 - request.setAttribute(QNetworkRequest.CacheLoadControlAttribute, - QNetworkRequest.AlwaysNetwork) - - if request.url().scheme().lower() != 'data': - suggested_fn = urlutils.filename_from_url(request.url()) - else: - # We might be downloading a binary blob embedded on a page or even - # generated dynamically via javascript. We try to figure out a more - # sensible name than the base64 content of the data. - origin = request.originatingObject() - try: - origin_url = origin.url() - except AttributeError: - # Raised either if origin is None or some object that doesn't - # have its own url. We're probably fine with a default fallback - # then. - suggested_fn = 'binary blob' - else: - # Use the originating URL as a base for the filename (works - # e.g. for pdf.js). - suggested_fn = urlutils.filename_from_url(origin_url) - - if suggested_fn is None: - suggested_fn = 'qutebrowser-download' - - return self._fetch_request(request, - target=target, - suggested_filename=suggested_fn, - **kwargs) - - def _fetch_request(self, request, *, qnam=None, **kwargs): - """Download a QNetworkRequest to disk. - - Args: - request: The QNetworkRequest to download. - qnam: The QNetworkAccessManager to use. - **kwargs: passed to fetch(). - - Return: - The created DownloadItem. - """ - if qnam is None: - qnam = self._networkmanager - reply = qnam.get(request) - return self.fetch(reply, **kwargs) - - @pyqtSlot('QNetworkReply') - def fetch(self, reply, *, target=None, auto_remove=False, - suggested_filename=None, prompt_download_directory=None): - """Download a QNetworkReply to disk. - - Args: - reply: The QNetworkReply to download. - target: Where to save the download as usertypes.DownloadTarget. - auto_remove: Whether to remove the download even if - ui -> remove-finished-downloads is set to -1. - - Return: - The created DownloadItem. - """ - if not suggested_filename: - if isinstance(target, usertypes.FileDownloadTarget): - suggested_filename = os.path.basename(target.filename) - elif (isinstance(target, usertypes.FileObjDownloadTarget) and - getattr(target.fileobj, 'name', None)): - suggested_filename = target.fileobj.name - else: - _, suggested_filename = http.parse_content_disposition(reply) - log.downloads.debug("fetch: {} -> {}".format(reply.url(), - suggested_filename)) - download = DownloadItem(reply, self._win_id, self) + def _init_item(self, download, auto_remove, suggested_filename): + """Initialize a newly created DownloadItem.""" download.cancelled.connect(download.remove) download.remove_requested.connect(functools.partial( self._remove_item, download)) @@ -940,104 +714,17 @@ class DownloadManager(QObject): download.data_changed.connect( functools.partial(self._on_data_changed, download)) download.error.connect(self._on_error) - download.redirected.connect( - functools.partial(self._on_redirect, download)) download.basename = suggested_filename idx = len(self.downloads) download.index = idx + 1 # "Human readable" index - self.begin_insert_rows.emit(QModelIndex(), idx, idx) + self.begin_insert_row.emit(idx) self.downloads.append(download) - self.end_insert_rows.emit() + self.end_insert_row.emit() if not self._update_timer.isActive(): self._update_timer.start() - if target is not None: - self._set_download_target(download, suggested_filename, target) - return download - - # Neither filename nor fileobj were given, prepare a question - filename, q = ask_for_filename( - suggested_filename, parent=self, - prompt_download_directory=prompt_download_directory, - url=reply.url()) - - # User doesn't want to be asked, so just use the download_dir - if filename is not None: - target = usertypes.FileDownloadTarget(filename) - self._set_download_target(download, suggested_filename, target) - return download - - # Ask the user for a filename - self._postprocess_question(q) - q.answered.connect( - functools.partial(self._set_download_target, download, - suggested_filename)) - q.cancelled.connect(download.cancel) - download.cancelled.connect(q.abort) - download.error.connect(q.abort) - q.ask() - - return download - - def _set_download_target(self, download, suggested_filename, target): - """Set the target for a given download. - - Args: - download: The download to set the filename for. - suggested_filename: The suggested filename. - target: The usertypes.DownloadTarget for this download. - """ - if isinstance(target, usertypes.FileObjDownloadTarget): - download.set_fileobj(target.fileobj) - download.autoclose = False - elif isinstance(target, usertypes.FileDownloadTarget): - download.set_filename(target.filename) - elif isinstance(target, usertypes.OpenFileDownloadTarget): - tmp_manager = objreg.get('temporary-downloads') - try: - fobj = tmp_manager.get_tmpfile(suggested_filename) - except OSError as exc: - msg = "Download error: {}".format(exc) - message.error(msg) - download.cancel() - return - download.finished.connect( - functools.partial(self._open_download, download, - target.cmdline)) - download.autoclose = True - download.set_fileobj(fobj) - else: # pragma: no cover - raise ValueError("Unknown download target: {}".format(target)) - - def _open_download(self, download, cmdline): - """Open the given download but only if it was successful. - - Args: - download: The DownloadItem to use. - cmdline: Passed to DownloadItem.open_file(). - """ - if not download.successful: - log.downloads.debug("{} finished but not successful, not opening!" - .format(download)) - return - download.open_file(cmdline) - - @pyqtSlot(QNetworkRequest, QNetworkReply) - def _on_redirect(self, download, request, reply): - """Handle an HTTP redirect of a download. - - Args: - download: The old DownloadItem. - request: The new QNetworkRequest. - reply: The old QNetworkReply. - """ - log.downloads.debug("redirected: {} -> {}".format( - reply.url(), request.url())) - new_reply = reply.manager().get(request) - download.init_reply(new_reply) - - @pyqtSlot(DownloadItem) + @pyqtSlot(AbstractDownloadItem) def _on_data_changed(self, download): """Emit data_changed signal when download data changed.""" try: @@ -1045,29 +732,14 @@ class DownloadManager(QObject): except ValueError: # download has been deleted in the meantime return - self.data_changed.emit(idx, idx) + self.data_changed.emit(idx) @pyqtSlot(str) def _on_error(self, msg): """Display error message on download errors.""" message.error("Download error: {}".format(msg)) - def has_downloads_with_nam(self, nam): - """Check if the DownloadManager has any downloads with the given QNAM. - - Args: - nam: The QNetworkAccessManager to check. - - Return: - A boolean. - """ - assert nam.adopted_downloads == 0 - for download in self.downloads: - if download.uses_nam(nam): - nam.adopt_download(download) - return nam.adopted_downloads - - @pyqtSlot(DownloadItem) + @pyqtSlot(AbstractDownloadItem) def _remove_item(self, download): """Remove a given download.""" if sip.isdeleted(self): @@ -1078,9 +750,9 @@ class DownloadManager(QObject): except ValueError: # already removed return - self.begin_remove_rows.emit(QModelIndex(), idx, idx) + self.begin_remove_row.emit(idx) del self.downloads[idx] - self.end_remove_rows.emit() + self.end_remove_row.emit() download.deleteLater() self._update_indexes() if not self.downloads: @@ -1089,32 +761,54 @@ class DownloadManager(QObject): def _update_indexes(self): """Update indexes of all DownloadItems.""" - first_idx = None for i, d in enumerate(self.downloads, 1): - if first_idx is None and d.index != i: - first_idx = i - 1 d.index = i - if first_idx is not None: - self.data_changed.emit(first_idx, -1) + self.data_changed.emit(-1) + + 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) + download.error.connect(question.abort) class DownloadModel(QAbstractListModel): """A list model showing downloads.""" - def __init__(self, downloader, parent=None): + def __init__(self, qtnetwork_manager, webengine_manager=None, parent=None): super().__init__(parent) - self._downloader = downloader - # FIXME we'll need to translate indices here... - downloader.data_changed.connect(self._on_data_changed) - downloader.begin_insert_rows.connect(self.beginInsertRows) - downloader.end_insert_rows.connect(self.endInsertRows) - downloader.begin_remove_rows.connect(self.beginRemoveRows) - downloader.end_remove_rows.connect(self.endRemoveRows) + self._qtnetwork_manager = qtnetwork_manager + self._webengine_manager = webengine_manager + + qtnetwork_manager.data_changed.connect( + functools.partial(self._on_data_changed, webengine=False)) + qtnetwork_manager.begin_insert_row.connect( + functools.partial(self._on_begin_insert_row, webengine=False)) + qtnetwork_manager.begin_remove_row.connect( + functools.partial(self._on_begin_remove_row, webengine=False)) + qtnetwork_manager.end_insert_row.connect(self.endInsertRows) + qtnetwork_manager.end_remove_row.connect(self.endRemoveRows) + + if webengine_manager is not None: + webengine_manager.data_changed.connect( + functools.partial(self._on_data_changed, webengine=True)) + webengine_manager.begin_insert_row.connect( + functools.partial(self._on_begin_insert_row, webengine=True)) + webengine_manager.begin_remove_row.connect( + functools.partial(self._on_begin_remove_row, webengine=True)) + webengine_manager.end_insert_row.connect(self.endInsertRows) + webengine_manager.end_remove_row.connect(self.endRemoveRows) def _all_downloads(self): """Combine downloads from both downloaders.""" - return self._downloader.downloads[:] + if self._webengine_manager is None: + return self._qtnetwork_manager.downloads[:] + else: + return (self._qtnetwork_manager.downloads + + self._webengine_manager.downloads) def __len__(self): return len(self._all_downloads()) @@ -1125,21 +819,48 @@ class DownloadModel(QAbstractListModel): def __getitem__(self, idx): return self._all_downloads()[idx] - @pyqtSlot(int, int) - def _on_data_changed(self, start, end): + def _on_begin_insert_row(self, idx, webengine=False): + log.downloads.debug("_on_begin_insert_row with idx {}, " + "webengine {}".format(idx, webengine)) + if idx == -1: + self.beginInsertRows(QModelIndex(), 0, -1) + return + + assert idx >= 0, idx + if webengine: + idx += len(self._qtnetwork_manager.downloads) + self.beginInsertRows(QModelIndex(), idx, idx) + + def _on_begin_remove_row(self, idx, webengine=False): + log.downloads.debug("_on_begin_remove_row with idx {}, " + "webengine {}".format(idx, webengine)) + if idx == -1: + self.beginRemoveRows(QModelIndex(), 0, -1) + return + + assert idx >= 0, idx + if webengine: + idx += len(self._qtnetwork_manager.downloads) + self.beginRemoveRows(QModelIndex(), idx, idx) + + def _on_data_changed(self, idx, *, webengine): """Called when a downloader's data changed. Args: start: The first changed index as int. end: The last changed index as int, or -1 for all indices. + webengine: If given, the QtNetwork download length is added to the + index. """ - # FIXME we'll need to translate indices here... - start_index = self.index(start, 0) - qtutils.ensure_valid(start_index) - if end == -1: + if idx == -1: + start_index = self.index(0, 0) end_index = self.last_index() else: - end_index = self.index(end, 0) + if webengine: + idx += len(self._qtnetwork_manager.downloads) + start_index = self.index(idx, 0) + end_index = self.index(idx, 0) + qtutils.ensure_valid(start_index) qtutils.ensure_valid(end_index) self.dataChanged.emit(start_index, end_index) @@ -1350,7 +1071,7 @@ class DownloadModel(QAbstractListModel): return len(self) -class TempDownloadManager(QObject): +class TempDownloadManager: """Manager to handle temporary download files. @@ -1362,8 +1083,7 @@ class TempDownloadManager(QObject): files: A list of NamedTemporaryFiles of downloaded items. """ - def __init__(self, parent=None): - super().__init__(parent) + def __init__(self): self.files = [] self._tmpdir = None @@ -1412,3 +1132,6 @@ class TempDownloadManager(QObject): suffix=suggested_name) self.files.append(fobj) return fobj + + +temp_download_manager = TempDownloadManager() diff --git a/qutebrowser/browser/downloadview.py b/qutebrowser/browser/downloadview.py index c54b03e61..783a6a94d 100644 --- a/qutebrowser/browser/downloadview.py +++ b/qutebrowser/browser/downloadview.py @@ -25,7 +25,7 @@ import sip from PyQt5.QtCore import pyqtSlot, QSize, Qt, QTimer from PyQt5.QtWidgets import QListView, QSizePolicy, QMenu -from qutebrowser.browser.webkit import downloads +from qutebrowser.browser import downloads from qutebrowser.config import style from qutebrowser.utils import qtutils, utils, objreg diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 4d9407469..136b769b6 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -283,14 +283,10 @@ class HintActions: else: prompt = None - # FIXME:qtwebengine get a proper API for this - # pylint: disable=protected-access - qnam = elem._elem.webFrame().page().networkAccessManager() - # pylint: enable=protected-access - - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) - download_manager.get(url, qnam=qnam, prompt_download_directory=prompt) + # FIXME:qtwebengine do this with QtWebEngine downloads? + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window=self._win_id) + download_manager.get(url, prompt_download_directory=prompt) def call_userscript(self, elem, context): """Call a userscript from a hint. @@ -662,11 +658,6 @@ class HintManager(QObject): tab = tabbed_browser.currentWidget() if tab is None: raise cmdexc.CommandError("No WebView available yet!") - if (tab.backend == usertypes.Backend.QtWebEngine and - target == Target.download): - message.error("The download target is not available yet with " - "QtWebEngine.") - return mode_manager = objreg.get('mode-manager', scope='window', window=self._win_id) diff --git a/qutebrowser/browser/qtnetworkdownloads.py b/qutebrowser/browser/qtnetworkdownloads.py new file mode 100644 index 000000000..b75fee821 --- /dev/null +++ b/qutebrowser/browser/qtnetworkdownloads.py @@ -0,0 +1,469 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2014-2016 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser 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, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser 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 qutebrowser. If not, see . + +"""Download manager.""" + +import io +import shutil +import functools +import collections + +from PyQt5.QtCore import pyqtSlot, QTimer +from PyQt5.QtNetwork import QNetworkRequest, QNetworkReply + +from qutebrowser.utils import message, usertypes, log, urlutils +from qutebrowser.browser import downloads +from qutebrowser.browser.webkit import http +from qutebrowser.browser.webkit.network import networkmanager + + +_RetryInfo = collections.namedtuple('_RetryInfo', ['request', 'manager']) + + +class DownloadItem(downloads.AbstractDownloadItem): + + """A single download currently running. + + There are multiple ways the data can flow from the QNetworkReply to the + disk. + + If the filename/file object is known immediately when starting the + download, QNetworkReply's readyRead writes to the target file directly. + + If not, readyRead is ignored and with self._read_timer we periodically read + into the self._buffer BytesIO slowly, so some broken servers don't close + our connection. + + As soon as we know the file object, we copy self._buffer over and the next + readyRead will write to the real file object. + + Class attributes: + _MAX_REDIRECTS: The maximum redirection count. + + Attributes: + _retry_info: A _RetryInfo instance. + _redirects: How many time we were redirected already. + _buffer: A BytesIO object to buffer incoming data until we know the + target file. + _read_timer: A Timer which reads the QNetworkReply into self._buffer + periodically. + _manager: The DownloadManager which started this download + _reply: The QNetworkReply associated with this download. + _autoclose: Whether to close the associated file when the download is + done. + """ + + _MAX_REDIRECTS = 10 + + def __init__(self, reply, manager): + """Constructor. + + Args: + reply: The QNetworkReply to download. + """ + super().__init__(parent=manager) + self.fileobj = None + self.raw_headers = {} + + self._autoclose = True + self._manager = manager + self._retry_info = None + self._reply = None + self._buffer = io.BytesIO() + self._read_timer = usertypes.Timer(self, name='download-read-timer') + self._read_timer.setInterval(500) + self._read_timer.timeout.connect(self._on_read_timer_timeout) + self._redirects = 0 + self._init_reply(reply) + + def _create_fileobj(self): + """Create a file object using the internal filename.""" + try: + fileobj = open(self._filename, 'wb') + except OSError as e: + self._die(e.strerror) + else: + self._set_fileobj(fileobj) + + def _do_die(self): + """Abort the download and emit an error.""" + self._read_timer.stop() + self._reply.downloadProgress.disconnect() + self._reply.finished.disconnect() + self._reply.error.disconnect() + self._reply.readyRead.disconnect() + with log.hide_qt_warning('QNetworkReplyImplPrivate::error: Internal ' + 'problem, this method must only be called ' + 'once.'): + # See https://codereview.qt-project.org/#/c/107863/ + self._reply.abort() + self._reply.deleteLater() + self._reply = None + if self.fileobj is not None: + try: + self.fileobj.close() + except OSError: + log.downloads.exception("Error while closing file object") + + def _init_reply(self, reply): + """Set a new reply and connect its signals. + + Args: + reply: The QNetworkReply to handle. + """ + self.done = False + self.successful = False + self._reply = reply + reply.setReadBufferSize(16 * 1024 * 1024) # 16 MB + reply.downloadProgress.connect(self.stats.on_download_progress) + reply.finished.connect(self._on_reply_finished) + reply.error.connect(self._on_reply_error) + reply.readyRead.connect(self._on_ready_read) + reply.metaDataChanged.connect(self._on_meta_data_changed) + self._retry_info = _RetryInfo(request=reply.request(), + manager=reply.manager()) + if not self.fileobj: + self._read_timer.start() + # We could have got signals before we connected slots to them. + # Here no signals are connected to the DownloadItem yet, so we use a + # singleShot QTimer to emit them after they are connected. + if reply.error() != QNetworkReply.NoError: + QTimer.singleShot(0, lambda: self._die(reply.errorString())) + + def _do_cancel(self): + if self._reply is not None: + self._reply.finished.disconnect(self._on_reply_finished) + self._reply.abort() + self._reply.deleteLater() + self._reply = None + if self.fileobj is not None: + self.fileobj.close() + self.cancelled.emit() + + @pyqtSlot() + def retry(self): + """Retry a failed download.""" + assert self.done + assert not self.successful + new_reply = self._retry_info.manager.get(self._retry_info.request) + self._manager.fetch(new_reply, suggested_filename=self.basename) + self.cancel() + + def _get_open_filename(self): + filename = self._filename + if filename is None: + filename = getattr(self.fileobj, 'name', None) + return filename + + def _ensure_can_set_filename(self, filename): + if self.fileobj is not None: # pragma: no cover + raise ValueError("fileobj was already set! filename: {}, " + "existing: {}, fileobj {}".format( + filename, self._filename, self.fileobj)) + + def _after_set_filename(self): + self._create_fileobj() + + def _ask_confirm_question(self, title, msg): + no_action = functools.partial(self.cancel, remove_data=False) + message.confirm_async(title=title, text=msg, + yes_action=self._after_set_filename, + no_action=no_action, cancel_action=no_action, + abort_on=[self.cancelled, self.error]) + + def _set_fileobj(self, fileobj, *, autoclose=True): + """"Set the file object to write the download to. + + Args: + fileobj: A file-like object. + """ + if self.fileobj is not None: # pragma: no cover + raise ValueError("fileobj was already set! Old: {}, new: " + "{}".format(self.fileobj, fileobj)) + self.fileobj = fileobj + self._autoclose = autoclose + try: + self._read_timer.stop() + log.downloads.debug("buffer: {} bytes".format(self._buffer.tell())) + self._buffer.seek(0) + shutil.copyfileobj(self._buffer, fileobj) + self._buffer.close() + if self._reply.isFinished(): + # Downloading to the buffer in RAM has already finished so we + # write out the data and clean up now. + self._on_reply_finished() + else: + # Since the buffer already might be full, on_ready_read might + # not be called at all anymore, so we force it here to flush + # the buffer and continue receiving new data. + self._on_ready_read() + except OSError as e: + self._die(e.strerror) + + def _set_tempfile(self, fileobj): + self._set_fileobj(fileobj) + + def _finish_download(self): + """Write buffered data to disk and finish the QNetworkReply.""" + log.downloads.debug("Finishing download...") + if self._reply.isOpen(): + self.fileobj.write(self._reply.readAll()) + if self._autoclose: + self.fileobj.close() + self.successful = self._reply.error() == QNetworkReply.NoError + self._reply.close() + self._reply.deleteLater() + self._reply = None + self.finished.emit() + self.done = True + log.downloads.debug("Download {} finished".format(self.basename)) + self.data_changed.emit() + + @pyqtSlot() + def _on_reply_finished(self): + """Clean up when the download was finished. + + Note when this gets called, only the QNetworkReply has finished. This + doesn't mean the download (i.e. writing data to the disk) is finished + as well. Therefore, we can't close() the QNetworkReply in here yet. + """ + if self._reply is None: + return + self._read_timer.stop() + self.stats.finish() + is_redirected = self._handle_redirect() + if is_redirected: + return + log.downloads.debug("Reply finished, fileobj {}".format(self.fileobj)) + if self.fileobj is not None: + # We can do a "delayed" write immediately to empty the buffer and + # clean up. + self._finish_download() + + @pyqtSlot() + def _on_ready_read(self): + """Read available data and save file when ready to read.""" + if self.fileobj is None or self._reply is None: + # No filename has been set yet (so we don't empty the buffer) or we + # got a readyRead after the reply was finished (which happens on + # qute:log for example). + return + if not self._reply.isOpen(): + raise OSError("Reply is closed!") + try: + self.fileobj.write(self._reply.readAll()) + except OSError as e: + self._die(e.strerror) + + @pyqtSlot('QNetworkReply::NetworkError') + def _on_reply_error(self, code): + """Handle QNetworkReply errors.""" + if code == QNetworkReply.OperationCanceledError: + return + else: + self._die(self._reply.errorString()) + + @pyqtSlot() + def _on_read_timer_timeout(self): + """Read some bytes from the QNetworkReply periodically.""" + if not self._reply.isOpen(): + raise OSError("Reply is closed!") + data = self._reply.read(1024) + if data is not None: + self._buffer.write(data) + + @pyqtSlot() + def _on_meta_data_changed(self): + """Update the download's metadata.""" + if self._reply is None: + return + self.raw_headers = {} + for key, value in self._reply.rawHeaderPairs(): + self.raw_headers[bytes(key)] = bytes(value) + + def _handle_redirect(self): + """Handle an HTTP redirect. + + Return: + True if the download was redirected, False otherwise. + """ + redirect = self._reply.attribute( + QNetworkRequest.RedirectionTargetAttribute) + if redirect is None or redirect.isEmpty(): + return False + new_url = self._reply.url().resolved(redirect) + new_request = self._reply.request() + if new_url == new_request.url(): + return False + + if self._redirects > self._MAX_REDIRECTS: + self._die("Maximum redirection count reached!") + self.delete() + return True # so on_reply_finished aborts + + log.downloads.debug("{}: Handling redirect".format(self)) + self._redirects += 1 + new_request.setUrl(new_url) + old_reply = self._reply + old_reply.finished.disconnect(self._on_reply_finished) + self._read_timer.stop() + self._reply = None + if self.fileobj is not None: + self.fileobj.seek(0) + + log.downloads.debug("redirected: {} -> {}".format( + old_reply.url(), new_request.url())) + new_reply = old_reply.manager().get(new_request) + self._init_reply(new_reply) + + old_reply.deleteLater() + return True + + +class DownloadManager(downloads.AbstractDownloadManager): + + """Manager for currently running downloads. + + Attributes: + _networkmanager: A NetworkManager for generic downloads. + """ + + def __init__(self, win_id, parent=None): + super().__init__(parent) + self._networkmanager = networkmanager.NetworkManager( + win_id, None, self) + + @pyqtSlot('QUrl') + def get(self, url, **kwargs): + """Start a download with a link URL. + + Args: + url: The URL to get, as QUrl + **kwargs: passed to get_request(). + + Return: + The created DownloadItem. + """ + if not url.isValid(): + urlutils.invalid_url_error(url, "start download") + return + req = QNetworkRequest(url) + return self.get_request(req, **kwargs) + + def get_request(self, request, *, target=None, **kwargs): + """Start a download with a QNetworkRequest. + + Args: + request: The QNetworkRequest to download. + target: Where to save the download as downloads.DownloadTarget. + **kwargs: Passed to _fetch_request. + + Return: + The created DownloadItem. + """ + # WORKAROUND for Qt corrupting data loaded from cache: + # https://bugreports.qt.io/browse/QTBUG-42757 + request.setAttribute(QNetworkRequest.CacheLoadControlAttribute, + QNetworkRequest.AlwaysNetwork) + + if request.url().scheme().lower() != 'data': + suggested_fn = urlutils.filename_from_url(request.url()) + else: + # We might be downloading a binary blob embedded on a page or even + # generated dynamically via javascript. We try to figure out a more + # sensible name than the base64 content of the data. + origin = request.originatingObject() + try: + origin_url = origin.url() + except AttributeError: + # Raised either if origin is None or some object that doesn't + # have its own url. We're probably fine with a default fallback + # then. + suggested_fn = 'binary blob' + else: + # Use the originating URL as a base for the filename (works + # e.g. for pdf.js). + suggested_fn = urlutils.filename_from_url(origin_url) + + if suggested_fn is None: + suggested_fn = 'qutebrowser-download' + + return self._fetch_request(request, + target=target, + suggested_filename=suggested_fn, + **kwargs) + + def _fetch_request(self, request, **kwargs): + """Download a QNetworkRequest to disk. + + Args: + request: The QNetworkRequest to download. + **kwargs: passed to fetch(). + + Return: + The created DownloadItem. + """ + reply = self._networkmanager.get(request) + return self.fetch(reply, **kwargs) + + @pyqtSlot('QNetworkReply') + def fetch(self, reply, *, target=None, auto_remove=False, + suggested_filename=None, prompt_download_directory=None): + """Download a QNetworkReply to disk. + + Args: + reply: The QNetworkReply to download. + target: Where to save the download as downloads.DownloadTarget. + auto_remove: Whether to remove the download even if + ui -> remove-finished-downloads is set to -1. + + Return: + The created DownloadItem. + """ + if not suggested_filename: + try: + suggested_filename = target.suggested_filename() + except downloads.NoFilenameError: + _, suggested_filename = http.parse_content_disposition(reply) + log.downloads.debug("fetch: {} -> {}".format(reply.url(), + suggested_filename)) + download = DownloadItem(reply, manager=self) + self._init_item(download, auto_remove, suggested_filename) + + if target is not None: + download.set_target(target) + return download + + # Neither filename nor fileobj were given + + filename = downloads.immediate_download_path(prompt_download_directory) + if filename is not None: + # User doesn't want to be asked, so just use the download_dir + target = downloads.FileDownloadTarget(filename) + download.set_target(target) + return download + + # Ask the user for a filename + question = downloads.get_filename_question( + suggested_filename=suggested_filename, url=reply.url(), + parent=self) + self._init_filename_question(question, download) + message.global_bridge.ask(question, blocking=False) + + return download diff --git a/qutebrowser/browser/webengine/webenginedownloads.py b/qutebrowser/browser/webengine/webenginedownloads.py new file mode 100644 index 000000000..5286a6298 --- /dev/null +++ b/qutebrowser/browser/webengine/webenginedownloads.py @@ -0,0 +1,158 @@ +# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: + +# Copyright 2014-2016 Florian Bruhin (The Compiler) +# +# This file is part of qutebrowser. +# +# qutebrowser 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, either version 3 of the License, or +# (at your option) any later version. +# +# qutebrowser 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 qutebrowser. If not, see . + +"""QtWebEngine specific code for downloads.""" + +import os.path +import functools + +from PyQt5.QtCore import pyqtSlot, Qt +# pylint: disable=no-name-in-module,import-error,useless-suppression +from PyQt5.QtWebEngineWidgets import QWebEngineDownloadItem +# pylint: enable=no-name-in-module,import-error,useless-suppression + +from qutebrowser.browser import downloads +from qutebrowser.utils import debug, usertypes, message, log + + +class DownloadItem(downloads.AbstractDownloadItem): + + """A wrapper over a QWebEngineDownloadItem. + + Attributes: + _qt_item: The wrapped item. + """ + + def __init__(self, qt_item, parent=None): + super().__init__(parent) + self._qt_item = qt_item + qt_item.downloadProgress.connect(self.stats.on_download_progress) + qt_item.stateChanged.connect(self._on_state_changed) + + @pyqtSlot(QWebEngineDownloadItem.DownloadState) + def _on_state_changed(self, state): + state_name = debug.qenum_key(QWebEngineDownloadItem, state) + log.downloads.debug("State for {!r} changed to {}".format( + self, state_name)) + + if state == QWebEngineDownloadItem.DownloadRequested: + pass + elif state == QWebEngineDownloadItem.DownloadInProgress: + pass + elif state == QWebEngineDownloadItem.DownloadCompleted: + log.downloads.debug("Download {} finished".format(self.basename)) + self.successful = True + self.done = True + self.finished.emit() + self.stats.finish() + elif state == QWebEngineDownloadItem.DownloadCancelled: + self.successful = False + self.done = True + self.cancelled.emit() + self.stats.finish() + elif state == QWebEngineDownloadItem.DownloadInterrupted: + self.successful = False + self.done = True + # https://bugreports.qt.io/browse/QTBUG-56839 + self.error.emit("Download failed") + self.stats.finish() + else: + raise ValueError("_on_state_changed was called with unknown state " + "{}".format(state_name)) + + def _do_die(self): + self._qt_item.downloadProgress.disconnect() + self._qt_item.cancel() + + def _do_cancel(self): + self._qt_item.cancel() + + def retry(self): + # https://bugreports.qt.io/browse/QTBUG-56840 + raise downloads.UnsupportedOperationError + + def _get_open_filename(self): + return self._filename + + def _set_fileobj(self, fileobj): + raise downloads.UnsupportedOperationError + + def _set_tempfile(self, fileobj): + self._set_filename(fileobj.name, force_overwrite=True) + + def _ensure_can_set_filename(self, filename): + state = self._qt_item.state() + if state != QWebEngineDownloadItem.DownloadRequested: + state_name = debug.qenum_key(QWebEngineDownloadItem, state) + raise ValueError("Trying to set filename {} on {!r} which is " + "state {} (not in requested state)!".format( + filename, self, state_name)) + + def _ask_confirm_question(self, title, msg): + no_action = functools.partial(self.cancel, remove_data=False) + question = usertypes.Question() + question.title = title + question.text = msg + question.mode = usertypes.PromptMode.yesno + question.answered_yes.connect(self._after_set_filename) + question.answered_no.connect(no_action) + question.cancelled.connect(no_action) + self.cancelled.connect(question.abort) + self.error.connect(question.abort) + message.global_bridge.ask(question, blocking=True) + + def _after_set_filename(self): + self._qt_item.setPath(self._filename) + self._qt_item.accept() + + +class DownloadManager(downloads.AbstractDownloadManager): + + """Manager for currently running downloads.""" + + def install(self, profile): + """Set up the download manager on a QWebEngineProfile.""" + profile.downloadRequested.connect(self.handle_download, + Qt.DirectConnection) + + @pyqtSlot(QWebEngineDownloadItem) + def handle_download(self, qt_item): + """Start a download coming from a QWebEngineProfile.""" + suggested_filename = os.path.basename(qt_item.path()) + + download = DownloadItem(qt_item) + self._init_item(download, auto_remove=False, + suggested_filename=suggested_filename) + + filename = downloads.immediate_download_path() + if filename is not None: + # User doesn't want to be asked, so just use the download_dir + target = downloads.FileDownloadTarget(filename) + download.set_target(target) + return + + # Ask the user for a filename - needs to be blocking! + question = downloads.get_filename_question( + suggested_filename=suggested_filename, url=qt_item.url(), + parent=self) + self._init_filename_question(question, download) + + message.global_bridge.ask(question, blocking=True) + # The filename is set via the question.answered signal, connected in + # _init_filename_question. diff --git a/qutebrowser/browser/webengine/webenginetab.py b/qutebrowser/browser/webengine/webenginetab.py index 8650774c4..46921d455 100644 --- a/qutebrowser/browser/webengine/webenginetab.py +++ b/qutebrowser/browser/webengine/webenginetab.py @@ -34,7 +34,8 @@ from PyQt5.QtWebEngineWidgets import (QWebEnginePage, QWebEngineScript, from qutebrowser.browser import browsertab, mouse from qutebrowser.browser.webengine import (webview, webengineelem, tabhistory, - interceptor, webenginequtescheme) + interceptor, webenginequtescheme, + webenginedownloads) from qutebrowser.utils import (usertypes, qtutils, log, javascript, utils, objreg) @@ -61,6 +62,11 @@ def init(): host_blocker, parent=app) req_interceptor.install(profile) + log.init.debug("Initializing QtWebEngine downloads...") + download_manager = webenginedownloads.DownloadManager(parent=app) + download_manager.install(profile) + objreg.register('webengine-download-manager', download_manager) + # Mapping worlds from usertypes.JsWorld to QWebEngineScript world IDs. _JS_WORLD_MAP = { diff --git a/qutebrowser/browser/webkit/mhtml.py b/qutebrowser/browser/webkit/mhtml.py index 41d4e7355..237898809 100644 --- a/qutebrowser/browser/webkit/mhtml.py +++ b/qutebrowser/browser/webkit/mhtml.py @@ -19,6 +19,7 @@ """Utils for writing an MHTML file.""" +import html import functools import io import os @@ -34,7 +35,8 @@ import email.message from PyQt5.QtCore import QUrl -from qutebrowser.browser.webkit import webkitelem, downloads +from qutebrowser.browser import downloads +from qutebrowser.browser.webkit import webkitelem from qutebrowser.utils import log, objreg, message, usertypes, utils, urlutils _File = collections.namedtuple('_File', @@ -341,9 +343,9 @@ class _Downloader: self.writer.add_file(urlutils.encoded_url(url), b'') return - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) - target = usertypes.FileObjDownloadTarget(_NoCloseBytesIO()) + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window=self._win_id) + target = downloads.FileObjDownloadTarget(_NoCloseBytesIO()) item = download_manager.get(url, target=target, auto_remove=True) self.pending_downloads.add((url, item)) @@ -536,10 +538,10 @@ def start_download_checked(dest, tab): q = usertypes.Question() q.mode = usertypes.PromptMode.yesno - q.text = "{} exists. Overwrite?".format(path) + q.title = "Overwrite existing file?" + q.text = "{} already exists. Overwrite?".format( + html.escape(path)) q.completed.connect(q.deleteLater) q.answered_yes.connect(functools.partial( _start_download, path, tab=tab)) - message_bridge = objreg.get('message-bridge', scope='window', - window=tab.win_id) - message_bridge.ask(q, blocking=False) + message.global_bridge.ask(q, blocking=False) diff --git a/qutebrowser/browser/webkit/network/networkmanager.py b/qutebrowser/browser/webkit/network/networkmanager.py index 7024e7635..81ce0eb79 100644 --- a/qutebrowser/browser/webkit/network/networkmanager.py +++ b/qutebrowser/browser/webkit/network/networkmanager.py @@ -135,11 +135,6 @@ class NetworkManager(QNetworkAccessManager): """Our own QNetworkAccessManager. Attributes: - adopted_downloads: If downloads are running with this QNAM but the - associated tab gets closed already, the NAM gets - reparented to the DownloadManager. This counts the - still running downloads, so the QNAM can clean - itself up when this reaches zero again. _requests: Pending requests. _scheme_handlers: A dictionary (scheme -> handler) of supported custom schemes. @@ -161,7 +156,6 @@ class NetworkManager(QNetworkAccessManager): # http://www.riverbankcomputing.com/pipermail/pyqt/2014-November/035045.html super().__init__(parent) log.init.debug("NetworkManager init done") - self.adopted_downloads = 0 self._win_id = win_id self._tab_id = tab_id self._requests = [] @@ -394,28 +388,6 @@ class NetworkManager(QNetworkAccessManager): # switched from private mode to normal mode self._set_cookiejar() - @pyqtSlot() - def on_adopted_download_destroyed(self): - """Check if we can clean up if an adopted download was destroyed. - - See the description for adopted_downloads for details. - """ - self.adopted_downloads -= 1 - log.downloads.debug("Adopted download destroyed, {} left.".format( - self.adopted_downloads)) - assert self.adopted_downloads >= 0 - if self.adopted_downloads == 0: - self.deleteLater() - - @pyqtSlot(object) # DownloadItem - def adopt_download(self, download): - """Adopt a new DownloadItem.""" - self.adopted_downloads += 1 - log.downloads.debug("Adopted download, {} adopted.".format( - self.adopted_downloads)) - download.destroyed.connect(self.on_adopted_download_destroyed) - download.do_retry.connect(self.adopt_download) - def set_referer(self, req, current_url): """Set the referer header.""" referer_header_conf = config.get('network', 'referer-header') diff --git a/qutebrowser/browser/webkit/webpage.py b/qutebrowser/browser/webkit/webpage.py index 00b253766..47adc0aa3 100644 --- a/qutebrowser/browser/webkit/webpage.py +++ b/qutebrowser/browser/webkit/webpage.py @@ -219,13 +219,7 @@ class BrowserPage(QWebPage): """Prepare the web page for being deleted.""" self._is_shutting_down = True self.shutting_down.emit() - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) - nam = self.networkAccessManager() - if download_manager.has_downloads_with_nam(nam): - nam.setParent(download_manager) - else: - nam.shutdown() + self.networkAccessManager().shutdown() def display_content(self, reply, mimetype): """Display a QNetworkReply with an explicitly set mimetype.""" @@ -252,9 +246,9 @@ class BrowserPage(QWebPage): after this slot returns. """ req = QNetworkRequest(request) - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) - download_manager.get_request(req, qnam=self.networkAccessManager()) + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window=self._win_id) + download_manager.get_request(req) @pyqtSlot('QNetworkReply*') def on_unsupported_content(self, reply): @@ -267,8 +261,8 @@ class BrowserPage(QWebPage): here: http://mimesniff.spec.whatwg.org/ """ inline, suggested_filename = http.parse_content_disposition(reply) - download_manager = objreg.get('download-manager', scope='window', - window=self._win_id) + download_manager = objreg.get('qtnetwork-download-manager', + scope='window', window=self._win_id) if not inline: # Content-Disposition: attachment -> force download download_manager.fetch(reply, diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index de557f940..d939d6928 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -29,7 +29,7 @@ from qutebrowser.utils import message, log, objreg, standarddir from qutebrowser.commands import runners from qutebrowser.config import config from qutebrowser.misc import guiprocess -from qutebrowser.browser.webkit import downloads +from qutebrowser.browser import downloads class _QtFIFOReader(QObject): diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index 7c8ba98c5..2cfea1510 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -413,7 +413,20 @@ class ConfigManager(QObject): CHANGED_OPTIONS = { ('content', 'cookies-accept'): _get_value_transformer({'default': 'no-3rdparty'}), + ('tabs', 'new-tab-position'): + _get_value_transformer({ + 'left': 'prev', + 'right': 'next'}), + ('tabs', 'new-tab-position-explicit'): + _get_value_transformer({ + 'left': 'prev', + 'right': 'next'}), ('tabs', 'position'): _transform_position, + ('tabs', 'select-on-remove'): + _get_value_transformer({ + 'left': 'prev', + 'right': 'next', + 'previous': 'last-used'}), ('ui', 'downloads-position'): _transform_position, ('ui', 'remove-finished-downloads'): _get_value_transformer({'false': '-1', 'true': '1000'}), diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index 1dadb4fb1..2e9c504ea 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -584,11 +584,11 @@ def data(readonly=False): "background."), ('select-on-remove', - SettingValue(typ.SelectOnRemove(), 'right'), + SettingValue(typ.SelectOnRemove(), 'next'), "Which tab to select when the focused tab is removed."), ('new-tab-position', - SettingValue(typ.NewTabPosition(), 'right'), + SettingValue(typ.NewTabPosition(), 'next'), "How new tabs are positioned."), ('new-tab-position-explicit', @@ -1814,4 +1814,14 @@ CHANGED_KEY_COMMANDS = [ (re.compile(r'^prompt-yes$'), r'prompt-accept yes'), (re.compile(r'^prompt-no$'), r'prompt-accept no'), + + (re.compile(r'^tab-close -l$'), r'tab-close --prev'), + (re.compile(r'^tab-close --left$'), r'tab-close --prev'), + (re.compile(r'^tab-close -r$'), r'tab-close --next'), + (re.compile(r'^tab-close --right$'), r'tab-close --next'), + + (re.compile(r'^tab-only -l$'), r'tab-only --prev'), + (re.compile(r'^tab-only --left$'), r'tab-only --prev'), + (re.compile(r'^tab-only -r$'), r'tab-only --next'), + (re.compile(r'^tab-only --right$'), r'tab-only --next'), ] diff --git a/qutebrowser/config/configtypes.py b/qutebrowser/config/configtypes.py index 4fb40bfbb..346b5a8d7 100644 --- a/qutebrowser/config/configtypes.py +++ b/qutebrowser/config/configtypes.py @@ -1379,18 +1379,20 @@ class SelectOnRemove(MappingType): """Which tab to select when the focused tab is removed.""" MAPPING = { - 'left': QTabBar.SelectLeftTab, - 'right': QTabBar.SelectRightTab, - 'previous': QTabBar.SelectPreviousTab, + 'prev': QTabBar.SelectLeftTab, + 'next': QTabBar.SelectRightTab, + 'last-used': QTabBar.SelectPreviousTab, } def __init__(self, none_ok=False): super().__init__( none_ok, valid_values=ValidValues( - ('left', "Select the tab on the left."), - ('right', "Select the tab on the right."), - ('previous', "Select the previously selected tab."))) + ('prev', "Select the tab which came before the closed one " + "(left in horizontal, above in vertical)."), + ('next', "Select the tab which came after the closed one " + "(right in horizontal, below in vertical)."), + ('last-used', "Select the previously selected tab."))) class ConfirmQuit(FlagList): @@ -1434,10 +1436,10 @@ class NewTabPosition(BaseType): def __init__(self, none_ok=False): super().__init__(none_ok) self.valid_values = ValidValues( - ('left', "On the left of the current tab."), - ('right', "On the right of the current tab."), - ('first', "At the left end."), - ('last', "At the right end.")) + ('prev', "Before the current tab."), + ('next', "After the current tab."), + ('first', "At the beginning."), + ('last', "At the end.")) class IgnoreCase(Bool): diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index 0e5a5c069..9e8f5a191 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -35,8 +35,8 @@ from qutebrowser.mainwindow import tabbedbrowser, messageview, prompt from qutebrowser.mainwindow.statusbar import bar from qutebrowser.completion import completionwidget, completer from qutebrowser.keyinput import modeman -from qutebrowser.browser import commands, downloadview, hints -from qutebrowser.browser.webkit import downloads +from qutebrowser.browser import (commands, downloadview, hints, + qtnetworkdownloads, downloads) from qutebrowser.misc import crashsignal, keyhintwidget @@ -258,10 +258,20 @@ class MainWindow(QWidget): def _init_downloadmanager(self): log.init.debug("Initializing downloads...") - download_manager = downloads.DownloadManager(self.win_id, self) - objreg.register('download-manager', download_manager, scope='window', - window=self.win_id) - download_model = downloads.DownloadModel(download_manager) + qtnetwork_download_manager = qtnetworkdownloads.DownloadManager( + self.win_id, self) + objreg.register('qtnetwork-download-manager', + qtnetwork_download_manager, + scope='window', window=self.win_id) + + try: + webengine_download_manager = objreg.get( + 'webengine-download-manager') + except KeyError: + webengine_download_manager = None + + download_model = downloads.DownloadModel(qtnetwork_download_manager, + webengine_download_manager) objreg.register('download-model', download_model, scope='window', window=self.win_id) diff --git a/qutebrowser/mainwindow/prompt.py b/qutebrowser/mainwindow/prompt.py index 7435a46a7..f5ed10009 100644 --- a/qutebrowser/mainwindow/prompt.py +++ b/qutebrowser/mainwindow/prompt.py @@ -29,6 +29,7 @@ from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QTimer, QDir, QModelIndex, from PyQt5.QtWidgets import (QWidget, QGridLayout, QVBoxLayout, QLineEdit, QLabel, QFileSystemModel, QTreeView, QSizePolicy) +from qutebrowser.browser import downloads from qutebrowser.config import style from qutebrowser.utils import usertypes, log, utils, qtutils, objreg, message from qutebrowser.keyinput import modeman @@ -687,11 +688,11 @@ class DownloadFilenamePrompt(FilenamePrompt): def accept(self, value=None): text = value if value is not None else self._lineedit.text() - self.question.answer = usertypes.FileDownloadTarget(text) + self.question.answer = downloads.FileDownloadTarget(text) return True def download_open(self, cmdline): - self.question.answer = usertypes.OpenFileDownloadTarget(cmdline) + self.question.answer = downloads.OpenFileDownloadTarget(cmdline) self.question.done() message.global_bridge.prompt_done.emit(self.KEY_MODE) diff --git a/qutebrowser/mainwindow/tabbedbrowser.py b/qutebrowser/mainwindow/tabbedbrowser.py index 879013f38..59c11666b 100644 --- a/qutebrowser/mainwindow/tabbedbrowser.py +++ b/qutebrowser/mainwindow/tabbedbrowser.py @@ -61,8 +61,8 @@ class TabbedBrowser(tabwidget.TabWidget): _filter: A SignalFilter instance. _now_focused: The tab which is focused now. _tab_insert_idx_left: Where to insert a new tab with - tabbar -> new-tab-position set to 'left'. - _tab_insert_idx_right: Same as above, for 'right'. + tabbar -> new-tab-position set to 'prev'. + _tab_insert_idx_right: Same as above, for 'next'. _undo_stack: List of UndoEntry namedtuples of closed tabs. shutting_down: Whether we're currently shutting down. _local_marks: Jump markers local to each page @@ -411,14 +411,14 @@ class TabbedBrowser(tabwidget.TabWidget): pos = config.get('tabs', 'new-tab-position-explicit') else: pos = config.get('tabs', 'new-tab-position') - if pos == 'left': + if pos == 'prev': idx = self._tab_insert_idx_left # On first sight, we'd think we have to decrement # self._tab_insert_idx_left here, as we want the next tab to be # *before* the one we just opened. However, since we opened a tab - # *to the left* of the currently focused tab, indices will shift by + # *before* the currently focused tab, indices will shift by # 1 automatically. - elif pos == 'right': + elif pos == 'next': idx = self._tab_insert_idx_right self._tab_insert_idx_right += 1 elif pos == 'first': diff --git a/qutebrowser/misc/ipc.py b/qutebrowser/misc/ipc.py index 4afae78e9..6641a9ee8 100644 --- a/qutebrowser/misc/ipc.py +++ b/qutebrowser/misc/ipc.py @@ -362,7 +362,7 @@ class IPCServer(QObject): @pyqtSlot() def on_timeout(self): """Cancel the current connection if it was idle for too long.""" - if self._socket is None: + if self._socket is None: # pragma: no cover log.ipc.error("on_timeout got called with None socket!") return log.ipc.error("IPC connection timed out " diff --git a/qutebrowser/utils/objreg.py b/qutebrowser/utils/objreg.py index 06a29c85e..d05424fc3 100644 --- a/qutebrowser/utils/objreg.py +++ b/qutebrowser/utils/objreg.py @@ -23,7 +23,6 @@ import collections import functools -import sip from PyQt5.QtCore import QObject, QTimer from qutebrowser.utils import log @@ -144,8 +143,12 @@ class ObjectRegistry(collections.UserDict): """Dump all objects as a list of strings.""" lines = [] for name, obj in self.data.items(): - if not (isinstance(obj, QObject) and sip.isdeleted(obj)): - lines.append("{}: {}".format(name, repr(obj))) + try: + obj_repr = repr(obj) + except (RuntimeError, TypeError): + # Underlying object deleted probably + obj_repr = '' + lines.append("{}: {}".format(name, obj_repr)) return lines diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 4e67a2eac..89480d241 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -268,56 +268,6 @@ JsWorld = enum('JsWorld', ['main', 'application', 'user', 'jseval']) MessageLevel = enum('MessageLevel', ['error', 'warning', 'info']) -# Where a download should be saved -class DownloadTarget: - - """Abstract base class for different download targets.""" - - def __init__(self): - raise NotImplementedError - - -class FileDownloadTarget(DownloadTarget): - - """Save the download to the given file. - - Attributes: - filename: Filename where the download should be saved. - """ - - def __init__(self, filename): - # pylint: disable=super-init-not-called - self.filename = filename - - -class FileObjDownloadTarget(DownloadTarget): - - """Save the download to the given file-like object. - - Attributes: - fileobj: File-like object where the download should be written to. - """ - - def __init__(self, fileobj): - # pylint: disable=super-init-not-called - self.fileobj = fileobj - - -class OpenFileDownloadTarget(DownloadTarget): - - """Save the download in a temp dir and directly open it. - - Attributes: - cmdline: The command to use as string. A `{}` is expanded to the - filename. None means to use the system's default application. - If no `{}` is found, the filename is appended to the cmdline. - """ - - def __init__(self, cmdline=None): - # pylint: disable=super-init-not-called - self.cmdline = cmdline - - class Question(QObject): """A question asked to the user, e.g. via the status bar. diff --git a/tests/end2end/features/downloads.feature b/tests/end2end/features/downloads.feature index 9a868cd57..f187a7336 100644 --- a/tests/end2end/features/downloads.feature +++ b/tests/end2end/features/downloads.feature @@ -75,6 +75,7 @@ Feature: Downloading things from a website. And I run :leave-mode Then no crash should happen + @qtwebengine_todo: ssl-strict is not implemented yet Scenario: Downloading with SSL errors (issue 1413) When I run :debug-clear-ssl-errors And I set network -> ssl-strict to ask @@ -85,7 +86,7 @@ Feature: Downloading things from a website. Scenario: Closing window with remove-finished-downloads timeout (issue 1242) When I set ui -> remove-finished-downloads to 500 - And I open data/downloads/download.bin in a new window + And I open data/downloads/download.bin in a new window without waiting And I wait until the download is finished And I run :close And I wait 0.5s @@ -95,7 +96,7 @@ Feature: Downloading things from a website. Given I have a fresh instance When I set storage -> prompt-download-directory to false And I set ui -> confirm-quit to downloads - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting And I wait until the download is finished And I run :close Then qutebrowser should quit @@ -171,18 +172,36 @@ Feature: Downloading things from a website. ## mhtml downloads + @qtwebengine_todo: :download --mhtml is not implemented yet Scenario: Downloading as mhtml is available When I open html And I run :download --mhtml And I wait for "File successfully written." in the log Then no crash should happen + @qtwebengine_todo: :download --mhtml is not implemented yet Scenario: Downloading as mhtml with non-ASCII headers When I open response-headers?Content-Type=text%2Fpl%C3%A4in And I run :download --mhtml --dest mhtml-response-headers.mht And I wait for "File successfully written." in the log Then no crash should happen + @qtwebengine_todo: :download --mhtml is not implemented yet + Scenario: Overwriting existing mhtml file + 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 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 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 + ## :download-cancel Scenario: Cancelling a download @@ -199,13 +218,13 @@ Feature: Downloading things from a website. Then the error "There's no download 42!" should be shown Scenario: Cancelling a download which is already done - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I run :download-cancel Then the error "Download 1 is already done!" should be shown Scenario: Cancelling a download which is already done (with count) - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I run :download-cancel with count 1 Then the error "Download 1 is already done!" should be shown @@ -218,6 +237,7 @@ Feature: Downloading things from a website. And "cancelled" should be logged # https://github.com/The-Compiler/qutebrowser/issues/1535 + @qtwebengine_todo: :download --mhtml is not implemented yet Scenario: Cancelling an MHTML download (issue 1535) When I open data/downloads/issue1535.html And I run :download --mhtml @@ -228,7 +248,7 @@ Feature: Downloading things from a website. ## :download-remove / :download-clear Scenario: Removing a download - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I run :download-remove Then "Removed download *" should be logged @@ -248,17 +268,17 @@ Feature: Downloading things from a website. Then the error "Download 1 is not done!" should be shown Scenario: Removing all downloads via :download-remove - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished - And I open data/downloads/download2.bin + And I open data/downloads/download2.bin without waiting And I wait until the download is finished And I run :download-remove --all Then "Removed download *" should be logged Scenario: Removing all downloads via :download-clear - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished - And I open data/downloads/download2.bin + And I open data/downloads/download2.bin without waiting And I wait until the download is finished And I run :download-clear Then "Removed download *" should be logged @@ -266,7 +286,7 @@ Feature: Downloading things from a website. ## :download-delete Scenario: Deleting a download - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I run :download-delete And I wait for "deleted download *" in the log @@ -289,13 +309,13 @@ Feature: Downloading things from a website. ## :download-open Scenario: Opening a download - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I open the download Then "Opening *download.bin* with [*python*]" should be logged Scenario: Opening a download with a placeholder - When I open data/downloads/download.bin + When I open data/downloads/download.bin without waiting And I wait until the download is finished And I open the download with a placeholder Then "Opening *download.bin* with [*python*]" should be logged @@ -318,7 +338,8 @@ Feature: Downloading things from a website. Scenario: Opening a download directly When I set storage -> prompt-download-directory to true - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting + And I wait for the download prompt for "*" And I directly open the download And I wait until the download is finished Then "Opening *download.bin* with [*python*]" should be logged @@ -328,6 +349,7 @@ Feature: Downloading things from a website. Scenario: Cancelling a download that should be opened When I set storage -> prompt-download-directory to true And I run :download http://localhost:(port)/drip?numbytes=128&duration=5 + And I wait for the download prompt for "*" And I directly open the download And I run :download-cancel Then "* finished but not successful, not opening!" should be logged @@ -338,7 +360,7 @@ Feature: Downloading things from a website. When I set storage -> prompt-download-directory to true And I open data/downloads/issue1725.html And I run :click-element id long-link - And I wait for "Asking question text=* title='Save file to:'>, *" in the log + And I wait for the download prompt for "*" And I directly open the download And I wait until the download is finished Then "Opening * with [*python*]" should be logged @@ -348,19 +370,19 @@ Feature: Downloading things from a website. Scenario: completion -> download-path-suggestion = path When I set storage -> prompt-download-directory to true And I set completion -> download-path-suggestion to path - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting Then the download prompt should be shown with "(tmpdir)/" Scenario: completion -> download-path-suggestion = filename When I set storage -> prompt-download-directory to true And I set completion -> download-path-suggestion to filename - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting Then the download prompt should be shown with "download.bin" Scenario: completion -> download-path-suggestion = both When I set storage -> prompt-download-directory to true And I set completion -> download-path-suggestion to both - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting Then the download prompt should be shown with "(tmpdir)/download.bin" ## storage -> remember-download-directory @@ -369,20 +391,20 @@ Feature: Downloading things from a website. When I set storage -> prompt-download-directory to true And I set completion -> download-path-suggestion to both And I set storage -> remember-download-directory to true - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting And I wait for the download prompt for "*/download.bin" And I run :prompt-accept (tmpdir)(dirsep)subdir - And I open data/downloads/download2.bin + And I open data/downloads/download2.bin without waiting Then the download prompt should be shown with "(tmpdir)/subdir/download2.bin" Scenario: Not remembering the last download directory When I set storage -> prompt-download-directory to true And I set completion -> download-path-suggestion to both And I set storage -> remember-download-directory to false - And I open data/downloads/download.bin + And I open data/downloads/download.bin without waiting And I wait for the download prompt for "(tmpdir)/download.bin" And I run :prompt-accept (tmpdir)(dirsep)subdir - And I open data/downloads/download2.bin + And I open data/downloads/download2.bin without waiting Then the download prompt should be shown with "(tmpdir)/download2.bin" # Overwriting files @@ -475,6 +497,7 @@ Feature: Downloading things from a website. And I run :download foo! Then the error "Invalid URL" should be shown + @qtwebengine_todo: pdfjs is not implemented yet Scenario: Downloading via pdfjs Given pdfjs is available When I set storage -> prompt-download-directory to false @@ -496,3 +519,9 @@ Feature: Downloading things from a website. And I wait until the download is finished Then the downloaded file download.bin should exist And the downloaded file download2.bin should not exist + + Scenario: Downloading a file with unknown size + When I set storage -> prompt-download-directory to false + And I open stream-bytes/1024 without waiting + And I wait until the download is finished + Then the downloaded file 1024 should exist diff --git a/tests/end2end/features/open.feature b/tests/end2end/features/open.feature index 45012e639..3f6715e6a 100644 --- a/tests/end2end/features/open.feature +++ b/tests/end2end/features/open.feature @@ -75,8 +75,8 @@ Feature: Opening pages Scenario: Opening in a new tab (explicit) Given I open about:blank - When I set tabs -> new-tab-position-explicit to right - And I set tabs -> new-tab-position to left + When I set tabs -> new-tab-position-explicit to next + And I set tabs -> new-tab-position to prev And I run :tab-only And I run :open -t http://localhost:(port)/data/numbers/7.txt And I wait until data/numbers/7.txt is loaded @@ -86,8 +86,8 @@ Feature: Opening pages Scenario: Opening in a new tab (implicit) Given I open about:blank - When I set tabs -> new-tab-position-explicit to right - And I set tabs -> new-tab-position to left + When I set tabs -> new-tab-position-explicit to next + And I set tabs -> new-tab-position to prev And I run :tab-only And I run :open -t -i http://localhost:(port)/data/numbers/8.txt And I wait until data/numbers/8.txt is loaded diff --git a/tests/end2end/features/tabs.feature b/tests/end2end/features/tabs.feature index b138397c5..1663dd8d7 100644 --- a/tests/end2end/features/tabs.feature +++ b/tests/end2end/features/tabs.feature @@ -35,8 +35,8 @@ Feature: Tab management - data/numbers/2.txt - data/numbers/3.txt (active) - Scenario: :tab-close with select-on-remove = right - When I set tabs -> select-on-remove to right + Scenario: :tab-close with select-on-remove = next + When I set tabs -> select-on-remove to next And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab @@ -46,8 +46,8 @@ Feature: Tab management - data/numbers/1.txt - data/numbers/3.txt (active) - Scenario: :tab-close with select-on-remove = left - When I set tabs -> select-on-remove to left + Scenario: :tab-close with select-on-remove = prev + When I set tabs -> select-on-remove to prev And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab @@ -57,8 +57,8 @@ Feature: Tab management - data/numbers/1.txt (active) - data/numbers/3.txt - Scenario: :tab-close with select-on-remove = previous - When I set tabs -> select-on-remove to previous + Scenario: :tab-close with select-on-remove = last-used + When I set tabs -> select-on-remove to last-used And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab @@ -70,30 +70,30 @@ Feature: Tab management - data/numbers/3.txt - data/numbers/4.txt (active) - Scenario: :tab-close with select-on-remove = left and --right - When I set tabs -> select-on-remove to left + Scenario: :tab-close with select-on-remove = prev and --next + When I set tabs -> select-on-remove to prev And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab And I run :tab-focus 2 - And I run :tab-close --right + And I run :tab-close --next Then the following tabs should be open: - data/numbers/1.txt - data/numbers/3.txt (active) - Scenario: :tab-close with select-on-remove = right and --left - When I set tabs -> select-on-remove to right + Scenario: :tab-close with select-on-remove = next and --prev + When I set tabs -> select-on-remove to next And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab And I run :tab-focus 2 - And I run :tab-close --left + And I run :tab-close --prev Then the following tabs should be open: - data/numbers/1.txt (active) - data/numbers/3.txt - Scenario: :tab-close with select-on-remove = left and --opposite - When I set tabs -> select-on-remove to left + Scenario: :tab-close with select-on-remove = prev and --opposite + When I set tabs -> select-on-remove to prev And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab @@ -103,8 +103,8 @@ Feature: Tab management - data/numbers/1.txt - data/numbers/3.txt (active) - Scenario: :tab-close with select-on-remove = right and --opposite - When I set tabs -> select-on-remove to right + Scenario: :tab-close with select-on-remove = next and --opposite + When I set tabs -> select-on-remove to next And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab @@ -114,19 +114,19 @@ Feature: Tab management - data/numbers/1.txt (active) - data/numbers/3.txt - Scenario: :tab-close with select-on-remove = previous and --opposite - When I set tabs -> select-on-remove to previous + Scenario: :tab-close with select-on-remove = last-used and --opposite + When I set tabs -> select-on-remove to last-used And I run :tab-close --opposite - Then the error "-o is not supported with 'tabs->select-on-remove' set to 'previous'!" should be shown + Then the error "-o is not supported with 'tabs->select-on-remove' set to 'last-used'!" should be shown Scenario: :tab-close should restore selection behavior - When I set tabs -> select-on-remove to right + When I set tabs -> select-on-remove to next And I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab And I open data/numbers/4.txt in a new tab And I run :tab-focus 2 - And I run :tab-close --left + And I run :tab-close --prev And I run :tab-focus 2 And I run :tab-close Then the following tabs should be open: @@ -143,29 +143,29 @@ Feature: Tab management Then the following tabs should be open: - data/numbers/3.txt (active) - Scenario: :tab-only with --left + Scenario: :tab-only with --prev When I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab And I run :tab-focus 2 - And I run :tab-only --left + And I run :tab-only --prev Then the following tabs should be open: - data/numbers/1.txt - data/numbers/2.txt (active) - Scenario: :tab-only with --right + Scenario: :tab-only with --next When I open data/numbers/1.txt And I open data/numbers/2.txt in a new tab And I open data/numbers/3.txt in a new tab And I run :tab-focus 2 - And I run :tab-only --right + And I run :tab-only --next Then the following tabs should be open: - data/numbers/2.txt (active) - data/numbers/3.txt - Scenario: :tab-only with --left and --right - When I run :tab-only --left --right - Then the error "Only one of -l/-r can be given!" should be shown + Scenario: :tab-only with --prev and --next + When I run :tab-only --prev --next + Then the error "Only one of -p/-n can be given!" should be shown # :tab-focus @@ -821,8 +821,8 @@ Feature: Tab management - data/hints/html/simple.html (active) - data/hello.txt - Scenario: opening tab with tabs->new-tab-position left - When I set tabs -> new-tab-position to left + Scenario: opening tab with tabs->new-tab-position prev + When I set tabs -> new-tab-position to prev And I set tabs -> background-tabs to false And I open about:blank And I open data/hints/html/simple.html in a new tab @@ -833,8 +833,8 @@ Feature: Tab management - data/hello.txt (active) - data/hints/html/simple.html - Scenario: opening tab with tabs->new-tab-position right - When I set tabs -> new-tab-position to right + Scenario: opening tab with tabs->new-tab-position next + When I set tabs -> new-tab-position to next And I set tabs -> background-tabs to false And I open about:blank And I open data/hints/html/simple.html in a new tab diff --git a/tests/end2end/features/test_downloads_bdd.py b/tests/end2end/features/test_downloads_bdd.py index ac82aa04f..65ccf1f3e 100644 --- a/tests/end2end/features/test_downloads_bdd.py +++ b/tests/end2end/features/test_downloads_bdd.py @@ -21,15 +21,10 @@ import os import sys import shlex -import pytest import pytest_bdd as bdd bdd.scenarios('downloads.feature') -pytestmark = pytest.mark.qtwebengine_todo("Downloads not implemented yet", - run=False) - - PROMPT_MSG = ("Asking question text=* " "title='Save file to:'>, *") diff --git a/tests/end2end/features/yankpaste.feature b/tests/end2end/features/yankpaste.feature index b568f23fe..9a4bf53a8 100644 --- a/tests/end2end/features/yankpaste.feature +++ b/tests/end2end/features/yankpaste.feature @@ -263,6 +263,7 @@ Feature: Yanking and pasting. And I run :click-element id qute-textarea And I wait for "Clicked editable element!" in the log And I run :insert-text Hello world + And I wait for "Inserting text into element *" in the log And I run :jseval console.log("textarea contents: " + document.getElementById('qute-textarea').value); # Enable javascript again for the other tests And I set content -> allow-javascript to true diff --git a/tests/unit/browser/test_adblock.py b/tests/unit/browser/test_adblock.py index a1a44d5ec..b0285a8f5 100644 --- a/tests/unit/browser/test_adblock.py +++ b/tests/unit/browser/test_adblock.py @@ -101,10 +101,11 @@ class FakeDownloadManager: def download_stub(win_registry): """Register a FakeDownloadManager.""" stub = FakeDownloadManager() - objreg.register('download-manager', stub, + objreg.register('qtnetwork-download-manager', stub, scope='window', window='last-focused') yield - objreg.delete('download-manager', scope='window', window='last-focused') + objreg.delete('qtnetwork-download-manager', scope='window', + window='last-focused') def create_zipfile(directory, files, zipname='test'): diff --git a/tests/unit/browser/webkit/test_downloads.py b/tests/unit/browser/webkit/test_downloads.py index 44f1f73d3..acee916b2 100644 --- a/tests/unit/browser/webkit/test_downloads.py +++ b/tests/unit/browser/webkit/test_downloads.py @@ -17,13 +17,46 @@ # You should have received a copy of the GNU General Public License # along with qutebrowser. If not, see . +import pytest -from qutebrowser.browser.webkit import downloads +from qutebrowser.browser import downloads, qtnetworkdownloads def test_download_model(qapp, qtmodeltester, config_stub, cookiejar_and_cache): """Simple check for download model internals.""" config_stub.data = {'general': {'private-browsing': False}} - manager = downloads.DownloadManager(win_id=0) + manager = qtnetworkdownloads.DownloadManager(win_id=0) model = downloads.DownloadModel(manager) qtmodeltester.check(model) + + +class TestDownloadTarget: + + def test_base(self): + with pytest.raises(NotImplementedError): + downloads._DownloadTarget() + + def test_filename(self): + target = downloads.FileDownloadTarget("/foo/bar") + assert target.filename == "/foo/bar" + + def test_fileobj(self): + fobj = object() + target = downloads.FileObjDownloadTarget(fobj) + assert target.fileobj is fobj + + def test_openfile(self): + target = downloads.OpenFileDownloadTarget() + assert target.cmdline is None + + def test_openfile_custom_command(self): + target = downloads.OpenFileDownloadTarget('echo') + assert target.cmdline == 'echo' + + @pytest.mark.parametrize('obj', [ + downloads.FileDownloadTarget('foobar'), + downloads.FileObjDownloadTarget(None), + downloads.OpenFileDownloadTarget(), + ]) + def test_class_hierarchy(self, obj): + assert isinstance(obj, downloads._DownloadTarget) diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py index 1afb95764..1253b6a0a 100644 --- a/tests/unit/config/test_config.py +++ b/tests/unit/config/test_config.py @@ -348,6 +348,16 @@ class TestKeyConfigParser: ('prompt-yes', 'prompt-accept yes'), ('prompt-no', 'prompt-accept no'), + + ('tab-close -l', 'tab-close --prev'), + ('tab-close --left', 'tab-close --prev'), + ('tab-close -r', 'tab-close --next'), + ('tab-close --right', 'tab-close --next'), + + ('tab-only -l', 'tab-only --prev'), + ('tab-only --left', 'tab-only --prev'), + ('tab-only -r', 'tab-only --next'), + ('tab-only --right', 'tab-only --next'), ] ) def test_migrations(self, old, new_expected): diff --git a/tests/unit/utils/usertypes/test_downloadtarget.py b/tests/unit/utils/usertypes/test_downloadtarget.py deleted file mode 100644 index 4da2e7a81..000000000 --- a/tests/unit/utils/usertypes/test_downloadtarget.py +++ /dev/null @@ -1,59 +0,0 @@ -# vim: ft=python fileencoding=utf-8 sts=4 sw=4 et: - -# Copyright 2016 Daniel Schadt -# -# This file is part of qutebrowser. -# -# qutebrowser 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, either version 3 of the License, or -# (at your option) any later version. -# -# qutebrowser 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 qutebrowser. If not, see . - -"""Tests for the DownloadTarget class.""" - -from qutebrowser.utils import usertypes - -import pytest - - -def test_base(): - with pytest.raises(NotImplementedError): - usertypes.DownloadTarget() - - -def test_filename(): - target = usertypes.FileDownloadTarget("/foo/bar") - assert target.filename == "/foo/bar" - - -def test_fileobj(): - fobj = object() - target = usertypes.FileObjDownloadTarget(fobj) - assert target.fileobj is fobj - - -def test_openfile(): - target = usertypes.OpenFileDownloadTarget() - assert target.cmdline is None - - -def test_openfile_custom_command(): - target = usertypes.OpenFileDownloadTarget('echo') - assert target.cmdline == 'echo' - - -@pytest.mark.parametrize('obj', [ - usertypes.FileDownloadTarget('foobar'), - usertypes.FileObjDownloadTarget(None), - usertypes.OpenFileDownloadTarget(), -]) -def test_class_hierarchy(obj): - assert isinstance(obj, usertypes.DownloadTarget)