The workaround added for #7820 seems to cause datalist dropdowns to lose focus
on Wayland. Let's just disable the old workaround on Qt versions that are not affected
by the original issue, which seems to be Qt 6.6.3+.
Fixes#8831.
libwayland-client.so is development symlink used during linking and there's no need to
have it installed (usually shipped in -devel/-dev packages) on user's machines. Instead
of hardcoding library file name, use same mechanism as in libX11 which let's Python
figure the details and share common logic between X11 and Wayland.
Fixes#8771
https://github.blog/changelog/2025-09-19-github-actions-macos-13-runner-image-is-closing-down/
- CI: Switch to macOS 15 Intel runner
(macOS 14 is still tested with Apple Silicon)
- Nightly: Use macOS 15 Intel runner for nightly releases
(macOS 14 would be better to align with actual Intel releases, but it is
a -large runner, thus possibly metered)
- Releases: Use macOS 14 for Intel releases
This is a -large runner, but releases don't happen often.
The docs show an example for adding domain filtering for configuration options. However the example only matches the root of a domain rather than all pages on a domain which is for example, the default case when using the `tsh` shortcut to disable/enable javascript on a page.
The fix for #8223 in 6f21accfae
was misguided: We don't really care about the statusbar being hidden,
controlled release of keyboard focus needs to happen in any case where
we're hiding the command widget (as that's when we lose keyboard focus).
Fixes#8750.
Equivalent to https://codereview.qt-project.org/c/qt/qtwebengine/+/663568
Specify native platform for ANGLE's EGL backend on Wayland
Set EGL_PLATFORM=wayland to force ANGLE to obtain EGL display connection
for wayland platform. Otherwise, the display connection for
EGL_DEFAULT_DISPLAY may belong to a platform which Nvidia's EGL driver
doesn't support. In case of unsupported platform, EGL may fallback to
Mesa software renderer (LLVMPipe) disabling hardware acceleration in
Chromium.
Fixes#8637
Qt 6.8.2 has a more fine-grained workaround:
https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/606122
This never seems to have it made to Qt 6.9+, but I can't seem to reproduce the
issue anymore (neither with PDF.js nor with Google Sheets), even on older
affected Qt versions, even on older Intel hardware. Maybe something else (mesa
etc.?) changed and this was fixed there?
Let's reenable this and find out if it breaks things again for someone.
Fixes#8346
See #7489, #8001, #8006
This is conceptually similar to the release_focus
signal added in 6aa19eb90f and
6f21accfae, but without having to thread the
signal through to TabbedBrowser and back.
Weirdly, doing self.setFocusPolicy(Qt.FocusPolicy.NoFocus) in
FullscreenNotification.__init__ did not help, so let's just set the focus
manually instead.
Fixes#8174.
Fixes#8625.
From knezi's analysis in #8722:
The problem was that in Qt, slots are called in the order of connection, so
even though there's a code that tries to set up the focus correctly, it's
run after the cmd widget is hidden and hence the focus is already moved and
it doesn't work as expected.
Follow-up for #2236/#8024.
Fixes#8223.
Supersedes and closes#8722.
Also see #8625 and #8174 (which are not fixed by this).
Qt < 6.9 doesn't crash in this situation, and seems to handle nested prompts
differently, causing the test to still wait for a prompt answer.
Follow-up to a13306a79f
For an unknown reason, if a download prompt is triple-nested, Qt segfaults here:
#0 add_lazy_attrs (td=0x7ffff52e0dd1 <QtPrivate::sizedFree(void*, unsigned long)+32>) at sip_core.c:6255
#1 sip_add_all_lazy_attrs (td=0x7ffff52e0dd1 <QtPrivate::sizedFree(void*, unsigned long)+32>) at sip_core.c:6304
#2 0x00007ffff636598e in sip_api_is_py_method_12_8 (gil=gil@entry=0x7fffffff8b8c, pymc=pymc@entry=0x7fffffff8b8b "", sipSelfp=sipSelfp@entry=0x7fffffff8ba8, cname=cname@entry=0x0, mname=mname@entry=0x7ffff636d57b "__dtor__") at sip_core.c:7402
#3 0x00007ffff6365c2c in sip_api_is_py_method_12_8 (mname=0x7ffff636d57b "__dtor__", cname=0x0, sipSelfp=0x7fffffff8ba8, pymc=0x7fffffff8b8b "", gil=0x7fffffff8b8c) at sip_core.c:7356
#4 callPyDtor (self=<optimized out>) at sip_core.c:5375
#5 sip_api_instance_destroyed_ex (sipSelfp=0x7fffffff8c40) at sip_core.c:5311
#6 0x00007ffff5fc9967 in sipQEventLoop::~sipQEventLoop() () from [...]/python3.13/site-packages/PyQt6/QtCore.abi3.so
#7 0x00007ffff0bcd749 in QFileInfoGatherer::getInfo (this=0x5555583f9bc0, fileInfo=...) at [...]/qt5/qtbase/src/gui/itemmodels/qfileinfogatherer.cpp:349
#8 0x00007ffff0be2629 in QFileSystemModelPrivate::fileSystemChanged (this=0x5555583f9870, path="/tmp/qbdl/download", updates=QList<std::pair<QString, QFileInfo>> (size = 3) = {...}) at [...]/qt5/qtbase/src/gui/itemmodels/qfilesystemmodel.cpp:1966
(full stacktrace has 183 frames)
After a lot of experimentation, I figured out that moving the `NullIconProvider`
to a class variable (or removing it entirely) seems to help. Note that making it
`global` (but keeping the instanciation inside `FilenamePrompt._init_fileview()`)
did not!
This is semi-blind fix: It's unclear to me if this is a proper fix, or if the
changed memory layout just causes the issue to temporary disappear.
However, given that `QFileInfoGatherer::getInfo()` calls into the icon provider,
it might very well be the real deal.
Closes#8674
window-latest switched to windows-2025, where GitHub doesn't preinstall NSIS:
https://github.com/actions/runner-images/issues/12677
Let's install it manually (untested, might need follow-up commits).
The release.yml workflow uses windows-2019 (and will switch to windows-2022 in a
follow-up commit), so it is unaffected for now.
With Python 3.14, since argparse enabled coloring by default:
17c5959aa3
running test_cmdutils.py failed with a lengthy traceback finishing in:
```
INTERNALERROR> pluggy.PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
INTERNALERROR> Plugin: /home/florian/proj/qutebrowser/git/tests/conftest.py, Hook: pytest_runtest_makereport
INTERNALERROR> PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
INTERNALERROR> Plugin: rerunfailures, Hook: pytest_runtest_makereport
INTERNALERROR> PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
INTERNALERROR> Plugin: hypothesispytest, Hook: pytest_runtest_makereport
INTERNALERROR> PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
INTERNALERROR> Plugin: pytest-bdd, Hook: pytest_runtest_makereport
INTERNALERROR> AttributeError: 'types.SimpleNamespace' object has no attribute 'verbose'
```
with one part pointing to Python internals:
```pytb
INTERNALERROR> File "/home/florian/proj/qutebrowser/git/.tox/py314-pyqt68/lib/python3.14/site-packages/_pytest/_code/code.py", line 669, in exconly
INTERNALERROR> lines = format_exception_only(self.type, self.value)
INTERNALERROR> File "/usr/lib/python3.14/traceback.py", line 180, in format_exception_only
INTERNALERROR> te = TracebackException(type(value), value, None, compact=True)
INTERNALERROR> File "/usr/lib/python3.14/traceback.py", line 1106, in __init__
INTERNALERROR> suggestion = _compute_suggestion_error(exc_value, exc_traceback, wrong_name)
INTERNALERROR> File "/usr/lib/python3.14/traceback.py", line 1653, in _compute_suggestion_error
INTERNALERROR> import _suggestions
INTERNALERROR> File "<frozen importlib._bootstrap>", line 1371, in _find_and_load
INTERNALERROR> File "<frozen importlib._bootstrap>", line 1342, in _find_and_load_unlocked
INTERNALERROR> File "<frozen importlib._bootstrap>", line 951, in _load_unlocked
INTERNALERROR> File "<frozen importlib._bootstrap>", line 496, in _verbose_message
INTERNALERROR> AttributeError: 'types.SimpleNamespace' object has no attribute 'verbose'
```
This happens when Python tries to access sys.flags.verbose, but we replaced it
with a fake object that does not have a .verbose attribute anymore (only
temporarily, but for some reason that's enough to trigger the issue anyways).
Since we can't mutate sys.flags, instead we create an object that copies all
sys.flags attributes, and then use that as a nicer replacement.
See #8529
This keeps our setup.py around for now, while still supporting a PEP-517
compliant build. It's the minimum required change to make modern pyroma stop
complaining, and hopefully to avoid deprecation warnings.
Partially duplicates #8560
See #3526
Unfortunately there is no way to get this information from Qt, so I had to
resort to some funny low-level C-like Python programming to directly use
libwayland-client and Xlib. Fun was had! Hopefully this avoids having to ask
for this information every time someone shows a bug/crash report, as there
are various subtleties that can be specific to the Wayland compositor in use.
Packages are slowly migrating to not having a __version__ attribute anymore,
instead relying on importlib.metadata to query the installed version.
jinja2 now shows a deprecation warning when accessing the __version__
attribute: https://github.com/pallets/jinja/pull/2098
For now we keep accessing __version__ for other packages (we still need the
logic for PyQt and its special version attributes anyways), but we fall back on
importlib.metadata.version if we can't get a version that way, and we stop
trying __version__ for jinja2.
With windows-2022 and windows-2025 on GitHub Actions,
we get:
qutescheme:data_for_url:128 url: qute://pdfjs/web/viewer.mjs, path: /web/viewer.mjs, host pdfjs
webenginequtescheme:requestStarted:105 Returning text/plain data
which causes:
JS: [qute://pdfjs/build/pdf.mjs:0] Failed to load module script: Expected a
JavaScript module script but the server responded with a MIME type of
"text/plain". Strict MIME type checking is enforced for module scripts per
HTML spec.
It's unclear why we get text/plain back there in the first place (can't
reproduce on a local Windows 11 install), but it's definitely wrong, so let's
just override that problematic case.
It's changed in this PR: https://github.com/mozilla/pdf.js/pull/19956
to have the version string as a comment in the file, instead of the
variable.
Making the regex more forgiving increases the chance of matching on the
wrong string on a past or future version. I've only tested with the
previous version (v5.2.133) and it seemed to still work there.
Changes I made to the regex:
* add the literal * to the match group of possible prefixes of pdfjsVersion
* make the quotes around the version optional
* make the semicolon at the end of the line optional
* add newline to the list of characters that can terminate the match
group (otherwise it was matching across the end of the line up to the
first string, kinda odd when there was a $ after the match group)
We only have to cast these variables on Qt5. If we cast them on Qt6 mypy
warns of a redundant cast. If we ignore that warning mypy complains of a
redundant ignore on Qt5.
Add a conditional on `machinery.IS_QT5` and only cast it required.
But also move that conditional out to a utility function to avoid
duplicating code in the function doing the actual logic.
I'm re-using the `_T` generic TypeVar defined above. Not sure if it's
proper to re-use them or not. Doesn't python have a better way of
defining generics now?
I added the `valid-type` ignore because mypy was complaining:
Variable "to_type" is not valid as a type
I'm not sure if there is some way to do it better, but `reveal_type()`
says that the call site is getting the right type back in the end.
With PyQtWebEngine-Qt5 5.15.17 (Qt 5.15.19), we seem to run into similar issues
like we already did with Qt 6.5:
https://github.com/qutebrowser/qutebrowser/issues/7624#issuecomment-14740084709cb54b2099
However, skipping the ELF test is not enough, as we also get warnings
at runtime (as we don't have any API to get the version at runtime).
We don't care much about Qt 5 at this stage, so let's just not output
warnings in that case (and nothing in the code should care about the exact
QtWebEngine patch level beyond 5.15.2 anyways).
For most user-facing things, we *can* get the exact version number from
the user-agent, so this should not actually affect much.
This sometimes fails with 'The data stream has read past the end of the data in
the underlying device', which turns up in crash reports around once a month.
This is somewhat similar to https://bugreports.qt.io/browse/QTBUG-117489 - but
without knowing the data and without being able to reproduce, it's unclear what
the culprit could be.
It's broken in weird ways since recently (`:version` not loading,
segfault in test_version.py). Since nobody should be using it anyways,
there is no point in spending time on debugging a tricky issue.
Next step is probably ripping it out completely, but that's a separate
can of worms.
See #4039
We create the DownloadManager with parent=qapp, which means they will stick
around forever after the test finished.
While we disconnect the QWebEngineProfile::downloadRequested() signal,
we keep the DownloadManager around, which also keeps around its download-update
timers.
Those will then result in tests/unit/utils/usertypes/test_timer.py::test_early_timeout_check
being flaky, as their _validity_check_handler slot keeps getting called in the
background. Due to the globally mocked time.monotonic(), this results in
nonsensical error messages such as:
Got logging message on logger misc with level WARNING:
Timer download-update (id ...) triggered too early:
interval 500 but only -1023.269s passed!
After this change, we now clean up those objects properly, thus fixing the
flakiness.
See #5390.
To fix a flaky tests/unit/utils/usertypes/test_timer.py::test_early_timeout_check
(where a download-update timer from an earlier test fails), I looked into what
usertypes.Timer instances where left over after a test finished.
It turns out that there were about 190 still existing "partial-match" and
"normal-inhibited" timers when breaking in test_timer.py, even when just running
tests in tests/unit/browser/ and tests/unit/utils/usertypes.
This is because we pass qapp as parent to the ModeManager we create, but that
means it will be forever alive if we don't take care of cleaning it up after a
test.
Perhaps our tests should have some sort of mechanism that checks whether there
are any "leftovers" after a test has finished (perhaps even as part of
pytest-qt?), but for now, let's just fix the issues we can directly see.
Skip this hypothesis version pending https://github.com/HypothesisWorks/hypothesis/issues/4375
Our test suite is currently failing due to running python with `-b` and
being configured to fail on warnings.
We'll pick it up on the next update run or so.
The previous fix in 3dc212a815 was insufficient,
as the inner `getattr(extract_result, "registered_domain")` was always evaluated
first (thus triggering the deprecation warning again).
We also cannot do:
getattr(extract_result, "top_domain_under_public_suffix", None) or extract_result.registered_domain
as `""` is a valid value for it.
Speculative fix for test_early_timeout_handler in
tests/unit/utils/usertypes/test_timer.py failing:
> assert len(caplog.messages) == 1
E AssertionError: assert 5 == 1
due to:
------------------------------ Captured log call -------------------------------
WARNING misc:usertypes.py:467 Timer download-update (id 620757000) triggered too early: interval 500 but only -609.805s passed
WARNING misc:usertypes.py:467 Timer download-update (id 922746881) triggered too early: interval 500 but only -609.429s passed
WARNING misc:usertypes.py:467 Timer download-update (id 1056964613) triggered too early: interval 500 but only -609.537s passed
WARNING misc:usertypes.py:467 Timer download-update (id 1912602631) triggered too early: interval 500 but only -609.671s passed
WARNING misc:usertypes.py:467 Timer t (id -1) triggered too early: interval 3 but only 0.001s passed
We sometimes tried to use hints before the page was fully rendered (?), thus
causing no elements to be found.
It also doesn't make much sense to test leaving insert mode if we aren't in
insert mode yet, so make sure we entered it first.
See #5390
My reproducer is this:
* open the browser with the auto insert mode JS primed, and two tabs:
`python3 -m qutebrowser -T -s input.insert_mode.auto_load true about:blank?1 about:blank?2`
* close the second tab: `d`
* re-open the closed tab then close it again real quick: `u` then `d`
If you have trouble reproducing, try increasing the 65ms delay in
`handle_auto_insert_mode` to be bigger (like 500ms).
Closes: #3895
This can be used to easily test a different PDF.js version manually (which is
installed via update_3rdparty.py), without having to uninstall the system-wide
one first.
Not yet quite sure what exactly is the culprit, but this seems to help for all
tests (!) to pass with Xvfb locally.
For now only scoped to Qt 6.9.0. Will probably already need to reevaluate with
the RC, but definitely with the final release.
See #8444
The DocumentPictureInPicture JS API added in Chromium 116 is not implemented in
QtWebEngine. This results in createWindow() being called with a window type with
random value, which then causes qutebrowser to bail out:
Traceback (most recent call last):
File ".../qutebrowser/browser/webengine/webview.py", line 123, in createWindow
raise ValueError("Invalid wintype {}".format(debug_type))
ValueError: Invalid wintype 843995690
Until this is fixed in Qt, we pass an argument to Chromium to disable the API
entirely, so that web pages hopefully fall back to something else.
In the case of the new Google Huddle feature, this results in them still working
with an on-page overlay instead.
Thanks to Joshua Cold and Vivia for helping to debug this!
Fixes#8449
See https://bugreports.qt.io/browse/QTBUG-132681
Ubuntu 20.04 will be EOL in April 2025, and PyQt 6.8 does not support being
installed on it anymore:
https://pyqt-builder.readthedocs.io/en/stable/releases.html
Other than for the oldest Qt 5 / Qt 6 envs, and for utility envs, let's use
Ubuntu 22.04 or 24.04.
The normal PDF.js build only officially supports the latest Chromium, so things
might break every once in a while with QtWebEngine (e.g. #8199, #7335).
Let's instead bundle and recommend the legacy build.
Closes#8332Closes#7721 (reworded)
Also see #7135
The test added in fbd148f983 was flaky because
reloading didn't wait for the page load to actually finish fully, and thus the
element not being found on the page.
Fix this by providing the page path to the step, and making sure it waits for it
to be fully loaded again.
Also undos some flaky tags done in 2018:
12e5375931c1c182d958
As this might have been the real culprit.
If not, those should be re-added.
See #8348, #5390
Turns out there are various places in the tests that somehow cause Qt to start
caching the dict location, not just actually accessing
.[is|set]SpellCheckEnabled() like I originally assumed.
Thus, move setting QTWEBENGINE_DICTIONARIES_PATH into conftest.py so it's run
before any tests. However, remove the sanity check after, as that requires
initializing a QWebEngineProfile which we might not always want to do when
running a subset of unit tests. If something goes wrong with this, chances are
we'll only notice later in the tests anyways.
Follow-up to db8e508530
See #8330
- Newest Linux/macOS/Windows environments (should be roughly same as release,
especially for Windows/macOS)
- Nightly binary builds
- Release automation
Closes#8205
Starting with Qt 6.8, Qt enforces that the spell checking dictionary path exists
when enabling spell checking (but it doesn't check whether there is actually
anything in there). This caused our test ensuring that spell checking gets
enabled properly to fail, as we never actually set a proper directory.
We now do, though we need to do so for the entire test session, as QtWebEngine
caches the directory.
Reverts 7475d38
See https://github.com/qutebrowser/qutebrowser/issues/8242#issuecomment-2333609589Fixes#8330
Due to a Qt bug, we get a misleading error even when trying to *disable*
spell checking: https://bugreports.qt.io/browse/QTBUG-131969
To avoid that from happening, we now only call setSpellCheckEnabled() if
the value actually changed.
See #8242, #8330 (not the same issue but related)
Similarly to #5998, XHR requests should be able to set their custom
Accept-Language values - and for some odd reason, stuff breaks on websites
sometimes when that's not respected.
There's no way for qutebrowser to know if a given header value is already set in
a request (i.e. whether we're adding or overriding). Thus we only really have
two options here:
1) Don't set any shared.custom_headers() for XHR requests at all.
2) Special-case Accept-Language here, because the issue usually is triggered by
the global override, but that already gets set just fine via QWebEngineProfile
anyways.
Given that 2) is the thing causing trouble in the wild and it's unclear what the
desired behavior for 1) is (e.g. for the DNT header), let's go for 2) here.
Was getting an import error when trying to import the webengine version.
Not going to skip the tests using this step since they seemed to be
working otherwise, probably this behavior isn't needed on webkit anyway.
A string comparison of version numbers relies on nice simple version
numbers with approximately the same amount of digits. Who knows what
various systems are running!
Switch to the integer format version number. It's harder to grep for
when dropping a Qt version, but hopefully we have enough "6.8"s around
it to compensate.
review feedback
Hopefully this is an okay header format for checkers to pick up, it's
the same as in qutebrowser/html/version.html except with the doctype
declaration above.
review feedback
I've mixed opinions on this. I'm not convinced that ternary expressions
are more readable than an if/else block.
Also if someone passes a string into this function it'll return
"access-paste" now.
Combine the `if exists` and `unlink` in one step and avoid any race
conditions with the file disappearing in between them.
Co-Authored-By: Florian Bruhin <me@the-compiler.org>
Previously it would have crashed with an AttributeError if a user had a
PyQt of 6.8 but Qt of 6.7.
Switch to catching any AttributeError which will hopefully cover
either or both of Qt and PyQt not being a new enough version, even if
it's a bit less of an explicit check.
This branch gets entered if the value of a setting linked to a feature
is anything other than True, False or "ask". I think this could only
happen due to a programming error, for example when you add an entry to
`_WebEnginePermissions` that was linked to a String type setting. So I
think putting an assert here instead of a warning should be fine and
more explicit. (Or should be be `utils.Unreachable`? Or `ValueError`?)
Relying on `hasattr()` made me feel a bit guilty. So I've moved these
conditionals to be backed by a class. The only class based alternative I
can think of is putting it on the base type and leaving all the other
config variables to raise errors. But that doesn't tell the type system
anything.
All the promptable feature permissions so far have been of type BoolAsk.
The prompt uses a "yesno" mode prompt and only results in a bool. The
persistence logic only supports bools.
Previously I made the shared prompt support the String type clipboard
permission setting by treating non "ask" values as False (you only get
prompted if the global setting is "none" anyway), but saving the prompt
results with `:prompt-accept --save` didn't work because the persistence
code only supported bools.
What we want to do when saving is convert `False` to "none" and `True`
to "access-paste". This mirrors the new webengine logic. It does mean we
can't let users choose to persist either none/access/access-paste, but
webengine doesn't prompt us if the page is already allowed "access" but
is trying to paste anyway. If it did we would have to use a non-yesno
prompt for this (perhaps it could be driven directly by the ConfigType).
For now I've added a new concept to the ConfigTypes to allow them to be
casted to and from bools, so that we can plumb this String type from the
boolean yesno prompt.
TODO:
* try to make it an interface so we don't have to use `hasattr` (even if
it means multiple inheritance)
* add test coverage to test_configtypes.py
The test page is using a JS API that is too new for qtwebkit:
23:07:54.898 DEBUG js shared:javascript_log_message:190 [http://localhost:37635/data/prompt/clipboard.html:97] TypeError: undefined is not an object (evaluating 'navigator.clipboard.readText')
This is failing with:
ERROR tests/end2end/test_invocations.py::test_permission_prompt_across_restart - PermissionError: [WinError 5] Access is denied: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpytep6gj0\\cache\\webengine\\Cache\\Cache_Data\\data_0'
In pytest teardown, while trying to clean up a temp dir, probably the
basedir. The test above waits for shutdown at the end of the test, maybe
that's what's needed here.
Otherwise maybe just `@pytest.mark.skipif(utils.is_windows)` 🤷
For tests that do the same prompt in the same session, they are failing
on Qt6.8 and PyQWebEnginet6.7 because we can't disable the new WebEngine
behaviour.
We can work around this by getting a fresh instance for each test, but I
don't want to make more tests slower if I don't have to. So move that
logic into a custom "fresh instance" prompt that will only create one
when needed.
WebEngine now supports prompting for permission when a web page tries to
access the clipboard. Previously we only supported fixed permissions that
applied globally.
This commit
1. adds support for permission requests with the new ClipboardReadWrite
permission
2. tweaks the generic JS prompt handler in browser/shared.py to handle
seeing a setting which isn't of type BoolAsk
3. adds end2end tests around clipboard permissions, both the existing
global ones and the new per-URL ones
1. the ClipboardReadWrite permission
I added this as an int because the relevant PyQt isn't out yet and users
with 6.8 running with PyQt6.7 are already seeing this.
I added an "ask" value to the existing String type
`content.javascript.clipboard` setting and set the
default/global/fallback permissions to False if ask is set globally.
Hmm, maybe we should change the default actually... I'll have to check
what the other prompt supporting settings default to.
2. tweaked prompt handler
This was treating the string values that weren't "ask" (like "none" and
"access") as truthy and allowing the action. I've changed the bool
checks to be exact checks to add a warning if this happens again.
Then I added an exception to the warning logging for known cases like
this. I did try looking at adding a new setting type. Something that was
descended from String but had an `__eq__` method that understood bools
and would treat `access-paste` as True and everything else as False. But
that didn't work out because it looks like config values are stored as
python values and the config classes are just static and don't actually
hold values. Oh well, maybe a better pattern will emerge with time.
3. tests
Apparently there were no tests around the clipboard settings. Perhaps
not a coincidence given how confusing they are (what does access-paste
mean?).
I copied a test file from some random test site, tweaked it a little bit
and got to work. For the paste test it's a bit awkward because I don't
know if we have a way to fill the clipboard in a nice way for the tests.
So I'm just matching on "Text pasted: *", which is usually an empty
string.
The prompt tests require running against Qt6.8 to get the new prompt
behaviour (don't need pyqt68 though).
There are some changes in this area in Qt6.8, so it would be good to
have some test coverage.
The "access permission - copy" one is broken in 6.8. Still need to raise
that upstream.
QtWebEngine has a new feature where it will remember what permissions
you have granted or denied. It has three options regarding permissions
storage:
AskEveryTime -- don't store
StoreInMemory -- store in memory only
StoreOnDisk -- store in memory and on disk
By default it does the StoreOnDisk behavior. Having webengine remember
whether you granted or denied a permission would make the qutebrowser UX
around that area inconsistent. For example the default y/n actions for a
permission prompt in qutebrowser will only accept that single
permission request, and you'll be re-prompted if you reload the page.
Users may be used to this and if webengine started remembering
permission grants the users may be surprised to find that the page was
accessing features without prompting them for permission anymore.
Additionally we already have our own permission storage machinery in
autoconfig.yml.
This commit will set the webengine feature to AskEveryTime, which disables
any storing of permission grants.
Also adjusts the skip marker of the affected tests so they'll be enabled
again on Qt versions with the appropriate permissions API.
This re-enables the pylint too-many-positional-arguments for the main
application code. It's still disabled for tests because that's how you pull in
pytlint fixtures, and I don't think we want to push people into being creative
with fixtures just to get around that.
When functions are called with many positional arguments the reader has to do
a bit of heavy lifting to figure out in what position a value is being passed,
and it's easier to make mistakes. So I would like to encourage using keyword
arguments for long argument lists.
I've set the `max-positional-arguments` to a completely arbitrary 7, from a
completely arbitrary 5, because there were many more violations under 7. If
like 99% of our functions fit under 7 it's probably fine.
Regarding the exceptions:
* objreg.register: I grepped it and it looks like everything is only passing
the first two args as positional already, lucky!
* `_get_color_percentage`: only one usage of it, but I was in "add directive
comment" mode
* update_3rdparty.py: only one usage, already using kwargs
* pyqtProperty: idk
* commands.py: "its complicated". Many methods in this file map to commands
used in qutebrowser's command mode. In that case it's usual for them to be
called as flags, rather than positional. But it could be complicated to wade
into that, and having one file excluded isn't so bad.
With the upgrade to MarkupSafe 3.0, something funny happened when trying to pass
the GUIProcess object to jinja after launching a userscript:
[...]
File "[...]/qutebrowser/browser/qutescheme.py", line 291, in qute_process
src = jinja.render('process.html', title=f'Process {pid}', proc=proc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "[...]/qutebrowser/utils/jinja.py", line 123, in render
return environment.get_template(template).render(**kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
File "html/process.html", line 11, in block 'content'
File "[...]/lib/python3.11/site-packages/markupsafe/__init__.py", line 42, in escape
if hasattr(s, "__html__"):
^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: wrapped C/C++ object of type GUIProcess has been deleted
This can be reproduced with:
qutebrowser --temp-basedir ':cmd-later 0 spawn -u -o /bin/echo test'
We pass the `GUIProcess` to the Jinja template as `proc`, which then formats it as
`{{ proc }}`` (to stringify it). For some reason, with the newest MarkupSafe/Jinja
versions, this now triggers the `if hasattr(s, "__html__")` check in MarkupSafe
(which has been around for a while). That then presumably causes PyQt to try and
access the underlying C++ object for `GUIProcess``, but that has already been
deleted.
But why is it deleted in the first place, if we keep track of even completed
processes data ever since we added `:process` in a3adba81c? It looks like the Qt
parent-child relationship is the culprit here: When we pass a parent to the
`GUIProcess`` from the userscript runner, it will get deleted as soon as said
runner is cleaned up (which happens after the userscript has finished).
We probably never noticed this before because we only accessed data from the
Python wrapper and not from the C++ side, but it still seems like a good idea
to avoid passing a parent for a long-lived object (with time-based cleanup) in
the first place.
Added in 3.3.0:
https://pylint.pycqa.org/en/latest/whatsnew/3/3.3/index.html
Some of those arguments could probably indeed be keyword-only,
but for some of the functions shown by pylint, those are qutebrowser command
handlers where a positional argument has different semantics.
Did run with ruff pretending to use Python 3.10,
because otherwise it won't reformat those:
ruff check --select 'UP035' --fix --config 'target-version = "py310"' --unsafe-fixes
This is because collections.abc.Callable inside Optional[...] and Union[...] is
broken with Python 3.9.0 and 3.9.1:
https://github.com/asottile/pyupgrade/issues/677https://github.com/astral-sh/ruff/issues/2690https://github.com/python/cpython/issues/87131
However, pylint can detect problematic usages (of which we only have one),
so we might as well use the new thing everywhere possible for consistency.
Also see #7098
To avoid WebEngine remembering granted permissions across restarts,
remove their persistence file when we start up.
This is only technically required when Qt=>6.8 and PyQt<6.8. But we only
take action if the file exists anyway, so it's safe enough to run all
the time and that means less conditional code to test ;)
There are a few options for where we could do this cleanup, I'm choosing
to do it at the latest point possible, which is right before we set
`setPersistentStoragePath()`, since the permissions manager is
re-initialized after that, see https://bugreports.qt.io/browse/QTBUG-126595
TODO:
* call the new setPersistentPermissionsPolicy API when PyQt>=6.8
Qt 6.8 has its own permission grant persistence features. This means
that if you accept a permission prompt in qutebrowser, and don't save
it, it will be remembered by webengine anyway and you won't be
re-prompted again.
This test demonstrates that behaviour by temporarily granting a
permission, restarting the browser in the same basedir, then seeing if
we get prompted for the permission again or not. If not it fails on the
"Asking question" line.
We can't do much about re-prompting for a permission in the same browser
instance (Qt saves the permission grant in memory too) but we can clean
up the persisted permission files on browser starts so it doesn't
remember it forever. At that point the skip mark can be removed from
this test.
I just searched for qt5 and deleted stuff. EZ.
Will leave on a branch for a bit and see if I feel like testing this at
all, otherwise maybe leave this stuff in here and make it not called.
Not 100% sure that we need to remove all this stuff when we just want
the CI to go green. But tbh if we don't need to make Qt5 releases then
we don't need it. Better to be bold and pull it out than have to work
around it in the future. And we can always revert the commit.
They require maintenance, but we don't have a great need for Qt5 builds.
See https://github.com/qutebrowser/qutebrowser/issues/8260
All the Qt5 switches are still in tox, the build release script and the
nsis installer.
This flag is vital for the allow-list configuration to be picked up.
It should be set globally and `dev-qt/qtwebengine` should be
recompiled after it's enabled.
Ref #8313.
See 433074c681, this is the same cause. An older version of a
package being included in requirements files because setuptools injects
its vendored packages into sys.path and we use pip freeze to build lock
files. Then when you install two requirements files at the same time they
end up having conflicting versions. This at least means we include the
latest version, which will do until we move to a method of generating
lock files that just works off of the raw requirements file.
Qt have updated their permission storage feature so it respects our
the setting our basedir feature uses, so now all the tests that use
"Given I have a fresh instance" are passing.
The remaining failing ones do pass if I make them run in a fresh
instance, but I am leaving them as xfail because a) opening a new
instance is slow b) the new upstream behaviour causes a regression in
the qutebrowser behavior (you don't get re-prompted where you would have
been previously) so I feel like it is correct for some tests to be
failing! We have to set AskEveryTime at some point and we can address
them then.
The message is:
The following paths were searched for Qt WebEngine dictionaries:
/tmp/qutebrowser-basedir-qrhbqblr/data/qtwebengine_dictionaries
but could not find it.
Spellchecking can not be enabled.
Tests are failing on "Logged unexpected errors".
Currently the dependency update job is failing[1] because one of the
tests installs multiple requirements files before running the tests and
it claims they have conflicting versions of `importlib_resources` (6.4.0
vs 6.4.4). 6.4.0 is in the pinned files and there is a 6.4.4 available.
Looking though the logs the first time I see importlib_resources==6.4.0
is when printing the requirements for the `test` requirements file.
But it's not mentioned at all when installing that file. Which makes me
think it found it's way into the virtualenv by some other means.
Looking at git blame for the test requirements lock file, it looks like
importlib_resources was introduced in
https://github.com/qutebrowser/qutebrowser/pull/8269 and indeed I can
see version 6.4.0 in setuptools vendored folder[2].
So it looks like this is another issue caused by setuptools adding their
vendored packages into sys.path.
Options I can see for resolving this:
a. add importlib_resources as a dependency in requirements.txt-raw so
that we always pull down the newest one, even though we don't need it
b. add an @ignore line for importlib_resources
* I think in the unlikely event we end up needing it then it being
ignored might be hard to spot
c. drop python 3.8 support
d. switch to a requirements compilation method that doesn't use `pip
freeze`
I've chosen (a) here because I think it's less surprising than (b), less
work than (c) and I already have a PR up for (d). And it's only pulled
down for 3.8 anyhow, so we'll drop this workaround when we drop that.
[1]: https://github.com/qutebrowser/qutebrowser/actions/runs/10660624684/job/29544897516
[2]: https://github.com/pypa/setuptools/tree/main/setuptools/_vendor
This bit is printed right about the test result summary, so now that the
file names are just test names, printing them out just above the full
test paths in the results seems a bit redundant.
The section header prints out the file path with the screenshots and
that's the important part. It looks fine to me printing a section header
without any section contents. Example:
-------------------- End2end screenshots available in: /tmp/pytest-of-user/pytest-108/pytest-screenshots ---------------------
=================================================== short test summary info ====================================================
FAILED tests/end2end/features/test_completion_bdd.py::test_deleting_an_ornpen_tab_via_the_completion - AssertionError: assert 'http://local...ata/hello.txt' == 'http://local...ata/sello.txt'
FAILED tests/unit/utils/test_resources.py::TestReadFile::test_glob_deleting_resources_subdir[True-pathlib] - AssertionError: assert ['html/subdir...ir-file.html'] == ['html/subdir...ir-sile.html']
FAILED tests/unit/utils/test_resources.py::TestReadFile::test_glob_deleting_resources_subdir[False-zipfile] - AssertionError: assert ['html/subdir...ir-file.html'] == ['html/subdir...ir-sile.html']
FAILED tests/unit/utils/test_resources.py::TestReadFile::test_glob_deleting_resources_subdir[True-zipfile] - AssertionError: assert ['html/subdir...ir-file.html'] == ['html/subdir...ir-sile.html']
FAILED tests/end2end/features/test_utilcmds_bdd.py::test_cmdrepeatlast_with_modeswitching_command_deleting - AssertionError: assert 'http://local...ata/hello.txt' == 'http://local...ata/sello.txt'
FAILED tests/unit/utils/test_resources.py::TestReadFile::test_glob_deleting_resources_subdir[False-pathlib] - AssertionError: assert ['html/subdir...ir-file.html'] == ['html/subdir...ir-sile.html']
=========================================== 6 failed, 23 passed, 8 skipped in 22.59s ===========================================
Hopefully now that we have reporting in the test results, and pytest
retaining of old directories, we don't have to encode so much
information in the filenames to help you make sense of them.
Previously the png filenames looked like this:
2024-08-24T12_42_11.160556-tests_end2end_features_test_completion_bdd.py__test_deleting_an_open_tab_via_the_completion.png
Now they just have the individual test name, eg:
test_deleting_an_open_tab_via_the_completion.png
The two times people will want to look at these files and I want to make
sure they can find what they are looking for are:
* running the tests locally
* the directory with the images is printed out right above the
pytest summary, hopefully that is a clear enough reference to the
tests and that has the full path to the tests, not just the name
* if people run multiple test runs and want to find older images
they will have to know, or guess, how the pytest temp dir naming
scheme works, or go back in their terminal scrollback
* when downloading images as artifacts to debug tests
* The zip files with images from a job currently have names like
end2end-screenshots-2024-08-18-fef13d4-py310-pyqt65-ubuntu-22.04.zip
* Hopefully that zip file name is specific enough
* I'm not sure if the individual filenames with just test name in
them are specific enough for this case. But presumably people will
be looking at the run logs in CI anywhere and will be able to
match up a failing test with the screenshot easy enough
Pytest appears to sanitize test names enough for upload-artifact.
Couldn't see any docs on it, but I put all the characters it complains
about in a BDD test name and they all go stripped out.
The upload artifact action can't collect artifacts from /tmp/ in the test
runners. So now that we are writing the screenshots that we want to collect
later to the pytest `tmp_path` location we need to make sure that lives
somewhere the later actions can find it.
Pytest uses `tempfile.gettempdir()` to find the temp dir, and that respects a
number of environment variables including `TMPDIR`. This commits sets TMPDIR
to RUNNER_TEMP in in our test runners to make pytest uses the temp dir that's
mounted into the action containers.
For the docker based runners I can use the `env` map, but for the ubuntu ones
it didn't let me expand `${{ runner.temp }}` in the end map under `step`, so
I'm writing it to the env file for the runner instead. It failed to parse the
action yaml and said:
> Unrecognized named-value: 'runner'. Located at position 1 within expression: runner.temp
For the user part of the `pytest-of-$user` directory, I looked at the new
screenshot related test summary lines to see what the user was called.
`runner` on the ubuntu containers and `user` in our docker containers. Pytest
maintains the "pytest-current" symlink to the latest temp folder.
On GitHub the RUNNER_TEMP dir is inside the user's home directory. I
think the spirit of the check is making sure you aren't touching stuff
like ~/.config/qutebrowser/ in the tests, if it's within a specified
tempdir it should be fine
I would like it to be obvious to contributors who run the tests locally
that there are screenshots of the processes under test that they can
examine. I don't think it's obvious that there could be useful files
sitting round in a temp directory.
This commit adds the screenshot file paths to a user property on failed
tests then adds a custom report section that pulls that lists those
properties. That way when there is errors users will get the paths to
the images printed out alongside the report of failed tests.
I find it difficult to navigate the internals of pytest. I tried various
ways of printing information and getting that information to methods
that could do the printing but couldn't get anything to work. I ended up
entirely copying this SO post which worked really well for attaching
information to test results in a place that is accessable to the
reporting hook: https://stackoverflow.com/a/64822668
It's added to the end of the existing terminal report hook, because
while it seems you can have two of those hooks, things can get pretty
confusing with interleaved reports and not all of them running every
time.
--------------------- End2end screenshots available in: /tmp/pytest-of-user/pytest-56/pytest-screenshots ---------------------
2024-08-17T14_49_35.896940-tests_end2end_features_test_utilcmds_bdd.py__test_cmdrepeatlast_with_modeswitching_command_deleting.png
2024-08-17T14_49_37.391229-tests_end2end_features_test_completion_bdd.py__test_deleting_an_open_tab_via_the_completion.png
=================================================== short test summary info ====================================================
FAILED tests/end2end/features/test_utilcmds_bdd.py::test_cmdrepeatlast_with_modeswitching_command_deleting - AssertionError: assert 'http://local...ata/hello.txt' == 'http://local...ata/sello.txt'
FAILED tests/end2end/features/test_completion_bdd.py::test_deleting_an_open_tab_via_the_completion - AssertionError: assert 'http://local...ata/hello.txt' == 'http://local...ata/sello.txt'
====================================================== 2 failed in 5.18s =======================================================
From adding debug messages I can see:
RUNNER_TEMP=/home/runner/work/_temp
/tmp/pytest-of-runner/pytest-0/pytest-screenshots
Means that I don't think GHA will be able to collect the temp files
because they are not being written to the temp dir being mounted in from
outside. I think that's the case anyway. Might have to pass
--basetemp=$RUNNER_TEMP to pytest or set TEMP or something. TODO
Saving screenshots to the temp directories managed by pytest means we
don't have to worry about cleaning up from previous runs because pytest
will create a new folder for each run. Now that we aren't cleaning stuff
up means we don't have to worry about workers clobbering each other
because all they are going to do is write to files with the names of
tests which have already been distributed amongst them.
Moving to the pytest temp dirs instead of a hardcoded one also means
that it'll be less obvious to users where the screenshots are. Pytest
doesn't seem to have much UX around pointing people to interesting
artifacts in the "temp" dir. So I'll have another look at adding this
information to the test report.
Since this implementation is now more tightly couple with pytest I've
pulled some code out of the QuteProc process into fixtures.
TODO:
* add screenshot locations to test report
* adapt GHA zip file creation to get files from /tmp/pytest-of-$user/pytest-current/pytest-screenshots
* review filenames to see if pytest does a good enough sanitization for
us, from what I've seen it doesn't slugify that path of the tests, and
it tends to truncate names. I think having the full test path in the
filenames could be useful for people who download the zip file from
the github actions to investigate CI failures
Ohhh! I didn't realize the e2e tests where running in parallel already.
Interesting.
Anyhow, use the builtin `filelock` module to make sure different test
processes in the same session don't re-create the screenshot directory.
This is based on advice here: https://pytest-xdist.readthedocs.io/en/latest/how-to.html#making-session-scoped-fixtures-execute-only-once
I'm saving the lock object to the session stash because it seems the
lock is released when the FileLock object is destroyed. So this ties
it's lifecycle to the test session lifecycle, with xdist hopefully all
the tests processes live for the whole run.
When taking screenshots of the test process running under xvfb it's
offset from the top left corner, the default geometry of qutebrowser is
800x600+50+50. The default size of pytest-xvfb is 800x600, which means
part of the browser window is outside the X11 screen and doesn't get
captured.
This commit duplicates the width and height from the default geometry in
mainwindow.py but sets the x and y offsets to zero so that the browser
window is fully contained within the X11 window.
This commit takes a screenshot of the active browser window when an
end2end test fails. When running on CI a zip file of screenshots will be
attached to the run summary as an artifact. When run locally screenshots
will be left in /$TMPDIR/pytest-screenshots/.
The screenshot is of the Xvfb screen that the tests are running under.
If there are multiple windows open it will likely only show the active
window because a) we aren't running with a window manager b) the Xvfb
display is, by default, the same size as the browser window.
I'm not sure if xvfb is used on the Window runs in CI. We could fall
back to trying to take screenshots if not running under xvfb but I'm a
bit wary of an automatic feature that takes screenshots of people's
desktops when running locally. Even if they just to to /tmp/ it might be
surprising. We can change it later if it turns out we need to try to
take screenshots in more cases.
I'm using pillow ImageGrab, the same as pyvirtualdisplay.smartdisplay. I'm
getting the display number from the pytest-xvfb plugin and formatting it
appropriately (pyvirtualdisplay has an already formatted one which is used by
the smartdisplay, but that's not accessible).
Pillow is now a requirement for running the tests. I thought about making
it gracefully not required but I'm not sure how to inform the user with
a warning from pytest, or if they would even want one. Maybe we could
add a config thing to allow not taking screenshots?
I had to bump the colordepth config for pytest-xvfb otherwise pillow
complained that the default 16bit color depth wasn't supported as it
only supports 24bit, see https://github.com/python-pillow/Pillow/blob/1138ea5370cbda5eb328ec949
8c314d376c81265/src/display.c#L898
I'm saving screenshots to a temp dir because I don't want to put them in
my workdir when running locally. I want to clear the directory for each
run so that you don't get confused by looking at old images. I'm not
100% sure about the lifecycle of the process classes though. Eg if we
have two processes they might both try to create the output directory.
I'm using pytest.session.stash to save the directory so perhaps the
lifecycle of the stash will handle that? Not sure.
Ideally the images would be uploaded somewhere where we could click
through and open them in the browser without having to download a zip
file, but I'm not sure how to achieve that.
It would be nice to print in the test logs that a screenshot was saved
and where to. Just so you could copy paste the filename instead of
having to match the sanitized filename against failing test names. But I
don't know how to log stuff from this stage in the pytest lifecycle.
TODO:
* I'm not sure that the screenshot captures the whole browser window?
Maybe the browser windows is bigger than the X11 display?
Closes: #7625
Starting with PyInstaller 6.10 (6.9?), we're supposed to tell PyInstaller when
we restart our application (and a subprocess should outlive this process).
In their words:
The above requirement was introduced in PyInstaller 6.9, which changed the
way the bootloader treats a process spawned via the same executable as its
parent process. Whereas previously the default assumption was that it is
running a new instance of (the same) program, the new assumption is that the
spawned process is some sort of a worker subprocess that can reuse the
already-unpacked resources. This change was done because the worker-process
scenarios are more common, and more difficult to explicitly accommodate
across various multiprocessing frameworks and other code that spawns worker
processes via sys.executable.
https://pyinstaller.org/en/stable/common-issues-and-pitfalls.html#independent-subprocesshttps://pyinstaller.org/en/stable/CHANGES.html (6.10)
While from a quick test on Windows, things still worked without setting the
variable (possibly because we don't use a onefile build), it still seems
reasonable to do what PyInstaller recommends doing.
Follow-up to #8269.
Few new vendored packages showing up from setuptools for environments
where pkg_resources is being imported for some reasons.
I don't think these requirements should be in our requirements files,
they aren't direct dependancies and they aren't declared as dependancies
of setuptools (and we are currently excluding setuptools from our
requirements files anyway, although apparently that is not the right
thing to do these days). These are actually not installed as normal
packages by are vendored packages shipped with setuptools.
Options I see to deal with them:
1. suck it up and add them to the compiled requirements files
* not ideal, but should be harmless. They are real packages that the
setuptools authors have chose to use
2. exclude these new packages using the markers in comments
* maybe, seems like it could lead to issues in the future if any of
these packages start getting declared as proper dependancies
3. find out where pkg_resources is being imported and stop it
* I don't seem to be able to reproduce this behaviour locally, even
when using a py3.8 docker container. And we are literally only
running `pip freeze` via subprocess, what could the difference be?
* I don't particularly want to delve into the arcane python packaging
stuff, it seems to be layers and layers of very specific issues and
old vendored packages
4. stop using pip freeze to compile requirements files and just compute
them based off of the raw files themselves
* Don't give us the chance to use stuff that we don't depend on but
happens to be installed. We get other nice things with this too
This commit does (1). I'll open a follow up PR to do (4).
Ah! I'm having flashbacks to last year.
1. pyqt5 has plural enum names = define conditional type variable
2. pyqt5 doesn't wrap all the nullable things in Optional = sneakily
make the existing overload function signature conditional.
There might be some other was to solve this, not sure. I know we have
qtutils.add_optional() but in this case it's complaining that the
signature doesn't match the parent. Narrowing or widening the type of
the returned object doesn't affect the function signature. Possibly
we could define our own type variable MaybeOptional...
The methods in `completionmodel.py` dealing with completion categories
were annotated with `QAbstractItemModel`. In mypy's latest 3.11 update
it correctly pointed out that there is code relying on some attributes,
like `name`, being on the categories but `QAbstractItemModel` didn't
implement those attributes.
This commit adds a new base class for completion categories which
defines the extra attributes we expect. It also changes the type hints
to ensure all list categories inherit from it.
There is a couple of downsides to the current implementation:
* It's using multiple inheritance
* the completionmodel code currently expects categories to have all
the methods of `QAbstractItemModel` plus a few other attributes.
Each of the categories inherit from a different Qt model, so we
can't just remove the Qt model from their class definition.
* trying to extract the Qt models to a `widget` class is way too much
work to fit in a dependency update, and I'm not sure it'll be the
right thing to do because the categories are primarily Qt models, so
we would have have to proxy most methods. Perhaps if they added
their extra metadata to a central registry or something
* I tried using a typing.Protocol for BaseCategory but when trying to
make it also inherit from QAbstractItemModel it got grumpy at me
* It doesn't enforce that the attributes are actually set
* it makes mypy happy that they are there, but there is nothing
warning child classes they have forgotten to set them. Mypy does at
least warn about categories that don't inherit from `BaseCategory`
so implementors will hopefully go there an look at it.
* Apparently you can do some stuff with abstract properties, that
might even have type hinting support. But that's a bit much for me
to want to pile in there tonight
At lest the type hints in `completionmodel.py` are more correct now!
New mypy 3.11 update got smarter and raise some issues, they appear to
be correct in all cases.
There are several `type: ignore[unreachable]` comments in conditionals
on `sys.stderr` being None, which were introduce in a comment
specifically to handle a case where `sys.stderr` could be None. So
presumable the ignore comments were just to shut mypy up when it was
making mistakes.
In `debug.py` it was complaining that the class handling branch was
unreachable, because the type hint was overly restrictive. We do indeed
handle both classes and objects.
`log.py` got some extra Optional annotations around a variable that
isn't set if `sys.stderr` is None.
mypy 1.11 has new and improved support for checking partial functions,
and it works great! It says:
qutebrowser/components/misccommands.py: note: In function "_print_preview":
qutebrowser/components/misccommands.py:74: error: Unexpected keyword argument "callback" for "to_printer" of
"AbstractPrinting" [call-arg]
diag.paintRequested.connect(functools.partial(
^
qutebrowser/browser/browsertab.py:269: note: "to_printer" of "AbstractPrinting" defined here
We indeed removed the callback arg in 377749c76f
And running `:print --preview` on webkit crashes with:
TypeError: WebKitPrinting.to_printer() got an unexpected keyword argument 'callback'
With this change print preview works again (on webkit), which I'm a
little surprised by!
mypy 1.11 has stricter checking of the type signature overridden methods: https://github.com/python/mypy/blob/master/CHANGELOG.md#stricter-checks-for-untyped-overrides
There's a couple of places where I added type hints and had to duplicate the
default kwarg value from the parent.
In `completionmodel.py` it was complaining that the type signature of
`parent()` didn't match that of `QAbstractItemModel` and `QObject`. I've
changed it to be happy, and incidently made it so the positional arg is
optional, otherwise it's impossible to call `QObject.parent()`. Options that I
see:
1. support both variant of parent() - what I've done, the technically correct
solution
2. have the two overload definitions but in the actual implementation make the
positional argument required - would mean one overload signature was a lie,
but would make it more clear how to call `CompletionModel.parent()
3. do type: ignore[override] and leave it as it was
In the end I don't expect there to be many callers of
`CompletionModel.parent(child)`.
I also added a few more function type hints to `completionmodel.py` while I
was there. Not all of them though!
In `objreg.py` I expanded the user of `_IndexType` because as
7b9d70203f say, the window register uses int as the key.
With PyQt 6, this gets represented as
QWebEnginePage.RenderProcessTerminationStatus(-1)
which is != -1, thus leading to a KeyError.
Updating to a RendererProcessTerminationStatus
enum value works fine on both PyQt5 and PyQt6.
In the Qt6.8.0-beta2 release for some reason the error message now looks like;
Failed to style frame: Failed to read a named property '_qutebrowser' from 'Window': Blocked a frame with origin "http://localhost:35549" from accessing a cross-origin frame.
It seems to have an extra "Failed to read a named property '_qutebrowser' from
'Window'" before the "Blocked a frame ..." bit. Seems like maybe a nested
exception situation? Not sure what's going on there but the exception is still
being caught, which is the point of the test.
Hopefully we don't have more issues with subframes cropping up...
Looks like the kde-unstable arch repo has updated again. It says
6.8.0beta2-1.
I guess the number might change again in the future, still a couple of
months to go before release.
Starting with the upgrade to Hypothesis 6.103.4 we got hangs when pytest exits.
This is caused by:
https://github.com/HypothesisWorks/hypothesis/pull/4013
combined with:
https://github.com/python/cpython/issues/102126
which was fixed in Python 3.10.11, but the latest 3.10 packaged by Archlinux was
3.10.10.
Thus, we instead build a newer 3.10 from the AUR.
This bumps the build time up to about 20 minutes on my machine, which is
probably acceptable since those are nightly builds only anyways. We could
probably half that by disabling --enable-optimization, but that would be at the
cost of making the actual test runs (which run more often) slower.
Closes#8247
Will be dropped on GitHub Actions tomorrow:
https://github.blog/changelog/2024-05-20-actions-upcoming-changes-to-github-hosted-macos-runners/
For unit tests, we now run them on macOS 13 instead, thus testing on all three
macOS versions we currently support.
For releases, this forces us to now support macOS 12 as the oldest supported
version and drop macOS 11 support. Thus, we should not have a v3.2.2 release.
Not backporting this commit so CI fails there rather than silently bumping up
requirements.
My virtualenv I used to run webkit has rotted long ago and I don't remember
how I set it up. There is a PyQtWebKit project on PyPI but I don't know
who that's published by.
So I figured I would write some notes for myself on using the docker container
used for CI instead. I chose to mount the current directory (which is
presumably a qutebrowser checkout!) directly into the container instead of
cloning it so I could have quicker feedback between making code changes and
running tests.
Then there's a couple of things that stem from that. Since the user in the
container is different from the one in the host we have to move some things
that are normally written to the current directory to be written elsewhere.
There are other ways to approach this (eg you can add `-u $(id -u)` to the
docker command line, although that makes things a bit confusing in the
container) but arguably it's good for the container not to be able to write to
the host, hence making that volume read only.
The TOX_WORK_DIR trick is from
[here](https://github.com/tox-dev/tox/issues/20), apart from with
`{toxinidir}` in it too because the pyroma env was failing with just
`.tox`, saying the pyroma binary needed to be in the allowlist, possibly
it was doing full path matching without normalizing.
The hypothesis folks
[here](https://github.com/HypothesisWorks/hypothesis/issues/2367#issuecomment-595524571)
say if you want to override the examples DB location with an env var to
do it yourself. It's actually only a warning from hypothesis, it says it
falls back to an in-memory DB, but I guess the tests run with
warnings-are-errors. You can also pass `database=None` to make
hypothesis skip example storage altogether.
I'm using tox to run commands in a virtualenv with the right stuff in it
because, uh, because I was copying the CI workflow actually. I just found out
about the `exec` subcommand to override the `commands` defined for the env,
neat! One point of awkwardness about that is that since we are using the
PyQt from the OS we need any virtualenv we use to have access to the OS
packages, which isn't the default for virtualenvs created by tox. The
text envs use the link_pyqt script for that but if you are using this
container and the first thing you do is run `tox exec` then that
wouldn't have been run. So I'm setting `VIRTUALENV_SYSTEM_SITE_PACKAGES`
to tell tox to always make the system packages available in the
virtualenvs it manages.
I did try using the mkvenv script instead of tox but it complained when
trying to install the current directory in editable mode because
setup.py tries to write to a git-commit-id file.
The "Async question interrupted by async one" test was failing on CI but
not locally. Not sure why, changing the tests to skip instead of xfail.
The downside is that we won't bet a notification when upstream fixes the
issues, hopefully they mark the QTBUG as closed and we see it that way.
I think we do have to do something else to deal with this persistent
permission thing anyway, assuming they don't change it to be
off-by-default, so I'm sure we'll be looking in this area again!
They'll at the very least be re-enabled when we get a PyQt 6.8.
The test (`test_failing_flush()`) is deliberately trying to write to a
close file. This is a new error in Qt 6.8 it seems.
Ignore just the specific file for this test in case it pops up somewhere
else later.
WebEngine now has it's own mechanism to remember permission, and it's
turned on by default. We can't disable it until PyQt picks up the new
`QWebEngineProfile::setPersistentPermissionsPolicy()`. So the first test
that prompts for a permission will persist that setting and later ones
will fail because they don't get the prompt they expect.
For now lets set them to xfail while we figure out what to do with
permission persisting for actual users. That we we can reduce the noise
in the test results!
The WebEngine permissions persistence mechanism doesn't seem to
respect whatever we are doing to separate storage per-basedir. Testing
with a temp basedir like so:
python3 -m qutebrowser -T https://web-push-book.gauntface.com/demos/notification-examples/
I see this file has been created under my home directory:
$ cat ~/.local/share/qutebrowser/qutebrowser/QtWebEngine/Default/permissions.json
{"Notifications":{"https://web-push-book.gauntface.com/":true}}
I've raised an issue upstream about that here: https://bugreports.qt.io/browse/QTBUG-126595
We've got two things to think about regarding how to deal with this new
on-by-default feature:
1. what do we do for the tests? We can Disable the feature (if on new
enough PyQt) or add a test setup step that ... restarts the browser
and deletes the permissions.json. That's not great
2. what do we do for real users? See below
By default I would recommend disabling the webengine one since we
already have our own. BUT we can't actually disable it until PyQt
updates with the new APIs, which can take a while, and it's pretty
likely people will be using the new Qt version before PyQt updates. So
it would be best to figure out what we can do before that! Can we make
it respect the basedir data path? Can we make it write to some fake file
we throwaway? chmod +i?
Hopefully Qt makes the permission JSON respect the data path we set and
then at the very least we can remove the JSON file after a permission is
set. It'll still be a change in behavior for users on Qt 6.8 and PyQt
6.7 though as it'll likely remember permissions within a browser
instance by default, which isn't the case for our implementation
currently.
Related to: https://github.com/qutebrowser/qutebrowser/issues/8242#issuecomment-2175949686
Background:
As part of my work on reproducible builds for openSUSE, I check that software still gives identical build results in the future.
The usual offset is +16 years, because that is how long I expect some software will be used in some places.
This showed up failing tests in our package build.
See https://reproducible-builds.org/ for why this matters.
With GitHub Actions now providing macOS 14 runners with M1 chips, we can
build a separate Apple Silicon release there and upload it.
Universal wheels are currently not possible, see #8229 for details.
Closes#6478
We want to make sure that the selection model gets deleted when clearing
the model, since we are switching from doing that directly to having it
happen indirectly based off of signals we don't manage.
Hopefully this doesn't end up to be flaky. I think we are depending on
this happening in two different Qt even loop runs (`model.deleteLater()`
in the first one then `selmod.deleteLater()` gets called from the
`model.deleted` signal).
I tried using `qtbot.wait_signals()` with a list but it segfaulted in
some cleanup method. Perhaps is doesn't handle the deleted signal well?
Alternately we could maybe just wait for the selmodel one and check
`sip.isdelted(model)` to check the model is gone too.
This starts happening reliably with the "Using session completion" end
to end test after the previous change where I made it so we don't set
the completion widget model to none, just call `deleteLater()` on the old
one. The relevant part of the test is:
And I run :session-save hello
# This loads a completion model with one entry
When I run :cmd-set-text -s :session-load
# This one selects the entry, which clears the model because the
# session completion only applies to the first word after the
# command, which we've just selected.
And I run :completion-item-focus next
# This does nothing, since we have no completion model loaded,
# not sure why it is in the test, presumably making sure no
# exception is thrown.
And I run :completion-item-focus next
The log message is reliable in the test but it's a bit flaky to
reproduce manually. I've reproduced it via `for f in 1 2;do qute-send -i
/tmp/qutebrowser-basedir-npzdpupy/runtime/ipc-8075cf54f63b8e1bc05db34f41292c38
":completion-item-focus next" ;done` if I manually go through the
scenario a few times. Possibly it would reproduce easier if I pin it to
one process using `taskset`.
I think the reason for this is that the model is marked for deletion in
the next event loop, then a signal goes out, then the selection model is
marked for deletion on the next event loop. And possibly this only
happens between the model and the selection model being deleted?
To avoid a leak when calling `QTreeView.setModel(None)`, this commit switches
to relying on the `model.destroyed` signal to make sure related state is
cleaned up. Upstream bug: https://bugreports.qt.io/browse/QTBUG-49966
When you call `setModel(None)` on a QTreeView it causes a small memory leak
of `QItemSelectionModel` objects created by the QTreeView's child QHeaderView
object.
`QAbstractItemView` will create a new `QItemSelectionModel` whenever a model
is set, even if that model is `None`. When the new model is non-null the
new selection model will be set to be deleted when the new model is, but when
the new model is None the new selection model will be linked to the
static `QAbstractItemModelPrivate::staticEmptyModel()`. Since that empty model
lives forever, so do the related section models, unless callers take care to
clean them up themselves.
Both `QTreeView` and it's child `QHeaderView` implement `QAbstractItemView`
and have this behaviour. For the `QTreeView` we were making sure to delete
the old selection model ourselves (as of fe1215c74d). But for the one
created by the `QHeaderView` we can't get a reference it because
`QTreeView.setModel()` would call `QHeaderView.setModel()` and then
`QHeaderView.setSelectionModel()` right away to assign it's own
selection model to the child, leaving no references to the selection
model created by `QHeaderView.setModel()`.
I was previously using `header.findChildren(QItemSelectionModel)` to clean up
old orphaned selection models, but this approach is a lot simpler!
To observe this for yourself you can plonk something like this in
`set_model()` before the early return and switch between the old and new
implementation and see how it changes behaviour.
header = self.header()
header_children = header.findChildren(QItemSelectionModel)
our_children = self.findChildren(QItemSelectionModel)
print(f"{len(our_children)=} {len(header_children)=}")
You can also observer the selection models accumulating in Gammaray
(https://github.com/KDAB/GammaRay/) if you just open and close the selection a
lot and then filter the object view by "model".
The relevant code is in `QTreeView` and `QAbstractItemView`'s
`setModel()`, `setSlectionModel()` and `modelDestroyed()`. Bot mostly in
the `setModels()` where you can see the relevant signals being connected
and disconnected.
https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/itemviews/qtreeview.cpp#n179Fixes: #7947
The previous way of obtaining the Bang's URL was using the
response's body, using the url atrribute of a meta tag. The way
this was handled was a bit problematic: some URLs were of the format
/l/?uddg=<URL>&<some-other-query-arguments>, while others (such as google)
is simply <URL>. This could have been fixed by using an if-else, but I
preferred another approach. Lite DuckDuckGo sends a 303 response when
using Bangs, instead of a 200 response which makes the redirect. The
location header of the 303 response is the search engine's URL, so it's
easy to implement this function, working for both afformentioned URL
templates.
2023-02-26 15:47:13 +01:00
340 changed files with 6047 additions and 1993 deletions
# Run tests, passing positional args through to pytest.
tox -e py-qt5 -- tests/unit
----
Profiling
~~~~~~~~~
@ -582,6 +602,7 @@ Info pages:
- chrome://device-log/ (QtWebEngine >= 6.3)
- chrome://gpu/
- chrome://sandbox/ (Linux only)
- chrome://qt/ (QtWebEngine >= 6.7)
Misc. / Debugging pages:
@ -592,6 +613,7 @@ Misc. / Debugging pages:
- chrome://ukm/ (QtWebEngine >= 5.15.3)
- chrome://user-actions/ (QtWebEngine >= 5.15.3)
- chrome://webrtc-logs/ (QtWebEngine >= 5.15.3)
- chrome://extensions/ (QtWebEngine >= 6.10)
Internals pages:
@ -767,10 +789,12 @@ New PyQt release
qutebrowser release
~~~~~~~~~~~~~~~~~~~
* Make sure there are no unstaged changes and the tests are green.
* Make sure there are no unstaged or unpushed changes.
* Make sure CI is reasonably green.
* Make sure all issues with the related milestone are closed.
* Mark the https://github.com/qutebrowser/qutebrowser/milestones[milestone] as closed.
* Consider updating the completions for `content.headers.user_agent` in `configdata.yml`.
* Consider updating the completions for `content.headers.user_agent` in `configdata.yml`
and the Firefox UA in `qutebrowser/browser/webengine/webenginesettings.py`.
* Minor release: Consider updating some files from main:
- `misc/requirements/` and `requirements.txt`
- `scripts/`
@ -780,7 +804,8 @@ qutebrowser release
**Automatic release via GitHub Actions (starting with v3.0.0):**
* Double check Python version in `.github/workflows/release.yml`
* Run the `release` workflow on the `main` branch, e.g. via `gh workflow run release -f release_type=major` (`release_type` can be `major`, `minor` or `patch`; you can also override `python_version`)
* Run the `release` workflow on the `main` branch, e.g. via `gh workflow run release -f release_type=minor` (`release_type` can be `major`, `minor` or `patch`; you can also override `python_version`)
* Consider running `gh run watch` or `gh run view --web` to watch the progress
@ -2274,21 +2276,22 @@ The following placeholders are defined:
* `{upstream_browser_key}`: "Version" for QtWebKit, "Chrome" for
QtWebEngine.
* `{upstream_browser_version}`: The corresponding Safari/Chrome version.
* `{upstream_browser_version_short}`: The corresponding Safari/Chrome
version, but only with its major version.
* `{qutebrowser_version}`: The currently running qutebrowser version.
The default value is equal to the unchanged user agent of
QtWebKit/QtWebEngine.
The default value is equal to the default user agent of
QtWebKit/QtWebEngine, but with the `QtWebEngine/...` part removed for
increased compatibility.
Note that the value read from JavaScript is always the global value. With
QtWebEngine between 5.12 and 5.14 (inclusive), changing the value exposed
to JavaScript requires a restart.
Note that the value read from JavaScript is always the global value.
This setting supports link:configuring{outfilesuffix}#patterns[URL patterns].
Type: <<types,FormatString>>
Default: +pass:[Mozilla/5.0 ({os_info}) AppleWebKit/{webkit_version} (KHTML, like Gecko) {qt_key}/{qt_version} {upstream_browser_key}/{upstream_browser_version} Safari/{webkit_version}]+
Default: +pass:[Mozilla/5.0 ({os_info}) AppleWebKit/{webkit_version} (KHTML, like Gecko) {upstream_browser_key}/{upstream_browser_version_short} Safari/{webkit_version}]+
[[content.hyperlink_auditing]]
=== content.hyperlink_auditing
@ -2344,18 +2347,20 @@ Default: +pass:[false]+
=== content.javascript.clipboard
Allow JavaScript to read from or write to the clipboard.
With QtWebEngine, writing the clipboard as response to a user interaction is always allowed.
On Qt < 6.8, the `ask` setting is equivalent to `none` and permission needs to be granted manually via this setting.
This setting supports link:configuring{outfilesuffix}#patterns[URL patterns].
Type: <<types,String>>
Type: <<types,JSClipboardPermission>>
Valid values:
* +none+: Disable access to clipboard.
* +access+: Allow reading from and writing to the clipboard.
* +access-paste+: Allow accessing the clipboard and pasting clipboard content.
* +ask+: Prompt when requested (grants 'access-paste' permission).
Default: +pass:[none]+
Default: +pass:[ask]+
[[content.javascript.enabled]]
=== content.javascript.enabled
@ -2764,10 +2769,9 @@ Type: <<types,FlagList>>
Valid values:
* +ua-whatsapp+
* +ua-google+
* +ua-slack+
* +ua-googledocs+
* +ua-gnome-gitlab+
* +js-whatsapp-web+
* +js-discord+
* +js-string-replaceall+
@ -3890,7 +3894,7 @@ Chromium has various sandboxing layers, which should be enabled for normal brows
Open `chrome://sandbox` to see the current sandbox status.
Changing this setting is only recommended if you know what you're doing, as it **disables one of Chromium's security layers**. To avoid sandboxing being accidentally disabled persistently, this setting can only be set via `config.py`, not via `:set`.
* +auto+: Disable on Qt6 < 6.6.0, enable otherwise
* +auto+: Disable on Qt versions with known issues, enable otherwise
* +never+: Enable accelerated 2d canvas
Default: +pass:[auto]+
[[qt.workarounds.disable_accessibility]]
=== qt.workarounds.disable_accessibility
Disable accessibility to avoid crashes on Qt 6.10.1.
This setting requires a restart.
This setting is only available with the QtWebEngine backend.
Type: <<types,String>>
Valid values:
* +always+: Disable renderer accessibility
* +auto+: Disable on Qt versions with known issues, enable otherwise
* +never+: Enable renderer accessibility
Default: +pass:[auto]+
[[qt.workarounds.disable_hangouts_extension]]
=== qt.workarounds.disable_hangouts_extension
Disable the Hangouts extension.
The Hangouts extension provides additional APIs for Google domains only.
Hangouts has been replaced with Meet, which appears to work without this extension.
Note this setting gets ignored and the Hangouts extension is always disabled to avoid crashes on Qt 6.5.0 to 6.5.3 if dark mode is enabled, as well as on Qt 6.6.0.
This setting requires a restart.
This setting is only available with the QtWebEngine backend.
Type: <<types,Bool>>
Default: +pass:[false]+
[[qt.workarounds.locale]]
=== qt.workarounds.locale
Work around locale parsing issues in QtWebEngine 5.15.3.
^QObject::connect: Connecting from COMPAT signal \(QWebEnginePage::featurePermissionRequest(ed|Canceled)\(QUrl,QWebEnginePage::Feature\)\)
xfail_strict=true
filterwarnings=
error
default:Test process .* failed to terminate!:UserWarning
# Python 3.12: https://github.com/ionelmc/pytest-benchmark/issues/240 (fixed but not released)
ignore:(datetime\.)?datetime\.utcnow\(\) is deprecated and scheduled for removal in a future version\. Use timezone-aware objects to represent datetimes in UTC. (datetime\.)?datetime\.now\(datetime\.UTC\)\.:DeprecationWarning:pytest_benchmark\.utils
Florian Bruhin
dependabot[bot]
qutebrowser bot
Jan Palus
gesh
OmeletWithoutEgg
Rebecca
Teddy
mathis renaud
Andy Flesner
Sander
toofar
dezeroku
user202729
Harm te Molder
Jun Chen
sarabande
Ruben Gonzalez
Aryan Dev Shourie
bosshogg
Marcel Schilling
Willow Barraco
bitraid
Sviatoslav Sydorenko
Fernando Ramos
owl
Bernhard M. Wiedemann
Tobias Naumann
Michael M
Michael M
Eolien55