Handle smart dark mode setting with Qt 5.15.0

Fixes #5505

5 files changed, 89 insertions, 31 deletions

diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc
index 25206f286..2f5c89701 100644
--- a/doc/changelog.asciidoc
+++ b/doc/changelog.asciidoc
@@ -67,6 +67,8 @@ Changed
   instead of the former
   `--asciidoc path/to/python path/to/asciidoc.py`.
 - Dark mode is now supported with Qt 5.15.2 (which is not released yet)
+- The default for the darkmode `policy.images` setting is now set to `smart`
+  which fixes issues with e.g. formulas on Wikipedia.
 - The `readability-js` userscript now adds some CSS to improve the reader mode
   styling in various scenarios:
   * Images are now shrinked to the page width, similarly to what Firefox' reader
@@ -145,6 +147,9 @@ Fixed
 - When a windowed inspector is clicked, insert mode now isn't entered anymore.
 - When `:undo` to re-open a tab but `tabs.tabs_are_windows` was set between
   closing and undoing the close, qutebrowser crashed. This is now fixed.
+- With QtWebEngine 5.15.0, setting the darkmode image policy to `smart` leads to
+  renderer process crashes. The offending setting value is now ignored with a
+  warning.
 - Fixes for the `qute-pass` userscript:
   * With newer `gopass` versions, a deprecation notice was copied as
     password due to `qute-pass` using it in a deprecated way.
diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc
index eacb830af..fdc096758 100644
--- a/doc/help/settings.asciidoc
+++ b/doc/help/settings.asciidoc
@@ -1663,7 +1663,7 @@ On QtWebKit, this setting is unavailable.
 [[colors.webpage.darkmode.policy.images]]
 === colors.webpage.darkmode.policy.images
 Which images to apply dark mode to.
-WARNING: On Qt 5.15.0, this setting can cause frequent renderer process crashes due to a https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/304211[bug in Qt].
+WARNING: With QtWebengine 5.15.0, this setting can cause frequent renderer process crashes due to a https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/304211[bug in Qt]. Thus, the 'smart' setting is ignored and treated like 'never' in that case.
 
 This setting requires a restart.
 
@@ -1675,7 +1675,7 @@ Valid values:
  * +never+: Never apply dark mode filter to any images.
  * +smart+: Apply dark mode based on image content.
 
-Default: +pass:[never]+
+Default: +pass:[smart]+
 
 On QtWebEngine, this setting requires Qt 5.14 or newer.
 
diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml
index 5f100e767..8b99e4ff4 100644
--- a/qutebrowser/config/configdata.yml
+++ b/qutebrowser/config/configdata.yml
@@ -2745,7 +2745,7 @@ colors.webpage.darkmode.contrast:
     QtWebKit: false
 
 colors.webpage.darkmode.policy.images:
-  default: never
+  default: smart
   type:
     name: String
     valid_values:
@@ -2755,10 +2755,11 @@ colors.webpage.darkmode.policy.images:
   desc: >-
       Which images to apply dark mode to.
 
-      WARNING: On Qt 5.15.0, this setting can cause frequent renderer process
-      crashes due to a
+      WARNING: With QtWebengine 5.15.0, this setting can cause frequent renderer
+      process crashes due to a
       https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/304211[bug
-      in Qt].
+      in Qt]. Thus, the 'smart' setting is ignored and treated like 'never' in
+      that case.
   restart: true
   backend:
     QtWebEngine: Qt 5.14
diff --git a/qutebrowser/config/qtargs.py b/qutebrowser/config/qtargs.py
index a9d92e95d..46b54cf34 100644
--- a/qutebrowser/config/qtargs.py
+++ b/qutebrowser/config/qtargs.py
@@ -24,9 +24,15 @@ import sys
 import typing
 import argparse
 
+try:
+    from PyQt5.QtWebEngine import PYQT_WEBENGINE_VERSION
+except ImportError:
+    # Added in PyQt 5.13
+    PYQT_WEBENGINE_VERSION = None
+
 from qutebrowser.config import config
 from qutebrowser.misc import objects
