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.

2 files changed, 99 insertions, 1 deletions

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)