From c0be28ebee3e1837aaf3f30ec534ccd6d038f129 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 23 Sep 2023 14:05:36 +1200 Subject: Add extra suffixes to filepicker Works around a Qt bug where QFileDialog might not see all the filename extensions you have registered for a mimetype. There is an issue where if you have suffixes (filename extensions) registered in your home dir and in a system dir, it'll only see the ones in your home dir. So get suffixes from python and proactively add them into the list the list provided by the website. Duplicating work that Qt would already do, if it weren't for the bug case we are working around. Note that this does NOT help with mimetypes like `image/*` which, we have seen in at least one example (on #7866). If we wanted to handle that case it it looks like we could get the list from: [suffix for suffix, mime in mimetypes.types_map.items() if mime.startswith('image/')] I added two tests. The first is a simple one but it felt a bit imcomplete without actually testing that we were passing the right thing to `super().chooseFiles()`. Not sure how "bad" it is to be mocking `super()` and what should be instance methods unbound from the class. Regarding the `qtutils.version_check()`, here we are only interested in the Qt version check, but that class also checks the PyQt version. The practical effect of this is that we would probably end up doing the workaround for Qt 6.7.0 if we run against that with PyQt 6.6.x. But since we are just adding stuff, it isn't slow and isn't called in a hot path it doesn't really matter. --- qutebrowser/browser/webengine/webview.py | 32 ++++++++++++- tests/unit/browser/webengine/test_webview.py | 68 ++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/qutebrowser/browser/webengine/webview.py b/qutebrowser/browser/webengine/webview.py index f3f652ad0..4fb48c060 100644 --- a/qutebrowser/browser/webengine/webview.py +++ b/qutebrowser/browser/webengine/webview.py @@ -4,6 +4,7 @@ """The main browser widget for QtWebEngine.""" +import mimetypes from typing import List, Iterable from qutebrowser.qt import machinery @@ -15,7 +16,7 @@ from qutebrowser.qt.webenginecore import QWebEnginePage, QWebEngineCertificateEr from qutebrowser.browser import shared from qutebrowser.browser.webengine import webenginesettings, certificateerror from qutebrowser.config import config -from qutebrowser.utils import log, debug, usertypes +from qutebrowser.utils import log, debug, usertypes, qtutils _QB_FILESELECTION_MODES = { @@ -258,6 +259,26 @@ class WebEnginePage(QWebEnginePage): self.navigation_request.emit(navigation) return navigation.accepted + @staticmethod + def extra_suffixes_workaround(upstream_mimetypes): + """Return any extra suffixes for mimetypes in upstream_mimetypes. + + Return any file extensions (aka suffixes) for mimetypes listed in + upstream_mimetypes that are not already contained in there. + + WORKAROUND: for https://bugreports.qt.io/browse/QTBUG-116905 + Affected Qt versions > 6.2.2 (probably) < 6.7.0 + """ + if not (qtutils.version_check("6.2.3") and not qtutils.version_check("6.7.0")): + return set() + + suffixes = {entry for entry in upstream_mimetypes if entry.startswith(".")} + mimes = {entry for entry in upstream_mimetypes if "/" in entry} + python_suffixes = set() + for mime in mimes: + python_suffixes.update(mimetypes.guess_all_extensions(mime)) + return python_suffixes - suffixes + def chooseFiles( self, mode: QWebEnginePage.FileSelectionMode, @@ -265,6 +286,15 @@ class WebEnginePage(QWebEnginePage): accepted_mimetypes: Iterable[str], ) -> List[str]: """Override chooseFiles to (optionally) invoke custom file uploader.""" + extra_suffixes = self.extra_suffixes_workaround(accepted_mimetypes) + if extra_suffixes: + log.webview.debug( + "adding extra suffixes to filepicker: before=%s added=%s", + accepted_mimetypes, + extra_suffixes, + ) + accepted_mimetypes = accepted_mimetypes + list(extra_suffixes) + handler = config.val.fileselect.handler if handler == "default": return super().chooseFiles(mode, old_files, accepted_mimetypes) diff --git a/tests/unit/browser/webengine/test_webview.py b/tests/unit/browser/webengine/test_webview.py index 98bf34f3b..b213713e2 100644 --- a/tests/unit/browser/webengine/test_webview.py +++ b/tests/unit/browser/webengine/test_webview.py @@ -4,11 +4,14 @@ import re import dataclasses +import mimetypes import pytest +from unittest import mock webview = pytest.importorskip('qutebrowser.browser.webengine.webview') from qutebrowser.qt.webenginecore import QWebEnginePage +from qutebrowser.utils import qtutils from helpers import testutils @@ -58,3 +61,68 @@ def test_enum_mappings(enum_type, naming, mapping): for name, val in members: mapped = mapping[val] assert camel_to_snake(naming, name) == mapped.name + + +@pytest.fixture +def suffix_mocks(monkeypatch): + def guess(mime): + mimetypes_map = { + "image/jpeg": [".jpg", ".jpe"], + "video/mp4": [".m4v", ".mpg4"], + } + return mimetypes_map.get(mime, []) + + monkeypatch.setattr(mimetypes, "guess_all_extensions", guess) + + def version(string): + if string == "6.2.3": + return True + if string == "6.7.0": + return False + raise AssertionError(f"unexpected version {string}") + + monkeypatch.setattr(qtutils, "version_check", version) + + +EXTRA_SUFFIXES_PARAMS = [ + (["image/jpeg"], {".jpg", ".jpe"}), + (["image/jpeg", ".jpeg"], {".jpg", ".jpe"}), + (["image/jpeg", ".jpg", ".jpe"], set()), + ( + [ + ".jpg", + ], + set(), + ), # not sure why black reformats this one and not the others + (["image/jpeg", "video/mp4"], {".jpg", ".jpe", ".m4v", ".mpg4"}), +] + + +@pytest.mark.parametrize("before, extra", EXTRA_SUFFIXES_PARAMS) +def test_suffixes_workaround_extras_returned(suffix_mocks, before, extra): + assert extra == webview.WebEnginePage.extra_suffixes_workaround(before) + + +@pytest.mark.parametrize("before, extra", EXTRA_SUFFIXES_PARAMS) +@mock.patch("qutebrowser.browser.webengine.webview.super") # mock super() calls! +def test_suffixes_workaround_choosefiles_args( + mocked_super, + suffix_mocks, + config_stub, + before, + extra, +): + # We can pass the class as "self" because we are only calling a static + # method of it. That saves us having to initilize the class and mock all + # the stuff required for __init__() + webview.WebEnginePage.chooseFiles( + webview.WebEnginePage, + QWebEnginePage.FileSelectionMode.FileSelectOpen, + [], + before, + ) + expected = set(before).union(extra) + + assert len(mocked_super().chooseFiles.call_args_list) == 1 + called_with = mocked_super().chooseFiles.call_args_list[0][0][2] + assert sorted(called_with) == sorted(expected) -- cgit v1.2.3-54-g00ecf