-from qutebrowser.utils import usertypes, qtutils, utils
+from qutebrowser.utils import usertypes, qtutils, utils, log
 
 
 def qt_args(namespace: argparse.Namespace) -> typing.List[str]:
@@ -63,17 +69,11 @@ def qt_args(namespace: argparse.Namespace) -> typing.List[str]:
 
 def _darkmode_prefix() -> str:
     """Return the prefix to use for darkmode settings."""
-    try:
-        from PyQt5.QtWebEngine import PYQT_WEBENGINE_VERSION
-    except ImportError:
-        # Added in PyQt 5.13
+    if PYQT_WEBENGINE_VERSION is None or PYQT_WEBENGINE_VERSION < 0x050f02:
         return 'darkMode'
-
-    if PYQT_WEBENGINE_VERSION >= 0x050f02:
+    else:
         # QtWebEngine 5.15.2 comes with Chromium 83
         return 'forceDarkMode'
-    else:
-        return 'darkMode'
 
 
 def _darkmode_settings() -> typing.Iterator[typing.Tuple[str, str]]:
@@ -111,6 +111,14 @@ def _darkmode_settings() -> typing.Iterator[typing.Tuple[str, str]]:
         False: 'false',
     }
 
+    # Our defaults for policy.images are different from Chromium's, so we mark it as
+    # mandatory setting - except on Qt 5.15.0 where we don't, so we don't get the
+    # workaround warning below if the setting wasn't explicitly customized.
+    smart_image_policy_broken = PYQT_WEBENGINE_VERSION == 0x050f00
+    mandatory_settings = set()
+    if not smart_image_policy_broken:
+        mandatory_settings.add('policy.images')
+
     _setting_description_type = typing.Tuple[
         str,  # qutebrowser option name
         str,  # darkmode setting name
@@ -123,17 +131,17 @@ def _darkmode_settings() -> typing.Iterator[typing.Tuple[str, str]]:
             ('enabled', 'Enabled', bools),
             ('algorithm', 'InversionAlgorithm', algorithms),
         ]  # type: typing.List[_setting_description_type]
-        mandatory_setting = 'enabled'
+        mandatory_settings.add('enabled')
     else:
         settings = [
             ('algorithm', '', algorithms),
         ]
-        mandatory_setting = 'algorithm'
+        mandatory_settings.add('algorithm')
 
     settings += [
-        ('contrast', 'Contrast', None),
         ('policy.images', 'ImagePolicy', image_policies),
         ('policy.page', 'PagePolicy', page_policies),
+        ('contrast', 'Contrast', None),
         ('threshold.text', 'TextBrightnessThreshold', None),
         ('threshold.background', 'BackgroundBrightnessThreshold', None),
         ('grayscale.all', 'Grayscale', bools),
@@ -147,10 +155,17 @@ def _darkmode_settings() -> typing.Iterator[typing.Tuple[str, str]]:
         # actually turned on.
         value = config.instance.get(
             'colors.webpage.darkmode.' + setting,
-            fallback=setting == mandatory_setting)
+            fallback=setting in mandatory_settings)
         if isinstance(value, usertypes.Unset):
             continue
 
+        if setting == 'policy.images' and value == 'smart' and smart_image_policy_broken:
+            # WORKAROUND for
+            # https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/304211
+            log.init.warning("Ignoring colors.webpage.darkmode.policy.images = smart "
+                             "because of Qt 5.15.0 bug")
+            continue
+
         if mapping is not None:
             value = mapping[value]
 
diff --git a/tests/unit/config/test_qtargs.py b/tests/unit/config/test_qtargs.py
index 784978c72..49ace57bc 100644
--- a/tests/unit/config/test_qtargs.py
+++ b/tests/unit/config/test_qtargs.py
@@ -18,9 +18,16 @@
 
 import sys
 import os
+import logging
 
 import pytest
 
+try:
+    from PyQt5.QtWebEngine import PYQT_WEBENGINE_VERSION
+except ImportError:
+    # Added in PyQt 5.13
+    PYQT_WEBENGINE_VERSION = None
+
 from qutebrowser import qutebrowser
 from qutebrowser.config import qtargs, configdata
 from qutebrowser.utils import usertypes, version
@@ -400,16 +407,19 @@ class TestQtArgs:
         parsed = parser.parse_args([])
         args = qtargs.qt_args(parsed)
 
-        old_setting = '--blink-settings=darkModeEnabled=true'
-        new_setting = '--blink-settings=forceDarkModeEnabled=true'
+        old = '--blink-settings=darkModeEnabled=true,darkModeImagePolicy=2'
+        new = '--blink-settings=forceDarkModeEnabled=true,forceDarkModeImagePolicy=2'
 
-        assert old_setting in args or new_setting in args
+        assert old in args or new in args
 
 
 def add_prefix(name):
     return qtargs._darkmode_prefix() + name
 
 
+smart_image_policy_broken = PYQT_WEBENGINE_VERSION == 0x050f00
+
+
 class TestDarkMode:
 
     pytestmark = utils.qt514
@@ -458,23 +468,32 @@ class TestDarkMode:
                             lambda version, exact=False, compiled=True:
                             new_qt)
 
+        if expected:
+            expected.append((add_prefix('ImagePolicy'), '2'))
+
         assert list(qtargs._darkmode_settings()) == expected
 
     @pytest.mark.parametrize('setting, value, exp_key, exp_val', [
         ('contrast', -0.5,
-         add_prefix('Contrast'), '-0.5'),
+         'Contrast', '-0.5'),
         ('policy.page', 'smart',
-         add_prefix('PagePolicy'), '1'),
-        ('policy.images', 'smart',
-         add_prefix('ImagePolicy'), '2'),
+         'PagePolicy', '1'),
+        pytest.param(
+            'policy.images', 'smart',
+            'ImagePolicy', '2',
+            marks=pytest.mark.skipif(
+                PYQT_WEBENGINE_VERSION == 0x050f00,
+                reason='smart setting is broken with QtWebEngine 5.15.0'
+            )
+        ),
         ('threshold.text', 100,
-         add_prefix('TextBrightnessThreshold'), '100'),
+         'TextBrightnessThreshold', '100'),
         ('threshold.background', 100,
-         add_prefix('BackgroundBrightnessThreshold'), '100'),
+         'BackgroundBrightnessThreshold', '100'),
         ('grayscale.all', True,
-         add_prefix('Grayscale'), 'true'),
+         'Grayscale', 'true'),
         ('grayscale.images', 0.5,
-         add_prefix('ImageGrayscale'), '0.5'),
+         'ImageGrayscale', '0.5'),
     ])
     def test_customization(self, config_stub, monkeypatch,
                            setting, value, exp_key, exp_val):
@@ -484,9 +503,27 @@ class TestDarkMode:
                             lambda version, exact=False, compiled=True:
                             True)
 
-        expected = [(add_prefix('Enabled'), 'true'), (exp_key, exp_val)]
+        expected = []
+        expected.append((add_prefix('Enabled'), 'true'))
+        if exp_key != 'ImagePolicy':
+            expected.append((add_prefix('ImagePolicy'), '2')),
+        expected.append((add_prefix(exp_key), exp_val))
+
         assert list(qtargs._darkmode_settings()) == expected
 
+    def test_broken_smart_images_policy(self, config_stub, monkeypatch, caplog):
+        config_stub.val.colors.webpage.darkmode.enabled = True
+        config_stub.val.colors.webpage.darkmode.policy.images = 'smart'
+        monkeypatch.setattr(qtargs, 'PYQT_WEBENGINE_VERSION', 0x050f00)
+
+        with caplog.at_level(logging.WARNING):
+            settings = list(qtargs._darkmode_settings())
+
+        assert caplog.messages[-1] == (
+            'Ignoring colors.webpage.darkmode.policy.images = smart because of '
+            'Qt 5.15.0 bug')
+        assert settings == [('darkModeEnabled', 'true')]
+
     def test_new_chromium(self):
         """Fail if we encounter an unknown Chromium version.