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
- Drop trailing comma inside trivial tuple
- Use r"""...""" for string containing ", as \" inside r"..." is taken literally
(I'm surprised it works!)
- Use ['"] instead of ('|")
- Also adjust the inner [^'] to [^'"] for consistency
This reverts commit 7144e72116.
Turns out this adds complexity, but doesn't really help with debugging that
much, and also needs us to guess what the correct ID might be.
We should rather go for a nicer (but more invasive) cleanup by having a separate
IPCConnection class that stores a socket and ID, see #8208.
As of at the very least the latest version the version string looks
like:
const pdfjsVersion = "4.2.67";
So change the regex to allow double quotes too.
I'm trying to update pdf.js in the bleeding edge CI jobs. It complains
that either it can't find PyQt or it can't find yaml depending on how I
invoke tox. Joy. Since dict stuff isn't run by default in this script
hopefully that is the only broken import path and moving it into the
function lets the pdfjs (and ace) bit of the script work fine.
Actually, looking at the stack traces below, both of them are from dict
related code!
tox exec -re bleeding -- python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 15, in <module>
from qutebrowser.qt.core import pyqtSignal, pyqtBoundSignal, QObject
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/core.py", line 17, in <module>
machinery.init_implicit()
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/qt/machinery.py", line 278, in init_implicit
raise NoWrapperAvailableError(info)
qutebrowser.qt.machinery.NoWrapperAvailableError: No Qt wrapper was importable.
python scripts/dev/update_3rdparty.py --gh-token ***
Traceback (most recent call last):
File "/__w/qutebrowser/qutebrowser/scripts/dev/update_3rdparty.py", line 20, in <module>
from scripts import dictcli
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/dictcli.py", line 25, in <module>
from qutebrowser.browser.webengine import spell
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/browser/webengine/spell.py", line 14, in <module>
from qutebrowser.utils import log, message, standarddir
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/message.py", line 17, in <module>
from qutebrowser.utils import usertypes, log
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/usertypes.py", line 16, in <module>
from qutebrowser.utils import log, qtutils, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/qtutils.py", line 39, in <module>
from qutebrowser.utils import usertypes, utils
File "/__w/qutebrowser/qutebrowser/scripts/dev/../../scripts/../qutebrowser/utils/utils.py", line 29, in <module>
import yaml
ModuleNotFoundError: No module named 'yaml'
This installs pdf.js in a selection of CI jobs. Previously the PDF.js
tests (in qutescheme.feature) were skipped in CI because it wasn't
installed anywhere. There has been a couple of recent cases where pdf.js
started depending on javascript features that are too new for even the
most recent QtWebEngine to support. The aim of this commit is to catch
that case. This doesn't add coverage for older webengine releases.
This also incidentally updates the ace editor in these test jobs, since that is
also updated by default by the update_3rdparty script. Hopefully that
doesn't cause issues.
The reasoning for installing on each type of job:
*ubuntu jobs*: not installed - while our main test runs are on ubuntu
there is an upstream issue where many assets used by pdf.js (like icons
used in the toolbar) aren't packaged, see #7904. This causes warning
messages because assets requested via qutescheme can't be found, which
causes the tests to fail. We could a) install pdf.js from source instead
of using the ubuntu one b) ignore the warning logs c) skip this
environment and rely on tests elsewhere. I've chosen to do (c). I don't
see a huge benefit in testing pdf.js across multiple environments if we
aren't using it installed from the OS anyway. We could install from
source but currently all the Qt < 6.5 tests are failing from some other
JS error, and I think fixing that is out of scope of this issue.
*docker Qt6*: installed - the archlinux pdfjs package works fine and we are
only testing the most recent Qt versions because arch users are expected
to stay up to date.
*docker Qt5*: not installed - doesn't support JS features required by
PDF.js. I guess we could install the legacy build from source here. I'm
mostly worried about catching new breakages for this commit though
*windows*: installed - we install pdf.js from source when making a
release so it would be nice to do that in tests too.
*macos*: not installed - the tests that were catching the breakages are
end2end tests which we don't run on mac. And I think there was an
error from the :versions tests here, don't remember.
*bleeding edge*: installed - from source
pdf.js tests fail on Qt < 6.5 with `Uncaught TypeError: Cannot read
properties of null (reading 'mainContainer')`
The `TestPDFJSVersion.test_real_file` unit tests currently fails because
`version._pdfjs_version()` returns `unknown (bundled)`, not sure why. I
think this is pre-existing and it also wasn't being run on CI.
The `test_cleanup()` test for guiprocess was triggering the early timer
warning messages because it was using a VeryCourse timer with a 100ms
interval. Changed it to to a normal Course timer (the default) for that
test.
For the `ipc-timeout` timer we know that is happening in the tests on
windows. It's being logged in lost of e2e tests, the elapsed times it's
logging are between 0 and 0.020s. I'm not sure if it's the right thing
to be changing the log level in production code or marking the messages
as expected in test code.
Since we are now checking for early timer firings in usertype.Timer I
figured why do it in two places, lets call the check method on Timer in
the handler in the IPC class too.
I also removed the platform and version check because while the bug only
happens under those conditions the check to ensure a timer event was
valid should work everywhere. Right? We can add them back of someone
wants but I'm not sure I see a point.
It would be nice if we could filter out invalid timer events in
usertypes.Timer and not require the consuming code to have to do
anything. But that would be more work, so lets wait until more places of
code actually need to care about it, chances are it'll be fixed in Qt
soon enough anyway.
I also think usertypes.Timer should be restarting the time so it gets
called at the proper time. But if we aren't filtering the events that
would mean it gets called twice.
Since we added some sanity checking in usertypes.Timer() around
QTBUG-124496 it would be convenient if there was a reminder for future
timer users to use our Timer object instead. Here's one!
It's looking for QTimer initialisations, we are still allowing
QTimer.singleShot(), although that probably can hit the same issue.
It uses an end-of-line anchor in the regex so you can put a comment (any
comment) on the end of the line to ignore the check.
- Add a couple new "raise utils.Unreachable" to avoid
possibly-used-before-assignment issues.
- Simplify an "if" for the same reason
- Remove an unneeded "return"
- Use "NoReturn" to prepare for pylint knowing about it in the future:
https://github.com/pylint-dev/pylint/issues/9674
- Add some ignores for used-before-assignment false-positives
- Ignore new undefined-variable messages for Qt wrapers
- Ignore a new no-member warning for KeySequence:
https://github.com/pylint-dev/astroid/issues/2448#issuecomment-2130124755
Easier fix instead of 6c4be8ef03.
Seems to get picked up just fine, and shouldn't hurt when it's not needed, as we
don't use --pre. Thus, no development releases should be installed.
6.7 is released now, some distros are already shipping it!
This commit:
1. adds a new 6.7 requirements file (the plain 6 one has already been
updated by the bot)
2. adds a new tox env referring to the new requirements file
3. updates the mac and windows installer jobs to run with pyqt67 with the
assumption we'll be including that in our next release
4. adds two new CI environments for 6.7, one each for python 3.11 and 3.12
(3.12 only came out like 6 months ago)
5. updates a couple of references to the py37 tox env that looked like they
were missed, 3.7 support was dropped in 93c7fdd
6. updates various ubuntu containers to the latest LTS instead of the previous
related one - this is quite unrelated to this change but I thought I would
give it a go, no need to use the old one unless we are specifically testing
on it?
- linters - they use tox but probably use system libraries
- these all run in nested containers anyway, should be fully isolated
- codeql - eh, uses a third party action, check the docs if it fails
- irc - as above
Change `CompletionView.on_clear_completion_selection()` to call the Qt
selection model getter, instead of our one. Since it can be called when the
selection model has already been cleared and our one asserts that there
is a selection model to return.
Back in the distant past there was a change to handle the completion widget's
selection model being None when the `on_clear_completion_selection()` slot was
called: https://github.com/qutebrowser/qutebrowser/commit/88b522fa167e2f97b
More recently a common getter for the selection model was added so we could
have a single place to apply type narrowing to the returned object from the Qt
getter (the type hints had been updated to be wrapped in `Optional`): https://github.com/qutebrowser/qutebrowser/commit/92dea988c01e745#diff-1559d42e246323bea35fa064d54d48c990999aaf4c732b09ccd448f994da74cf
It seems this is one place where it does, and already did, handle that
optional. So it didn't need to change to use the new getter. This is called
from the `Command.on_mode_left` signal, not sure why the selection model is
None here. Perhaps it already gets cleared by the effects of the `hide_cmd`
signal that gets fired earlier, or perhaps even from the `self.hide()` on the
line before. Anyway, this was working for several years and seems harmless
enough.
I was getting crash reports from someone about this. Not sure what's going wrong
there (hence the additional information in the exception).
What's clear however is that we're raising ParseError, but only handling that
when actually parsing. The code calling copy_/_find_webengine_resources only
handles OSError. So let's raise a FileNotFoundError instead.
We already had all this information in a comment anyways.
I made it machine-readable using:
s/#\s+(\d*)\.(\d*)\.(\d*): Security fixes up to ([^ ]*)\s+\((.*)\)/utils.VersionNumber(\1, \2, \3): (_BASES[XX], '\4'), # \5
plus some manual post-processing.
Thanks to that, we can now get the security version from that data even on
QtWebEngine < 6.3, if that information is known. When we fall back to a base
version (e.g. 6.7.99 -> 6.7), we make sure to not pretend that we have the .0
state of things, though.
Finally, we cross-check the information against the current Qt version if we
have the API, which mostly ensures the data is accurate for human readers.
See #7187 and #8139.
More readable now that we have more information in it.
Also always show the source, now that we have the space for it, and "UA" isn't
the obvious default anymore anyways.
Similarly to 24d01ad257, failing Qt 5.15 tests
showed some evidence of us being stuck in command mode in the next test file
(hints.feature). On the first test there ("Scenario: Using :hint-follow outside
of hint mode (issue 1105)"):
17:38:51.073 ERROR message message:error:63 hint-follow: This command
is only allowed in hint mode, not command.
but:
end2end.fixtures.testprocess.WaitForTimeout: Timed out after 15000ms waiting
for {'category': 'message', 'loglevel': 40, 'message': 'hint-follow: This
command is only allowed in hint mode, not normal.'}.
I agree with what has been said: This should never happen, because we restart
the qutebrowser process between test files. I did some of the mentioned "more
examination" but also don't have an explanation.
To avoid more flaky tests, let's roll with another bandaid solution.
With pytest 8.2, pytest.importorskip(...) now only considers ModuleNotFoundError
rather than all ImportErrors, and warns otherwise:
https://github.com/pytest-dev/pytest/pull/12220
While we could override this via
pytest.importorskip(..., exc_type=machinery.Unavailable)
this is a simpler solution, and it also makes more sense semantically:
We only raise Unavailable when an import is being done that would otherwise
result in a ModuleNotFoundError anyways (e.g. trying to import QtWebKit on Qt
6).
When qutebrowser is running but its installation has been deleted/moved, it
fails in somewhat mysterious but predictable ways. This is e.g. the case
currently, when people upgrade their Archlinux packages and upgrade from Python
3.11 to 3.12. When doing that with qutebrowser open, on the next page load, it
will:
- Have a crashed renderer process, because (assumingly) the process executable
is gone on disk.
- Which then causes us trying to render an error page, but that fails due to
broken_qutebrowser_logo.png being gone from disk.
- The FileNotFoundError then causes jinja2 to import jinja2.debug at runtime,
but that *also* fails because the jinja2 package is gone.
We work around this by loading the PNG into RAM early, and then using the cached
version instead. This amends b4a2352833 which did
the same with HTML/JS resources, but never for this PNG, which (looking at crash
logs) seems to be a somewhat common breakage.
Alternatives I've considered:
- Catching the FileNotFoundError and not showing an error page at all.
- Generating a PNG with an explanatory text via QPainter and returning that.
However, with the renderer process crash happening in the first place for
unknown reasons, it's unclear if the error page ever gets actually displayed...
Let's roll with this for now, and if this causes a repeating renderer process
crash, fix that separately (also see #5108).
mypy:
Mypy knows about the QDataStream.Status.SizeLimitExceeded attribute now,
so we can remove the ignore. But mypy for pyqt5 doesn't know about it,
so put the whole graceful block behind an additional conditional as well
to hide it from pyqt5 mypy.
cheroot:
looks like the format of the error message we are already ignoring
changed slightly. The `ssl/tls` bit changed to `sslv3`, at least in our
setup.
Previously (a209c86c55) I've added a delay in browser code before sending
events to the page to account with a race condition where events weren't
processed after navigating. Then I had to add extra checks to tests that had
tight timing requirements.
That was for click-element, this commit reverts an attempt at the same
strategy for fake-key and instead adds wait statements in the tests that where
hitting the original race condition (sending events "too soon" after a
navigation).
The reason for the different approach here is that after adding the delay in
fake-key I had to add an extra "wait for log line" message to a few tests with
tight timing requirements to watch for a Tab key press. That worked great for
webengine but it made some tests start failing on webkit. We don't seem to get
that log message on webkit. I've got no-idea why and frankly think I've spent
way too much time on just a handful of tests already.
It's unfortunate we have to add manually delays in the e2e tests. It makes me
think if anyone else adds a test in the future with this combination of steps
(open page, run fake-key) they'll run into the same issue and it'll be hard to
spot. Oh well, we'll deal with that when it comes up.
The tests that where failing on webkit are the ones in caret.feature
touched in this commit.
Reverts included in this commit:
Revert "Delay fake-key events by 10ms"
This reverts commit 9f050c7460.
Revert "Wait for evidence of Tab key press in tests before proceeding"
This reverts commit d47d247941.
In the previous commit 9f050c7460 "Delay fake-key events by 10ms" I delayed
events for fake-key by 10ms to deal with a timing issue with sending events
right after a navigation not being processed.
Just like last time that's caused a few tests with tight timing constraints to
break because they proceed to the next command before the fake-key events have
taken effect.
Maybe it would have been better to just put the waits in the e2e tests in the
first case instead of in the production code? Well, we'll see. Maybe I'll
never have to deal with this again.
Similar to a209c86c55 "Delay QEvent driven mouse clicks by 10ms" it seems
that sending fake-key QEvents immediately after a navigation causes the events
to not be processed.
This is causing e2e tests in keyinput.feature to fail in a flaky, but
frequent, manner. This can also be resolved in some other ways like putting
a wait in the e2e tests like so:
When I open data/keyinput/log.html
And I wait 0.01s
And I run :fake-key <Escape>
But relying on maintainers to remember to do that seems error prone. If this
10ms delay turns out to be something to get rid of we could try keep this
delay to be used in less cases:
1. add some magic to e2e tests where it compares the previous and current line
and if it sees an open and a click-element or fake-key line next to each
other it delays for a bit
2. in the application code in tab.send_event() store the time of last
navigation and queue events for sending till after that
The affected tests in this case where:
tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_key_to_the_website
tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_special_key_to_the_website
tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_keychain_to_the_website
Exit prompt mode on the last test in downloads.feature because it's
preventing the next tests in editor.feature from running. Screenshots of
the first failing test in editor.feature shows a prompt open and
jsprompt open in the background.
This shouldn't happen as there is a module level fixture in conftest.py
that is supposed to close and re-open the browser process between each
feature file. That bears more examination but for now this change looks
pretty painless.
I've added a 10ms delay when sending a fake click via QEvents to help with a
timing issue in the prompt e2e tests in Qt6.7. Apparently that throws off the
tight timing of this one test! I guess the `:scroll bottom` comes before the
iframe has been clicked.
I'm a little worried that if we are depending on timing stuff like this we
should be looking at a more thorough solution. Maybe the "and follow" suffix
can trigger it to wait for "Clicked *editable element!"?
Also I can try to reduce the delay in the click, but 10ms is pretty short
already.
On CI with Qt 6.7 we are seeing several tests failing in a flaky, but
frequent, manner. These tests all seem to be doing :click-element and
sending a "fake" click, as in one driven by QEvents, to a button on the
test page after opening the page in an existing tab. In the logs we see
that the element was identified with JS correctly but we don't see any
evidence of the actual click after that.
I've been testing locally with `python3 -m pytest
tests/end2end/features/test_prompts_bdd.py -k test_javascript_confirm`.
Delaying the click event by as little as 5ms seems to make the tests
consistently pass. I'm setting it to an arbitrary 10ms here for no good
reason. It's unfortunate that we have to change production code to make
tests pass, it's unlikely that a 10ms window will affect real usage, and
the problem is only immediately after a navigation. If we can't find a
better way to resolve this and want to get rid of the 10ms delay when no
needed we could maybe look at when the last navigation time for the tab
was to decide whether to delay or not.
I also tried changing all the "open in" lines in the failing tests to be
"open in new tab" and that seemed to make the flaky tests pass
consistently too. But relying on people writing tests in a specific way
wouldn't really be fixing the problem.
Additionally, running just one test at a time (with `taskset -c 1 ...`)
made all the tests pass consistently. So it seems like the root cause
could have something to do with that, somehow. Or maybe just running the
pytest process and the browser process on the same CPU causes enough of
a delay to not trigger it.
I don't know what the root cause of this change is, or if we can use a
more event based way of knowing when the page (or its focus proxy) is
ready to receive events. I've tried digging through events and debugging
webengine and haven't got anywhere, hopefully this delay is painless
enough that it'll do. To summarize, the page, or focus proxy, doesn't
process events (all events, just mouse events?) immediately after
navigating.
I initially tried delaying all events in `tab.send_event()`, but that
caused some caret tests to fail (the `selectionfollow_with_link_tabbing`
e2e ones), not sure why. So I walked back to just delaying click events.
I've seen some intermittent flakyness in
`test_misc_bdd.py::test_clicking_on_focused_element` and
`tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_keychain_to_the_website`
which may indicate that `:fake-key` needs the same treatment. We'll see
how things stand once it's been stable on the main branch for a while
instead of being muddled up in this investigation branch.
I thought the issue might be that the RenderWidgetHostViewQt was being swapped out
on the new page load and the old one being sent the event. So I added a bunch
of debug logging to try to get a better view of that. None of this debug
logging is showing up when the tests fail so it seems that isn't the case.
The tests failing in the last four bleeding edge qt6 tests before these
changes were (test, count):
('tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_special_key_to_the_website', 2),
('tests/end2end/features/test_prompts_bdd.py::test_using_contentjavascriptalert', 2),
('tests/end2end/features/test_editor_bdd.py::test_select_two_files_with_multiple_files_command', 2),
('tests/end2end/features/test_misc_bdd.py::test_clicking_first_element_matching_a_selector', 2),
('tests/end2end/features/test_misc_bdd.py::test_clicking_on_focused_element', 2),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_prompt_with_default', 2),
('tests/end2end/features/test_prompts_bdd.py::test_using_contentjavascriptprompt', 2),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_confirm__aborted', 2),
('tests/end2end/features/test_editor_bdd.py::test_file_selector_deleting_temporary_file', 1),
('tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_keychain_to_the_website', 1),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_confirm__no', 1),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_confirm_with_default_value', 1),
('tests/end2end/test_insert_mode.py::test_insert_mode[100-input.html-qute-input-keypress-awesomequtebrowser]', 1),
('tests/end2end/features/test_misc_bdd.py::test_clicking_an_element_by_position', 1),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_confirm__yes', 1),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_confirm_with_invalid_value', 1),
('tests/end2end/test_insert_mode.py::test_insert_mode[125-input.html-qute-input-keypress-awesomequtebrowser]', 1),
('tests/end2end/features/test_editor_bdd.py::test_select_two_files_with_single_file_command', 1),
('tests/end2end/features/test_keyinput_bdd.py::test_fakekey_sending_key_to_the_website', 1),
('tests/end2end/features/test_misc_bdd.py::test_clicking_an_element_by_id_with_dot', 1),
('tests/end2end/features/test_prompts_bdd.py::test_javascript_alert_with_value', 1)
For debugging in GBD under test I ran the test process under GDB and
passed a script to GDB to print out a message, and then continue, when
it hit a breakpoint. Unfortunately, the tests always passed when run
under GDB.
For that I changed `_executable_args()` in quteprocess to return `"gdb",
"-q -x handleMouseEvent-breakpoint -args".split() + [executable] + args`
where `handleMouseEvent-breakpoint` is:
set breakpoint pending on
set debuginfod enabled off
break RenderWidgetHostViewQtDelegateClient::handleMouseEvent
commands
call (void) fprintf(stderr, "Entering handleMouseEvent\n")
continue
end
run
The tests were consistently passing, but erroring on the invalid gdb log lines.
This only shows up on the webkit CI job.
Probably something to do with the fact that we are using webkit builds
from the distant archives. I'm sure it'll break for real one of these
days.
flake8 got a new warning about a module name shadowing a builtin module: https://github.com/gforcada/flake8-builtins/pull/121
Probably would have been safe enough to ignore it. But I don't think
moving it is that hard anyway. Hopefully I didn't miss anything!
A couple of tests seem to be failing because there is two windows open and
they don't expect it. I haven't fixed the root cause, I looked through the
logs and couldn't see why a second window was open. But if this makes the
tests pass, I guess we can address that if someone reports it.
Also changed `_get_scroll_values()` to handle multiple tabs/windows being open
too while I was there.
Example logs: https://github.com/qutebrowser/qutebrowser/actions/runs/8548730512/job/23422922448
Sessions file from the test logs:
Current session data:
windows:
- geometry: !!binary |
AdnQywADAAAAAAAyAAAAMgAAA1EAAAKJAAAAMgAAADIAAANRAAACiQAAAAAAAAAAAyAAAAAyAAAA
MgAAA1EAAAKJ
tabs:
- active: true
history:
- active: true
last_visited: '2024-04-04T03:21:00'
pinned: false
scroll-pos:
x: 0
y: 0
title: about:blank
url: about:blank
zoom: 1.0
- active: true
geometry: !!binary |
AdnQywADAAAAAAAyAAAAMgAAA1EAAAKJAAAAMgAAADIAAANRAAACiQAAAAAAAAAAAyAAAAAyAAAA
MgAAA1EAAAKJ
tabs:
- active: true
history:
- active: true
last_visited: '2024-04-04T03:21:03'
pinned: false
scroll-pos:
x: 0
y: 40
title: Scrolling
url: http://localhost:39235/data/scroll/simple.html
zoom: 1.0
In the bleeding edge CI tests I'm seeing:
IGNORED: Path override failed for key base::DIR_APP_DICTIONARIES and path '/__w/qutebrowser/qutebrowser/.tox/bleeding/lib/python3.11/sitePath override failed for key base::DIR_APP_DICTIONARIES and path '/__w/qutebrowser/qutebrowser/.tox/bleeding/lib/python3.11/site-packages/PyQt6/Qt6/libexec/qtwebengine_dictionaries'
INVALID: -packages/PyQt6/Qt6/libexec/qtwebengine_dictionaries'
Where the second line looks like maybe spill over from the first one
that is for some reason being printed twice and interleaved! Maybe
different processes or threads?
I did a regex for "line ending with this and with no spaces in it" which
is hopefully safe enough to not pick up a wrong line accidentally.
Similarly to the last commit (982b8bdcec),
we run into a race between autofocus triggering and the test trying to
click the focused element.
This also affects the "when there is none" test, which loses the focus
before it has been set, thus ending up with a focused element but expecting
none.
We fix both in one go by manually focusing things with a tab keypress
instead of using autofocus.
See #8145 and #5390.
Fixes#8145, see #5390.
As long as we don't have a solution to get notified about focus happening
(#2471 possibly?), it looks like there is no better way to get notified
about this, so a delay will need to do for now.
With a Qt 6.7 developer build, the tests fail with:
ASSERT failure in QtGlobalStatic::ApplicationHolder<QAS>::PlainType*
QtGlobalStatic::ApplicationHolder<QAS>::pointer() [with QAS =
{anonymous}::Q_QAS_qtlsbLoader; PlainType = QFactoryLoader]: "The
application static was used without a QCoreApplication instance", file
.../qtbase/src/corelib/kernel/qapplicationstatic.h, line 54
Fatal Python error: Aborted
[...]
Current thread 0x00007c18bb3f3740 (most recent call first):
File ".../tests/unit/browser/webkit/test_certificateerror.py", line 23 in <module>
See https://codereview.qt-project.org/c/qt/qtbase/+/495239
jaraco added some utility libraries they maintain to the keyring package
that they also maintain.
Also update changelog URLs to be consistent since they have a "skeleton"
project which lays everything out and publishes all their projects the
same. Should we be preferring GH or RTD links? idk
Webengine added a getter for their chromium patch level back in Qt 6.3,
since they backport security fixes from chromium in the periods between
doing major chromium feature upgrades.
It's pulled from a hardcoded string in the webengine source
`src/core/web_engine_context.cpp` that's manually updated when they
backport something.
The "(plus any distribution patches)" bit in there is because it was
pointed out that some distributions backport their own security patches
or even use webengine from a branch when the hardcoded string only gets
updated at release time, despite patches being backported in the
meantime.
Closes: https://github.com/qutebrowser/qutebrowser/issues/7187
The lint ones are:
linters (pylint): qutebrowser/completion/completionwidget.py#L440
Consider using 'height = min(height, contents_height)' instead of unnecessary if block
linters (pylint): qutebrowser/browser/webengine/webview.py#L241
Useless suppression of 'no-member'
The no-member one might be due to this change: https://github.com/pylint-dev/pylint/issues/9246
For the importlib-resources one, I'm not sure why it's changed to be
underscore instead of a dash now. But at least it's consistent across
all the requirements files now!
I feel like I've seem this in a previous dependancy update too (for a
different package?) but I can't find that now.
Currently the unstable docker images are failing to build
(undefined symbol: _ZN5QFont11tagToStringEj, version
Qt_6. Looks like Qt has upgraded to 6.7 but pyqt6 hasn't been patched to
remove some symbols that are gone now).
But we might as well let the stable ones rebuild right?
It seems `sed -i` is not very portable. Initially we were using this
command:
sed -i '' '/.-d., .--debug.,/s/$/ default=True,/' qutebrowser/qutebrowser.py
and then that started breaking on windows, I'm not sure why, with "can't
read /.-d., .--debug.,/s/$/ default=True,/: No such file or directory".
Then we changed to:
sed -i '/.-d., .--debug.,/s/$/ default=True,/' qutebrowser/qutebrowser.py
so without the extension argument, but that broke on mac with "1:
"qutebrowser/qutebrowser.py": extra characters at the end of q command"
then we tried:
sed -i'' '/.-d., .--debug.,/s/$/ default=True,/' qutebrowser/qutebrowser.py
and that also broke on mac with the same error. On the recommendation of
stackoverflow I just changed it no not use in-place editing and do a
good old fashioned move afterwards. https://unix.stackexchange.com/questions/92895/how-can-i-achieve-portability-with-sed-i-in-place-editing
... record scratch ...
Apparently these GHA steps are being run in powershell in windows where
`mv` is implemented by `Move-Item` where you have to use -Force to
overwrite destination files.
But that's not portable. cp does happily overwrite without any
additional instruction though. So I'm doing cp instead of mv and then
removing the temp file.
Probably if this drags out anymore we should download something off of
pypi which is platform independent to handle it.
It gives an AttributeError for both signal.SIGHUP and
signal.Signals.SIGHUP. The Signals Enum is set up so you can use the
strings to key into it though, so that's nice. I was tempted to use a
walrus operator here but I think that's python 310 and I don't remember
what our minimum supported version is.
It doesn't look like this class has any unit tests currently. And since
we are adding a new signal handler to it I took the opportunity to add a
few tests to it to establish a bit of a framework so that next time we
touch it it will be easier to add more.
I didn't go for full coverage here. It's an improvement over what was
there previously and it should make merging more palatable. I don't
think handling SIGHUP is a very risky feature.
I chose to use mocker.patch over monkeypatch.setattr because I think it
provides a more friendly and powerful API due to being more tightly
integrated with magic mocks. And with `mocker.patch.object` you can even
use actual object names instead of strings, the same as monkeypatch
allows.
One thing I'm less confident with here is mocking all multiple things in
a couple of fixtures. Writing one fixture for each little thing I mock
doesn't feel appealing to me right now, but for mocks that tests need to
access there isn't really any idiomatic way to let the tests access
them. I previously just had the one fixture but pulled out the read
config one for that purpose. It's a pattern I'll have to think on a bit
I think. Probably having to have developers thing about the balance of
boilerplate vs accessibility is cognitive load that we want to avoid.
Hmm.
Anyway, here are the options I was looking at to let test code access
mocks that where all shoved away into the one fixture:
1. group the tests into a class and put the mocks in class variables:
could work fine but I don't think it's very idiomatic for pytest?
2. return some kind of meta object from the fixture that has the object
under test as one attribute and the mocks as other ones: hmm,
returning multiple values from a method always seemed a bit smelly to
me
3. make one fixture for each of the mocks, have the main fixture depend
on them all, tests that need to modify them can depend on them too:
writing all those fixtures seems tedious and I don't think it would
be a clear win in terms of readability. *sigh*, I suppose I should
pull the ones I'm modifying out at least so other people don't copy
my lazyness in the future
4. have the test code access the mocks from the real code, eg
`configfiles.sys.exit.side_effect = ...`: this works fine but I feel
like it's relying on an implementation detail and probably shouldn't
be encouraged? Not sure
In case anyone is struggling with git blame and wondering what the
QSocketNotifier stuff is about, this commit should explain it: 210ce8ca7c
Before this commit qutebrowser would just exit when receiving a SIGHUP.
Now it will reload the config using the bare config_source function, just like
the :config-source command does.
Closes#8108.
- Don't split the pattern in weird places used for str.join, instead
build up the individual groups and then "".join them, which seems more
readable to me.
- Don't reuse "val" for both the user-input pattern and the regex pattern.
With the change in #7955, if the regex did not match, it ended up retrying the
lookahead at every position of the string (expected), but then *also* repeated
that process for every position in the string. Thus, a non-matching pattern
ended up in O(n^2) performance (with n = length of URL).
Instead, anchor the pattern at the beginning of the string. This doesn't change
behaviour as we use .* at the beginning of every lookahead anyways, but it means
we end up with O(n) instead of O(n^2) performance.
Co-authored-by: toofar <toofar@spalge.com>
This reverts commit fe1faa14b9, reversing
changes made to 04af4c657d.
For my setup with 310 quickmarks and 94 bookmarks, this makes qutebrowser hang
for around a minute on every keypress.
As shown in detail by toofar in dff25d10bc,
mesa 23.3 seems to trigger some new error messages.
On my setup locally, it also triggers the warning that's ignored here.
Why that's not the case on CI or for others is unclear to me, I gave up
trying to understand graphics stuff on Linux.
I'll just presume this is the same underlying cause, and refer to the
commit message of the commit above for more details.
In `pylint_checkers/setup.py` we define the package name as `qute_pylint`. When
setuptools was creating the egg (or wheel?) it was sanitizing the package name
replacing all problematic chars, including underscore, with a hyphen (-). So
the output of `pip freeze`, among other things, have the name with a hyphen:
`qute-pylint`. This was fixed in setuptools so underscores are no longer
sanitized: https://github.com/pypa/setuptools/issues/2522
Change the regex to include both just in case someone is running it with the
old setuptools or something. Also because I'm not able to reproduce the
hyphen version of the name locally, not sure why. Maybe it depends on your
python or importlib version too?
Mesa upgraded from 23.2.1-2 to 23.3.1-1 a couple of days ago. Since then
there has been some non-fatal InvalidLine errors in the CI jobs (eg
https://github.com/qutebrowser/qutebrowser/actions/runs/7281982007/job/19843511920)
Based on https://gitlab.freedesktop.org/mesa/mesa/-/issues/10293 I'm
assuming these log messages don't actually indicate errors and the tests
pass when they are ignore.
Weirdly, I'm only seeing these errors related to the
`tests/end2end/test_invocations.py::test_misconfigured_user_dirs` test,
I'm not sure why. It doesn't look much different from the ones around
it.
Possibly we could ignore these lines just for that test, or maybe play
round with it more to find out why it is different. But since the lines
are non fatal I'll just ignore them everywhere. I'm a little worried
about that because they are quite generic, but at least they are still
logged when they are ignored.
They might change these error logs messages to be warning log messages
based on that issue. But it'll probably still fail the tests since we
match on all log lines?
The "always try zink" change was introduced in https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/25640
It looks like there might be a `LIBGL_KOPPER_DISABLE` which may skip
this behaviour. Not I'm not sure, the commit message says:
> don't load non-sw zink without dri3 support
> this is going to be broken, so don't bother trying
> also add LIBGL_KOPPER_DRI2 so people can continue to footgun if they
> really really want to
I assume we aren't trying to run with non-sw zink but with non-zink sw?
idk
Maybe we should be setting force_software_rendering=software-opengl or
LIBGL_ALWAYS_SOFTWARE instead so that it never tries to use the vulkan
backed zink?
A few more completions will now match search terms in any order:
`:quickmark-*`, `:bookmark-*`, `:tab-take` and `:tab-select` (for the quick
and bookmark categories).
I would like to make merging PRs lower friction. One aspect of that for
me is having to think about where to add the changelog info, whether it
should go in an existing section, whether I should create a new section,
what the format changelog is supposed to be in. These questions are a
bit coupled with the decision of whether to backport a change or not.
Those aren't hard questions but I don't usually have a long stretch of
time for open source work. So making it so I don't have to make those
decisions at merge time makes it easier for me to fit that work into my
day. Previously it seemed to me that the norm was to only have a future
changelog entry for the next patch release. Occasionally I would merge
stuff and add it to the patch release changelog entry and then think
about how I would have made getting any security fixes out harder or how
it would have to be corrected at backport time. So this is kind of a
pre-commitment that yes, we are going to be merging stuff to main that
won't make it to the next release.
A lot, but not all, of the above rambling will also be mitigated by
adopting a fragment based changelog management system (#7101), because
that means that more of the stuff we have to worry about when merging is
only in the context of the PR. Eg just describe that the change does,
don't worry too much about where that description is going to end up.
Other follow up stuff we could do if norms are established or need
re-enforcing:
* update contributor docs to describe more of the branching strategy as
it applies to merging
* update contributor docs to describe backporting steps and philosophy
* link changelog entries to milestones?
I added isort!=5.13.0 in misc/requirements/requirements-pylint.txt and
ran `python3 scripts/dev/recompile_requirements.py pylint` in `docker
run -v `pwd`:/work -w /work -it python:3.8-bookworm bash` based off of
main.
Then reverted the relevant part of the update dependencies commit with
`git show HEAD misc/requirements/requirements-pylint.txt | patch -R -p
1` and manually updated the two deps from the local update run :)
diff --git i/misc/requirements/requirements-pylint.txt w/misc/requirements/requirements-pylint.txt
index 80319adc03a7..1c8fa33e623a 100644
--- i/misc/requirements/requirements-pylint.txt
+++ w/misc/requirements/requirements-pylint.txt
@@ -11,7 +11,7 @@ idna==3.6
isort==5.12.0
mccabe==0.7.0
pefile==2023.2.7
-platformdirs==4.0.0
+platformdirs==4.1.0
pycparser==2.21
PyJWT==2.8.0
pylint==3.0.2
@@ -21,6 +21,6 @@ requests==2.31.0
six==1.16.0
tomli==2.0.1
tomlkit==0.12.3
-typing_extensions==4.8.0
+typing_extensions==4.9.0
uritemplate==4.1.1
# urllib3==2.1.0
ref: https://github.com/qutebrowser/qutebrowser/pull/8028#issuecomment-1849363267
Almost 7 years ago, it was observed that hiding the status bar causes some
websites being scrolled to the top: #2236.
Back then, it never really was clear why this happens. However, with the v3.0.0
release, we had a regression causing the same thing to happen when leaving
prompt mode: #7885.
Thanks to "git bisect", the culprit was found to be 8e152aa, "Don't give
keyboard focus to tab bar", which was a fix for #7820. However, it still wasn't
clear why this phenomenon happens.
What made things clearer to me was a combination of debugging and an old comment
by pevu: https://github.com/qutebrowser/qutebrowser/issues/2236#issuecomment-337882102
> Chromium-browser has the same issue. When you open lipsum.com, scroll down,
> then focus the location bar (url box), then press Tab, it will jump to the
> top of the page and focus the first link. This doesn't happen when you
> switch focus using the mouse.
>
> It seems to be an issue of how the view containing the website is focused
> when all qutebrowser ui elements disappear.
And indeed, tabbing into the web contents from the UI elements via the tab key
in Chromium causes the website to start at the top, presumably as an
accessibility feature?
Essentially, this is also what happens in qutebrowser when an UI element is
hidden while it still has focus: In QWidget::hide() (or, rather,
QWidgetPrivate::hide_helper()), Qt moves the focus to the next widget by
calling focusPrevNextChild(true):
https://github.com/qt/qtbase/blob/v6.6.1/src/widgets/kernel/qwidget.cpp#L8259-L8271
And apparently, focusPrevNextChild() basically does the same thing as pressing
the tab key, to the point that there is some code in Qt Declarative actually
making tab keypresses out of it (which I'm still not sure is related, or maybe
just the cause of #4579):
https://github.com/qt/qtdeclarative/blob/v6.6.1/src/quickwidgets/qquickwidget.cpp#L1415-L1429
Some debugging confirms that this is exactly what happening:
1) We hide the status bar (or prompt) which has keyboard focus
2) Qt focuses the web view, which triggers the Chromium feature (?) scrolling it
to the very top.
3) Only then, in TabbedBrowser.on_mod_left(), we noticed that the command or
prompt mode was left, and reassign focus to the web view properly.
In step 2), before this change, Qt happened to focus the tab bar (before we set
the focus manually to the web contents), and thus this didn't happen.
Not sure why it didn't focus the tab bar when we hid the status bar (maybe
because how our widget hierarchy works with TabbedBrowser?).
Python stacktrace of hiding prompt:
Traceback (most recent call first):
<built-in method hide of DownloadFilenamePrompt object at remote 0x7fffb8bc65f0>
File ".../qutebrowser/mainwindow/prompt.py", line 204, in _on_mode_left
self.show_prompts.emit(None)
File ".../qutebrowser/keyinput/modeman.py", line 434, in leave
self.left.emit(mode, self.mode, self._win_id)
File ".../qutebrowser/keyinput/modeman.py", line 445, in mode_leave
self.leave(self.mode, 'leave current')
C++ stacktrace, with the focus change presumably being passed of to Chromium
here: https://github.com/qt/qtwebengine/blob/dev/src/core/render_widget_host_view_qt_delegate_client.cpp#L714#0 QtWebEngineCore::RenderWidgetHostViewQtDelegateClient::handleFocusEvent(QFocusEvent*) () at /usr/src/debug/qt6-webengine/qtwebengine-everywhere-src-6.6.0/src/core/render_widget_host_view_qt_delegate_client.cpp:708
#1 QtWebEngineCore::RenderWidgetHostViewQtDelegateClient::handleFocusEvent(QFocusEvent*) () at /usr/src/debug/qt6-webengine/qtwebengine-everywhere-src-6.6.0/src/core/render_widget_host_view_qt_delegate_client.cpp:705
#2 0x00007fffe5fea70c in QtWebEngineCore::RenderWidgetHostViewQtDelegateClient::forwardEvent(QEvent*) () at /usr/src/debug/qt6-webengine/qtwebengine-everywhere-src-6.6.0/src/core/render_widget_host_view_qt_delegate_client.cpp:300
#3 0x00007fffe4dd5c79 in QQuickItem::event(QEvent*) (this=0x555556b6cd20, ev=0x7fffffffa320) at /usr/src/debug/qt6-declarative/qtdeclarative-everywhere-src-6.6.0/src/quick/items/qquickitem.cpp:8871
#4 0x00007ffff1f7318b in QApplicationPrivate::notify_helper(QObject*, QEvent*) (this=<optimized out>, receiver=0x555556b6cd20, e=0x7fffffffa320)
at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qapplication.cpp:3290
#5 0x00007ffff295e4a7 in () at /usr/lib/python3.11/site-packages/PyQt6/QtWidgets.abi3.so
#6 0x00007ffff59626d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) (receiver=0x555556b6cd20, event=0x7fffffffa320) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/corelib/kernel/qcoreapplication.cpp:1118
#7 0x00007ffff596271d in QCoreApplication::sendEvent(QObject*, QEvent*) (receiver=<optimized out>, event=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/corelib/kernel/qcoreapplication.cpp:1536
#8 0x00007fffe4f33f15 in QQuickDeliveryAgentPrivate::setFocusInScope(QQuickItem*, QQuickItem*, Qt::FocusReason, QFlags<QQuickDeliveryAgentPrivate::FocusOption>)
(this=<optimized out>, scope=<optimized out>, item=<optimized out>, reason=<optimized out>, options=...) at /usr/src/debug/qt6-declarative/qtdeclarative-everywhere-src-6.6.0/src/quick/util/qquickdeliveryagent.cpp:439
#9 0x00007fffe4dd348a in QQuickItem::setFocus(bool, Qt::FocusReason) (this=0x555556b724d0, focus=<optimized out>, reason=Qt::TabFocusReason) at /usr/include/qt6/QtCore/qflags.h:73
#10 0x00007fffe4e7239b in QQuickWindow::focusInEvent(QFocusEvent*) (this=<optimized out>, ev=<optimized out>) at /usr/src/debug/qt6-declarative/qtdeclarative-everywhere-src-6.6.0/src/quick/items/qquickwindow.cpp:231
#11 0x00007ffff1fc3a05 in QWidget::event(QEvent*) (this=0x555556457b50, event=0x7fffffffa770) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qwidget.cpp:9111
#12 0x00007ffff1f7318b in QApplicationPrivate::notify_helper(QObject*, QEvent*) (this=<optimized out>, receiver=0x555556457b50, e=0x7fffffffa770)
at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qapplication.cpp:3290
#13 0x00007ffff295e4a7 in () at /usr/lib/python3.11/site-packages/PyQt6/QtWidgets.abi3.so
#14 0x00007ffff59626d8 in QCoreApplication::notifyInternal2(QObject*, QEvent*) (receiver=0x555556457b50, event=0x7fffffffa770) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/corelib/kernel/qcoreapplication.cpp:1118
#15 0x00007ffff596271d in QCoreApplication::sendEvent(QObject*, QEvent*) (receiver=<optimized out>, event=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/corelib/kernel/qcoreapplication.cpp:1536
#16 0x00007ffff1f7f1b2 in QApplicationPrivate::setFocusWidget(QWidget*, Qt::FocusReason) (focus=0x555556457b50, reason=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qapplication.cpp:1538
#17 0x00007ffff1fca29d in QWidget::setFocus(Qt::FocusReason) (this=0x555556b1ceb0, reason=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qwidget.cpp:6580
#18 0x00007ffff1fb4f1b in QWidget::focusNextPrevChild(bool) (this=<optimized out>, next=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qwidget.cpp:6844
#19 0x00007ffff298d0ac in () at /usr/lib/python3.11/site-packages/PyQt6/QtWidgets.abi3.so
#20 0x00007ffff298d0ac in () at /usr/lib/python3.11/site-packages/PyQt6/QtWidgets.abi3.so
#21 0x00007ffff298d0ac in () at /usr/lib/python3.11/site-packages/PyQt6/QtWidgets.abi3.so
#22 0x00007ffff1fbdb76 in QWidgetPrivate::hide_helper() (this=this@entry=0x55555646a360) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qwidget.cpp:8271
#23 0x00007ffff1fbf158 in QWidgetPrivate::setVisible(bool) (this=0x55555646a360, visible=<optimized out>) at /usr/src/debug/qt6-base/qtbase-everywhere-src-6.6.0/src/widgets/kernel/qwidget.cpp:8447
[...]
We fix this problem by explicitly handling focus before hiding the UI elements.
This is done with a new TabbedBrowser.on_release_focus() slot, which is bound to
signals emitted just before things are hidden: The existing Command.hide_cmd()
for the status bar, and a new release_focus() signal for prompts.
Additionally, we make sure to not double-handle hiding in the statusbar
code when it's already handled separately for comamnd mode.
Unfortunately, no tests for this, as application window focus is required to
reproduce the issue. In theory, a test in scroll.feature could be added though,
which loads simple.html, scrolls down, shows/hides a prompt or the status bar,
and then checks the vertical scroll position is != 0.
Fixes#2236Fixes#7885
Makes the lambdas more flexible, e.g. mapping a single key to a different flag depending on Chromium version. Ended up being unneeded for reading from canvas flag, but still useful.
Implemented as a separate setting in Chromium, but exposed to qutebrowser users
as a value for `policy.images`, as it's a simple toggle that does not have any
effect when `policy.images` is not set to `smart` anyways.
To support this, the _Settings.mapping value now supports None values,
which leads to _Setting.chromium_tuple to return None, which means that
no switch is added in this case.
See #7646
Given that the two branches share rather little, it seems simpler to separate the two tests. Also use monkeypatch, since we don't use any of unittest.mock's complexity
This was fixed up in https://github.com/qutebrowser/qutebrowser/pull/8006
after a mypy update caused us to examine the typing.AnyStr thing a bit more.
But both overloads got set to have a `str` return type, so the possible
bytes return type got lost. Mypy didn't pick that up because `binary=True` is
only used in `qutebrowser/browser/webkit/cookies.py` which probably isn't
being type checked.
I had to remove the default from the binary arg of the bytes version (the ` =
...` bit) because if both overloads have the kwarg as optional mypy doesn't
know which to match a call with just one positional argument against. Eg
`savefile_open('some_file')` would match both.
I checked with reveal_type:
diff --git i/qutebrowser/utils/qtutils.py w/qutebrowser/utils/qtutils.py
index 89175ca4ee60..5b86e441a1bc 100644
--- i/qutebrowser/utils/qtutils.py
+++ w/qutebrowser/utils/qtutils.py
@@ -290,6 +290,20 @@ def savefile_open(
raise QtOSError(f, msg="Commit failed!")
+def test_savefile_types() -> None:
+ from typing_extensions import reveal_type
+
+ maybe_str_default = savefile_open("/tmp/string_file")
+ # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.str]]"
+ reveal_type(maybe_str_default)
+ maybe_str_explicit = savefile_open("/tmp/string_file", binary=False)
+ # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.str]]"
+ reveal_type(maybe_str_explicit)
+ maybe_bytes = savefile_open("/tmp/bytes_file", binary=True)
+ # note: Revealed type is "contextlib._GeneratorContextManager[typing.IO[builtins.bytes]]"
+ reveal_type(maybe_bytes)
+
+
def qcolor_to_qsscolor(c: QColor) -> str:
"""Convert a QColor to a string that can be used in a QStyleSheet."""
ensure_valid(c)
This is just to get the CI to re-trigger.
Apparently the checkout action doesn't actually checkout the branch
being proposed for merge but checks out an actual merge commit. So if
you push a PR while main is broken it'll say changes from main are on
your branch. That's a little unexpected.
main is fixed now and I tried re-running the CI jobs from the web UI but
they are still failing with the same errors. Hence this merge of main
just to get a change on the branch. I could have gone and found a typo
to fix instead. I know I've left enough of them around.
ref: https://github.com/actions/checkout/issues/881
We thought #7489 would be fixed on chrome 112 but it appears to still be
an issue. As discussed on the issue, it's not clear how many workflows
would be affected by accelerated 2d canvas and we don't have enough data
to detect the affected graphics configurations. So lets just disable
accelerated 2d canvas by default and users who want it turned on can do
so via the setting.
If some major use case pops up to enable this by default where possible
we can revisit and think of more nuanced feature detection.
I've kept the handling of callable values of `_WEBENGINE_SETTINGS`
because I don't like have to have something like
`--disable-accelerated-2d-canvas` written in code twice. The setting
above this one could probably be changed to use it too.
pdfjs.get_pdf_basename() returned None, causing in a TypeError. Instead of
throwing mocker.patch at it, fix the underlying issue.
Given we made the same mistake in three places:
- :version
- test_real_file for PDF.js
- is_available() in pdfjs.py (calls the function but doesn't use the result, so
is a nop now, even if PDF.js wasn't found)
...evidently we need to change the API so it still raises an exception if no
PDF.js is available.
Amends 0144ae3576.
Otherwise, doing :restart fails because it tries to copy quirk dir to quirk dir.
QtWebEngine reads the env var in resourcePath() in
src/core/web_engine_library_info.cpp. That only seems to be called from
WebEngineLibraryInfo::getPath(), and that in turn gets called from
ResourceBundle::LoadCommonResources() in src/core/resource_bundle_qt.cpp.
Finally, that only seems to be called during Chromium initialization, so it
seems alright for us to unset this as soon as we initialized the first profile.
It also seems to work fine in my testing on Google Meet, and indeed fixes
:restart.
Now we aren't using it for the patching we don't need it in this function.
Move the `get_pdfjs_basename()` call into `is_available()` too since having to
call two methods to check everything is there sounds like a pain.
We are setting window.Response to undefined to cause certain versions of
pdf.js to detect that fetch() isn't available and to fall back to XHRs. In
more recent versions of pdf.js the HTML string we were matching against has
changed, it has a 'type="module"' attribute now, so we need to change the
pattern we match against.
I don't think it matters where we put this mocking though, so I've just put it
after '<head>', which should hopefully be less fragile than matching against a
'<script>' element.
Note that this workaround isn't even relevant for the latest versions of
pdf.js (4+). It doesn't seem to use a check on `window.Response` as part of
fetch feature detection anymore (instead it uses fetch() for https? urls and
XHR otherwise). But I figured it was better to have the workaround applying
consistently, and not having an effect, vs having the workaround silently fail
to apply. Don't have a strong opinion on it though.
The other way to fix this would have been something like:
attrs = ""
if pdfjs_name.endswith(".mjs"):
attrs = ' type="module"'
pdfjs_script = f'<script src="../build/{html.escape(pdfjs_name)}"{attrs}></script>'
Now that pdf.js could be shipped with either js or mjs file extensions we
shouldn't hardcode the filename. Call the function for detecting the filename
instead. And make it public.
On CI now the sandbox test is failing on windows when we pop the header
line with an index error. It looks like the only line present on the
page is "Sandbox Status". It was working on CI the other day! Grrr
Hopefully it's a timing issue and the JS just hasn't finished running
yet? Not sure if just loading it again is the most reliable. Ideally we
would be listening for some event...
Pretty low effort but if this makes the test stop being flaky and we
don't have to look at it again that's fine with me.
Contrary to what I thought at the time when initially writing this,
typing.AnyStr isn't just an alias for IO[str] | IO[bytes], but is actually a
TypeVar. As per the Python docs, it should be used when there are *multiple*
places where the types need to match:
def concat(a: AnyStr, b: AnyStr) -> AnyStr:
return a + b
What we do instead is somewhat akin to "def fun() -> T:", which mypy already
comments on:
error: A function returning TypeVar should receive at least one argument
containing the same TypeVar [type-var]
def t() -> T:
Not quite sure why it doesn't in this case, or why it now raises an additional
error (possibly the new inferrence code or something?). Either way, with this
commit the annotations are now more correctly using Union[IO[str], IO[bytes]],
including typing.Literal overloads so that mypy actually knows what specific
type will be returned by a call.
PyQt 6.6 has been out for a while. Git uses on arch are already using
it. Likely our next pyinstaller release will be using it. This change
adds it to our test matrix, beyond the arch docker tests.
* Removing -dev tag from python 3.12 job
* Update ubuntu python 3.11 and 3.12 tests to use PyQt6.6
* Update macOS and windows tests to use PyQt6.6
* Allow running the nightly CI job on any branch, to get a pyinstaller
build binary from your own branch
Closes: #7989
We think that at some point pytest added color codes to the summary
output which broke these matcher regex. It looks like there is an issue
about stripping color codes here: https://github.com/actions/runner/issues/2341
I've added wildcard (or no-space) elements instead of trying to match
the color codes exactly (eg `\033\[31m` etc) because 1) that's not very
readable 2) I was having trouble getting that to work with egrep.
The goal is to match the target strings but not to match then later in
the line as part of a test log or whatever. For the '= short test
summary =' that should be pretty unique. For the ERROR|FAILED I reckon
that could be a bit more common. I'm just matching with any mount of
non-space characters because I reckon a space will crop up pretty early
in any line where ERROR isn't the first work.
I think this new matching will only apply to new or rebased PRs after it
is landed.
ref: https://github.com/qutebrowser/qutebrowser/issues/5390#issuecomment-1817503702
In the linux branch when it was doing:
header, *lines, empty, result = text.split("\n")
assert not empty
It was complaining that "empty" was "}", because the windows sandbox
page has JSON at the bottom now. The whole things looks to have changed
completely. I'm actually surprised it was working before, why would it
have been saying seccomp was enabled on windows?
Anyway, I did the debug-dump-text --plain that quteproc is doing in a VM
and tested this with sandboxing off an on. No idea how stable that will
be!
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
The nightly jobs have a `workflow_dispatch` action, which means you can
kick the job off on any branch. But the build steps has the branch to
build on hardcoded. I would like to be able to build windows and mac
builds without having a local build environment setup.
The docs for the checkout action says it default to the main branch,
so the scheduled actions should keep working fine. But now we'll be able
to create builds off of other branches too.
docs: https://github.com/actions/checkout#usage
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
It looks like our last release builds were done with python 3.11 and
PyQt 6.5.3. I'm expecting that since PyQt6.6 is out now our next release
will be on 6.6. So lets update the CI to match.
Questions:
* what about python12? I don't think there is a benefit to updating to
that, so lets leave it.
* what about pyqt6.5? Do we care about testing that? Maybe for homebrew
users? We aren't providing new builds with an old Qt right?
last release builds: https://github.com/qutebrowser/qutebrowser/actions/runs/6578864884
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
I'm not sure if we need a py3.11 pyqt6.5 variant or a py3.10 pyqt6.6
one? Those might well be combinations that people have (debian has 3.11
and 6.5 at the moment) but how much coverage do we need?
ref: https://github.com/qutebrowser/qutebrowser/issues/7989
I believe we are being afflicted by this issue: https://github.com/python/mypy/issues/16451
Although I'm not 100% sure because there is a lot going on in this
function and I haven't managed to grok it.
The mypy 1.7 release [notes][1.7] say you can disable the new type
inference by running `tox -e mypy-pyqt6 -- --old-type-inference` and
indeed mypy passes with that. So either our type hints are incorrect or
we are hitting a bug. Considering the inferred type hint has a `Never`
in it I'm leading toward it being a bug. So I'll bump the mypy version
down and hopefully next week the issue will be resolved.
The mypy output before this commit was:
mypy-pyqt6: commands[0]> .tox/mypy-pyqt6/bin/python -m mypy --always-true=USE_PYQT6 --always-false=USE_PYQT5 --always-false=USE_PYSIDE6 --always-false=IS_QT5 --always-true=IS_QT6 --always-true=IS_PYQT --always-false=IS_PYSIDE qutebrowser
qutebrowser/utils/qtutils.py:239: error: Argument 1 to "contextmanager" has incompatible type
"Callable[[str, bool, str], Iterator[IO[AnyStr]]]"; expected "Callable[[str, bool, str], Iterator[IO[Never]]]" [arg-type]
@contextlib.contextmanager
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LineParser":
qutebrowser/misc/lineparser.py:168: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/misc/lineparser.py: note: In member "save" of class "LimitLineParser":
qutebrowser/misc/lineparser.py:226: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._configfile, self._binary) as f:
^
qutebrowser/config/configfiles.py: note: In member "_save" of class "YamlConfig":
qutebrowser/config/configfiles.py:292: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(self._filename) as f:
^
qutebrowser/misc/sessions.py: note: In member "save" of class "SessionManager":
qutebrowser/misc/sessions.py:343: error: Need type annotation for "f" [var-annotated]
with qtutils.savefile_open(path) as f:
[1.7]: https://mypy-lang.blogspot.com/2023/11/mypy-17-released.html
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be None, we've been relying on them
being set for long enough.
`qWebEngineVersion()` and `qWebEngineChromiumVersion()` now are typed as
returning `Optional[str]`. In `from_api()` we can handle the
`chromium_version` being null, so pass that through, but we are depending on
the `qtwe_version` being set, so add an assert there.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be none though so I'm adding
some low effort null checking in here.
For the changes in `_SettingsWrapper` I'm not actually sure this is the
best way to be resolving this. mypy was complaining that `settings()` might be
None. How does asserting on the profile help? idk but that seems to make it
happy. Even more weirdly, none of these getters I changed seemed to be used
anywhere apart from tests. Except for getAttribute, that's used in webkit code.
Also the whole thing is checking the global default profile, generally
settings should be checked on the profile on the tab. So I think this might
just be some old code? idk, I just want to make mypy happy.
For the `init_user_agent()` change that was complaining about the profile
maybe being None.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
The type of the two list arguments of chooseFiles() have changed from
`Iterable[str]` to `Iterable[Optional[str]]`.
I'm not sure it makes much sense to have individual list elements as
None. That seems like a pretty unlikely case. So we could just put an
ignore comment somewhere. I've added another copy of the lists, with
longer names, to strip any hypothetical Nones out. This allows us to be
backwards compatible with the old type hints (and the current Qt5 ones),
since that doesn't have the Optional part in the signature.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
With PyQt6-WebEngine 6.6.0 some pointer return types are now wrapped in
Optionals[]. In practice they should never be none though so I'm sprinkling
some low effort null checking in here.
Another alternative is to move the inspector classes to be based off, and to
work with, our overridden child classes of the view and page. But that's a
whole other piece of work, we might have to deal with signals and such meant
for web views that we don't want with the inspector. (On the other hand it
might actually be good. The inspector sometimes is surprising in how it acts
differently from the main views.)
There's a bit later on where I changed a local variable name from
`inspector_page` to `new_page`. The `inspector_page` variable is used later
and the type checker was complaining.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
Changing the type to be our overridden implementation of the view means
we can push more common code down into the view class and make use of
overridden methods more consistently. In this case I'm adding some type
checking to some of the getters on the view.
`_set_widget()` should be being called from the concrete child tab
implementations. So the widget should always be our overridden version of
the Qt tab implementations.
Also change `_set_widget()` to use the common typevar.
Also change the _widget type in WebEngineTab to match what it is
actually being set to and to match all the _widgets in the other
concrete classes in that file.
ref: https://github.com/qutebrowser/qutebrowser/pull/7990
Seems mypy and I agree on something. It thinks that pathlib is abusing
the division operation and making python a less intuitive
language. Not sure why it is complaining though really, doing `Path /
"one" / "two"` seems to work fine in practice, and its on the pathlib
documentation page.
I was seeing this error locally and on CI
qutebrowser/misc/pakjoy.py:155: error: Unsupported left operand type for / ("str") [operator]
resources_dir /= "lib" / "QtWebEngineCore.framework" / "Resources"
On CI I'm seeing:
No such file or directory: '/Users/runner/work/qutebrowser/qutebrowser/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/resources'
Downloading the PyQt6_WebEngine_Qt6 wheel for mac from PyPI I can see the pak
file is at:
./PyQt6/Qt6/lib/QtWebEngineCore.framework/Resources/qtwebengine_resources.pak
And on a qutebrowser install made by pyinstaller it is at (symlinks in curly
braces):
/Applications/qutebrowser.app/Contents/{Resources,Frameworks}/PyQt6/Qt6/lib/QtWebEngineCore.framework/{Resources,Versions/A/Resources}/qtwebengine_resources.pak
And the Qt data path for reference is:
/Applications/qutebrowser.app/Contents/Frameworks/PyQt6/Qt6
So it looks like essentially we need to add a "lib/QtWebEngineCore.framework"
in there and capitalise "Resources".
A bit annoying to have the special case and hardocde paths like this. But it
should be pretty stable? I had a look at importlib_resources to try to see if
it could fine this stuff for us but I don't think it can.
I've checked with the pyinstaller windows builds and with the windows wheel
from PyPI and neither seem to need any special handling.
There is one test which does does the whole run through with the real
resources file from the Qt install, patches is to the cache dir, reads
it back and checks the json. All the other tests either use constructed
pak files or stop on earlier error cases.
For testing the actual parsing of the pak files I threw together a quick
factor method to make them.
I went back and forth over the parse code and looked for more error
cases, but it looks pretty solid to me
The "doesn't exist at expected location" error message is a bit
misleading, it actually prints the location in the directory we copied
stuff to instead of the source one. Oh well, it's good enough.
Fixes lint: shadows variables, docstrings, unused attribute
Move resource dir copying to its own method:
* will probably help with unit tests
* allows for cleaner overriding of what file to patch (instead of a big
nested block)
Adds a sneaky `assert` statement in there because the pak file should
definitely exist at that location. Although it not existing shouldn't
really be fatal...
Maintains the thing in `__main__`, although I'm not sure if we need to
keep that?
The only affected version is 6.6.0. I'm passing `avoid_init` because
this patching is done right before the user agent is parsed (which only
happens on Qt5).
It's been proposed that we don't need to do this patching for users
running on distros that backported the fix to their webengine. Which I
think means that we would only do the patching on our bundled installer
version for window and mac. But I'm not sure I agree with that. I would
rather not have had to re-compile the fix into my build, and I don't
know what other distributors position on backporting patches is either.
For example I'm on debian, if they had 6.6.0, would they have backported
this fix? It looks like they've only got build related patches on the
current version for example: https://sources.debian.org/patches/qt6-webengine/6.4.2-final+dfsg-12/
And there's the flatpak version etc.
Anyway, if we want to add that check in too I think it would look like:
if not hasattr(sys, 'frozen'):
# Assume if we aren't frozen we are running against a patched Qt
# version. So only apply this quirk in our bundled installers.
return
We need to set the environment variable before webengine reads the
resources file. It could be unhelpful to do the patching before the IPC
/ open-in-existing-instance stuff, because that could lead to removing
the resources file the running instance is using, any later than that is
just down to developer preference I think. I chose to move it as late as
possible for now, just for clarity on where in the lifecycle it's
necessary. (This also means we can skip the separate backend check, but
that is pretty minor.)
I moved the patching later and later at init and verified that if it is
done before the profiles are initialized it successfully prevents the
crash. If done after the profiles are initialized we still crash. I do
remember looking at the profile initialization when trying to disable
extensions so that makes sense.
It all just works!
Changes from the `__main__` implementation down below:
* copy the whole resources directory instead of just the one file:
looking at the implementation around QTWEBENGINE_RESOURCES_PATH it
uses the main resources file to find the one directory and then loads
the other resources from that dir. So I'm assuming it was crashing
because it couldn't find the other resources. Stack trace was:
#1 0x00007fffe95f1dca in content::ContentMainRunnerImpl::Initialize(content::ContentMainParams) ()
from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6
#2 0x00007fffe628cf09 in QtWebEngineCore::WebEngineContext::WebEngineContext() ()
from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6
#3 0x00007fffe628dfa4 in QtWebEngineCore::WebEngineContext::current() ()
from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6
#4 0x00000210b845c780 in ?? ()
#5 0x00000210b845c780 in ?? ()
#6 0x00007fffffffd480 in ?? ()
#7 0x00007fffe62391bb in QtWebEngineCore::ProfileAdapter::ProfileAdapter(QString const&) ()
from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6
* write stuff to our cache dir, not tmp ;)
I haven't actually checked this fixes an affected version of Qt (need to
rebuild or get from riverbank pypi). But I unpacked the pak file and it
has the right fake URL in it.
https://pylint.readthedocs.io/en/stable/user_guide/messages/refactor/prefer-typing-namedtuple.html
Says to using the class based typing.NamedTuple instead of
collections.namedtuple, which just constructs a class based off of
strings.
Here we are creating a dynamic class based on the fields in the SQL result.
It's only called once for a query, so I don't think speed is an issue.
Also it's not used by the completion, looks like just :history
With pylint 3 there is a new option: https://pylint.readthedocs.io/en/latest/user_guide/messages/convention/use-implicit-booleaness-not-comparison-to-zero.html
It's disabled by default but we enable all warnings and disabled them as
desired.
This one is of the opinion that:
if x == 0:
is bad and
if x:
is good.
I feel that the first one (x == 0) is more clear. We aren't checking for
truthiness here, we are checking for a literal value, its very
intentional.
One might argue that being precious about making the type here is
redundant in current year with type checking tooling and all that. But
there are like a hundred of these checks in the code base so it seems a
well established pattern anyhow.
In summary, the new warning doesn't have a very strong use case and we
would prefer to stick with out established pattern.
Slack now requires chrome 112+. At least one user says it still works
with 108 based. Although they did just do a UI refresh so I wouldn't be
surprised if something was broken with older versions.
Note that I'm not 100% sure that slack is actually doing a strict check
for 112, but based on their prior behaviour I assume so (they are
definitely checking for >99 so our old quirk is unhelpful at this
point).
Since I've told people to add ua-slack to their disabled quirks, should
I change the name of it now to make sure it gets re-enabled? Eh, they
can manage their own quirks.
We only want to be faking a newer chrome version, if we are already on a
newer one, we don't want to be faking an older one!
Another scenario is that we might want to fake a perpetually newer
version for some site. But that seems like a very problematic scenario
and I hope that never comes up.
Since the bug was fixed in qtbase it shouldn't matter what version of
PyQt (or webengine for that matter) you are running against. So pass
`compiled=False`.
Also expand the docstring of `check_version()` to explain a little more
what the `compiled` kwarg does. Based on a discussion here:
https://github.com/qutebrowser/qutebrowser/pull/7933#issuecomment-1732400960
What's still not clear to me is when the runtime and compiled Qt
versions can differ. I always have to rebuild PyQt when I switch Qt
versions, but maybe I'm doing something wrong there.
TODO: configure the linter to tell people to do this.
As an aside, I'm not sure I agree that this is a good idea. It seems the
f-string interpolation does have a cost when you are not logging at that
level, and we do support disabling the ram logger. But we can change it
with automation anyway.
This avoids having to mess about with static methods. And makes the test a bit
clearer as we aren't passing a class in place of an instance anymore.
Hopefully I put it in the right place. It's above the class where it is used.
Should it be at the top of the file? Bottom of the file? IDK
The right hand side is turned into a list because `list + set` isn't
supported by python for some reason (why? Just make it a list).
The left hand side is turned into a list because Iterable doesn't
support + (why?). Possible I could change it to Sequence, but meh.
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.
As Chromium 99.0.4785.0 (translates to Qt 6.4) renamed `TextBrightnessThreshold` to
`ForegroundBrightnessThreshold`, we need to reflect that in our mapping of the
qutebrowser option `colors.webpage.darkmode.threshold.text` to the Chromium setting.
By adding a new method (`_Definition.copy_replace_setting()`) we can introduce a new
`_Definition` for Qt 6.4 based on the preceding one for Qt 6.3, which contains the
renamed setting.
References:
* https://chromium-review.googlesource.com/c/chromium/src/+/3344100
* https://chromium-review.googlesource.com/c/chromium/src/+/3226389
I previously changed the assertion to be a subset match, to deal with a qt arg
being added based on the Qt version the tests were being run on. I didn't
notice this fixture that can set setting to avoid that dynamic-ness instead.
I added all three args to this function in an effort to allow for generic
feature detection, because other places further up in the code where using all
these variables for that. But I didn't look at how the stuff I
was passing in could be used. I can see `special_flags` has essentially already
been consumed. `namespace` is used for all kinds of stuff, there could
theoretically be a pretty simple mapping between the CLI arg and a setting
webengine setting but the only examples of that so for are the special flags
ones and debug flags, which are already generic in their own way. So if we've
gone this long without it we probably don't need it.
I would like to be able to disable this workound for new enough chromium
versions (we still need to test that chrome 111 will be fixed, but we
can always bump it up later).
I also wanted to use the declarative mapping `_WEBENGINE_SETTINGS` instead
of adding a new conditional in `_qtwebengine_args`, because that just
seems nicer.
So I changed `_WEBENGINE_SETTINGS` so that the value could also be a
function. The idea is that the function returns on of the other values
based on some feature detection. This also required passing down the
args being used for feature detection in the calling method already.
I feel like that dict is being a bit abused here and if the entries
could be turned into objects with a bit more consistency it might be
nice. But this isn't too bad, right?
Had to change the `test_qt_args` test to be a superset test instead of
exact match because the new `--disable-accelerated-2d-canvas` arg was
showing up in the results based on whatever Qt version you happen to be
running the tests on.
Ideally we would only enable this if we detected Intel graphics. But
that kind of feature detection is difficult to do cross platform. It's
easy to do with Qt (version.py) ... but it needs a QApplication and we
are trying to figure out what CLI args to pass to Qt, so we can't do
that.
So just disable it for everyone for now.
TODO:
* tests
* change to tri-state option, auto by default and make auto look at the
webengine version and on for Qt6 < 6.7.0
We replace the `(testdata)` placeholder with `testutils.abs_datapath()` in a few end2end
tests. So far we only needed to replace `(testdata)` with an OS path (e.g. in
`tests/end2end/features/spawn.feature`). By introducing `(testdata)` to an end2end test
in `tests/end2end/features/hints.feature`, we required a new use case: replacing
`(testdata)` as part of a valid file:// URI.
```
When I open file://(testdata)/some/file.txt
```
Replacing `(testdata)` in above BDD step with a plain OS path resulted in invalid URIs,
e.g. for the path "C:\\Users" above BDD step results in this invalid URI:
```
When I open file://C:\Users/some/file.txt
```
We deal with this by first isolating the `(testdata)` substitution in a single place.
Having `(testdata)` substitutions in a single place, we simply special-case the substitution
of file:// paths, which will be part of a URI.
Successful substitution for above BDD step looks like the following:
```
When I open file:///C:/Users/some/file.txt
```
As of Qt 6 navigating from local to remote origins requires user interaction. This broke
following links to remote origins via hints from a local file (e.g. `bookmarks.html`),
because hints use JavaScript to open links by default.
Example:
1. Have a local `bookmarks.html`:
```html
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>My bookmarks</title>
</head>
<body>
<a href="https://example.com/" id="link">some bookmark</a>
</body>
</html>
```
2. Open that local `bookmarks.html` in qutebrowser (e.g. `:open path/to/bookmarks.html`)
3. Start hinting
4. Follow a link to a bookmark (e.g. https://example.com/)
5. Instead of the link opening, be presented with `Your internet access is blocked`
error
To fix this, we simply force a user interaction for all hints on file:// URLs (like we
did for `qute://` URLs in 8defe1ae44).
We skip the end2end test for webkit, because webkit does not support URLSearchParams[1]
used in `tests/end2end/data/hints/link_inject.html`
[1] https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
Given the following scenario:
```
When I open file://(testdata)/some/file.html
Then file://(testdata)/some/file.html should be loaded
```
If the end2end data directory is not normalized, the scenario fails because we try to
compare
```
file:///home/palbrecht/dev/qutebrowser/tests/helpers/../end2end/data/hints/link_inject.html?port=50…
```
to
```
file:///home/palbrecht/dev/qutebrowser/tests/end2end/data/hints/link_inject.html?port=50…
```
Normalizing the path resolves the `..` and fixes the issue.
When handling counts during keyparsing we convert the count string to an integer. If the
count is too high (i.e. the count string has too many digits), we run into Python's
integer string conversion length limit[1]:
```
ValueError: Exceeds the limit (4300 digits) for integer string conversion: value has
4301 digits; use sys.set_int_max_str_digits() to increase the limit
```
Instead of blowing up with an exception, we now handle this more gracefully by showing
an error message.
Reproducer:
```
$ qutebrowser --temp-basedir ":later 500 fake-key -g $(printf '1%.0s' {1..4301})j"
```
**NOTE:**
I had to rename `_debug_log()`'s `message` argument to `msg`, because pylint yelled at
me for redefined-outer-name[2].
[1] https://docs.python.org/3/library/stdtypes.html#integer-string-conversion-length-limitation
[2] https://pylint.readthedocs.io/en/stable/user_guide/messages/warning/redefined-outer-name.html
This is required to use BDD steps like the following:
```
When I open file://path/to/file.html
```
If we don't treat `file:` as a special scheme, we implicitly convert it to an invalid
URL:
```
http://localhost:48595/file:///path/to/file.html
```
1. run jseval in main world: the script adds an element that calls the
`restore_video` function. This was failing with a "not found" message
on webengine, presumably because the dom click handler runs in the
main world and the function was over in the jseval world. The the
script predates webengine which is the backend that implements the
worlds.
2. remove a console log message, seems to be just noise and easy enough
to add back later
3. remove href attribute of the restore video link: this seemed to be
causing the `restore_video` method to be called twice. The second
time with `this` as the global Window object, which was causing an
error because that has a null `parentNode` attribute.
4. added the `cursor: pointer` style that was needed since the element
didn't have an href anymore
5. change the mpv flags `--terminal` -> `--quiet`. This means we get
error messages (eg from yt-dlp) in error logs and in the `:process`
page now. It can get a bit spammy though if you are running from a
terminal. I'm getting a log of keepalive warning and error logs from
ffmpeg. On the other hand it's really annoying to see a "process
failed, see :process for details" and having no error messages in
there.
Fixes: #7838
With Qt 6.3+, user interaction is required to navigate outside of qute:// from a
qute:// page.
Follow-up to 8defe1ae44.
Also see 216a9f9a9bFixes#7815
See #7220 - should be revisited once we have a qute-bookmarks:// instead where
we can adjust permissions when registering the URL handler.
Regressed in c2210539a9e2be1deacf8df8f432e035d9b9b9f0:
The current NSIS installer still adds the suffix. Let's keep it there for now
until we switch to the rewritten one.
See #6050
By default, we only get a narrow checkout, so we don't know about any
other branches. Use the GitHub API and some JS to get the release branch
instead.
Follow-up to a46e9f2036 to work correctly with
older Qt versions (< 6.4), where this is not a QQuickWidget apparently.
This also means we can simplify the workaround, as we are guaranteed to be on Qt
6.4+ anyways.
See #7820, #7831
When pressing buttons on some websites, or when starting to drag, it looks like
the WebView gets new QObject children which are not actually their focus proxy.
So far, this wasn't a big issue: We only ended up installing the tab event
filter on objects where it doesn't belong.
However, with the new focus workaround from #7820, we then ended up calling
`.setFocus()` on those QObjects, causing an AttributeError.
Thus, just don't do anything if we get new children that are not actually a
QQuickWidget.
Fixes#7831
Opening a download in a new tab leaves a "dead" tab (see example of a "dead" tab below)
behind. When saving a session containing such a "dead" tab, we end up with entries in
the session like this one:
```yaml
- active: true
history:
- active: true
last_visited: '1970-01-01T02:00:00'
pinned: false
scroll-pos:
x: 0
y: 0
title: ''
url: ''
zoom: 1.0
```
When loading a session containing such a "dead" tab, qutebrowser does not restore any
history of that session and logs the following error:
```
ERROR: Failed to load session default: PyQt6.QtCore.QUrl('') is not valid
```
As pointed out by @The-Compiler in this comment[1], the behavior of
`QWebEngineHistoryItem::isValid()`[2] changed somehow between Qt 6.4 and 6.5.
`QWebEngineHistoryItem::isValid()` now returns `True` for "dead" tabs, even though the
history item is not valid (i.e. `url().isValid()` returns `False`).
To fix this we simply add an additional check if the URL is valid before adding a tab to
the session to be saved.
[1] https://github.com/qutebrowser/qutebrowser/issues/7696#issuecomment-1672854592
[2] https://github.com/qt/qtwebengine/blob/v6.5.2/src/core/api/qwebenginehistory.cpp#L69-L75
See 5ff0a573f4
With that commit, encoding the header ourselves means that we'll actually be
navigating to the path `/b'data/...'` instead of `/data/...`.
This partially solves #7820. The web view still loses focus for an unknown
reason (apparently when swtiching out the rendering process?), but at least it
regains focus now when the window is unfocused and then focused again.
The CheckPlatform macro will prompt the user user to use the 32bit installer
if they are on a 32bit system. But we don't provide a 32bit installer anymore.
This commit changes the OS version check for Qt5 builds to be based on checking
version numbers ourselves too, so that we can have our own error message.
Also moves the Qt5 conditionals to be compile time ones.
We dropped 32bit support in #7804 and as a result removed the arch
suffix from the binary that pyinstaller produces. This commit removes it
form the lookup path in the installer too.
Note that we are leaving the arch string in the installer itself for
now. Mostly because it'll be removed as part of a later change when the
installer itself is refreshed. But it might also be useful to clarify in
the installer names what the arch is? Maybe, that reasoning might not
fit with the previous change to remove the arch strings.
The Qt docs for 6.5 say that the minimum supported version is Windows 10
1809.
Experimentally it seems qutebrowser and it's dependencies work fine on a
version as early 1607.
There should be no change in OS version requirements for the Qt5 build,
although we've dropped 32 bit support already and in a future version of
the installer we may bring the minimum OS version support in line with
the Qt6 requirements for simplicity too.
Added a new QT5 version into the NSIS scripts so we can do the different
version check per installer build. It just uses the python bool
serialization format so should always be "True" or "False", but I've
added a fallback anyway for consistency.
Reproducers:
python3 -c 'from qutebrowser.extensions import loader'
python3 -c 'import qutebrowser.commands.command'
Specifically the first on was being called from misc/qutebrowser.spec in
the nightly installer build jobs.
Running the full application still works fine somehow. So it might be
possible to import things in the right order to avoid the loop. But
since this is part of our API we probably don't want to require too much
juggling.
Restored from dd2fc8e10bb9d4a1bd0158110173793a18736d6b
Now that we are putting our data files in the qutebrowser/ directory
again pkgutil/importlib is getting confused by that dir existing and
returning us a FileLoader for `qutebrowser.components`, I think that's
what's happening anyway. Should try reverting that and this commit and
see if extensions get loaded right again.
So bring back this workaround of using the toc on the PyInstaller loader
to get the list of component modules for now.
ref: #7803
I don't want to deal with having to review development changes of
pyinstaller every week. So pin to one commit for now that we can
actually test. I'm subscribed to release notifications on github so I'll
manually change this back to point to the pyinstaller pypi package ones
it does.
Group commands related to commands/commandline by prefixing them with `cmd-`. In this
case we additionally renamed the command slightly to fit better with the `cmd-` prefix.
Group commands related to commands/commandline by prefixing them with `cmd-`. In this
case we additionally renamed the command slightly to fit better with the `cmd-` prefix.
Until we look at #7220 proper (thus splitting this into a qute-start:// which
could probably have more permissions to do remote requests), we'll need to do
with somewhat of a hack to allow this even if QtWebEngine does not.
Given the very limited scope (only from qute://start, only opening the search
engine URL, only with a form submitted request), this should be acceptable
without compromsing security in any way.
Fixes#7790
With Qt 6.3+, user interaction is required to navigate outside of qute:// from a
qute:// page.
Fixes#7815
See #7220 - should be revisited once we have a qute-bookmarks:// instead where
we can adjust permissions when registering the URL handler.
Needed mostly for urlmarks BDD tests so they can clear things between tests.
Hopefully with --all, this won't be accidentally triggered by users.
Preparation for #7815
While not documented that way:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/cwait?view=msvc-170
It looks like that Windows sometimes sets errno to EACCES here (causing a
PermissionError):
> os.waitpid(pid, 0) # pid, options... positional-only :(
E PermissionError: [Errno 13] Permission denied
I have no idea why it happens, but it results in flaky tests on CI.
We aren't particularly interested in this (we just want to make sure the process
is cleaned up before the next test runs...), so let's just ignore this.
We got a `DeprecationWarning` during the package build, which we were not able to
reproduce locally. For now we just don't turn this particular `DeprecationWarning` into
an exception to not fail CI.
This is still *very* basic, but it serves its purpose of failing for warnings during
package build.
I verified that `tox -e package` is failing by introducing some warnings with this change:
```diff
diff --git a/setup.py b/setup.py
index feb949595..6810eaf1e 100755
--- a/setup.py
+++ b/setup.py
@@ -51,8 +51,7 @@ def _get_constant(name):
try:
common.write_git_file()
setuptools.setup(
- packages=setuptools.find_namespace_packages(include=['qutebrowser',
- 'qutebrowser.*']),
+ packages=setuptools.find_namespace_packages(include=['qutebrowser']),
include_package_data=True,
entry_points={'gui_scripts':
['qutebrowser = qutebrowser.qutebrowser:main']},
```
Catches issues with invalid URLs early instead of later in the code (e.g. when
the brave adblocker runs on the URL).
Hopefully helps catch issues with broken config.py hacks calling redirect().
Seems the new flake8 release is pulling down a (somewhat) new
pycodestyle that prefers is/is not over ==/!= when comparing exact
types. They should behave the same.
ref: #7807
In the past various workarounds have been put in place to move/copy/symlink
files in the macOS app build to make PyQt work and let us sign it.
As of https://github.com/pyinstaller/pyinstaller/pull/7619 our downstream
patching should not be required.
The application seems to run fine.
The app size is 155 MB.
Signing is still to be verified.
It's a change from before but it's strictly more accurate anyway, in the
application we are always using sip from under the PyQt module, even if
PyQt5 registers it as the plain `sip` too. And now it's consistent with
what we have to do for PyQt6.
Fixed:
* The `PyQt{5,6}.sip` version is now shown correctly in the
:version|--version output. Previously that showed the version from the
standalone `sip` module which was only set for PyQt5.
Currently, ":version" fails to show the sip version for Qt6. This is
because the sip module can't imported in the same way as Qt5. In Qt5,
the sip module can be imported after "from PyQt5.QtCore import *". In Qt
6, this is no longer the case,
>>> from PyQt6.QtCore import *
>>> import sip
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'sip'
So let importlib import PyQt6.sip explicitly.
Not that we are looking up resources via importlib_resources for
pyinstaller builds too we need to change where the data files are
installed to to match what importlib_resources is expecting.
There was a comment in the previous resource lookup special case
complaining about the data files being at the top level so it seems this
is a change for the better anyhow.
Observed paths:
requested file: qutebrowser.app/Contents/Frameworks/qutebrowser/config/configdata.yml
actual file : qutebrowser.app/Contents/Frameworks/config/configdata.yml
Since we are pulling down PyInstaller off of the develop branch we need
to recompile the bootloader, because upstream only commits a new one
back to the branch on releases. Luckily all the compiler requirements
seem to already be installed on CI.
For the record the macOS CI is currently failing with
dlopen: dlopen(/Users/runner/work/qutebrowser/qutebrowser/dist/qutebrowser.app/Contents/MacOS/libpython3.10.dylib, 10): image not found
And upon inspection of dist/ that file seems to be at
./qutebrowser.app/Contents/Resources/libpython3.10.dylib
./qutebrowser.app/Contents/Frameworks/libpython3.10.dylib
./qutebrowser/_internal/libpython3.10.dylib
PyInstaller has [recently][symlinks] landed a change that should restore
the webengine sandbox on macOS builds. With a bit more testing we are
hoping that we can go ahead releasing builds based on that
in-development PyInstaller codebase even before they've made a release.
This commit pins our pyinstaller dependency to be the latest commit on
their develop branch.
[symlinks]: https://github.com/pyinstaller/pyinstaller/pull/7619
I don't think the dependencies jbos needs to be installing asciidoc anymore either but oh
well.
I just grepped for libxcb-shape0 and constituently added libxcb-cursor0
after it. I haven't checked it's in stretch but it should be.
ref: 6f5de192e0
We still want to make sure that IPC is shut down early when using :restart.
However, doing it again should be a nop when quitting qutebrowser then
shuts it down again.
This accidentally worked before 6373959 ("Always delay stage 2 shutdown, not
only with prompts"), because most operations in shutdown() just happened to be
idempotent. Because we never returned to the Qt mainloop between the first and
the second call, the self._server.deleteLater() call didn't actually delete
anything yet.
After that change, however, we do end up in shutdown() again with an actually
deleted C++ QLocalServer object, so trying to call .close() on it again failed
with a RuntimeError.
Fixed#7681
reuse annotate --license="GPL-3.0-or-later" --style python \
misc/nsis/uninstall_pages.nsh \
misc/nsis/install.nsh \
misc/nsis/uninstall.nsh
And fixing qutebrowser.nsi manually as that uses iso-8859-1 and the reuse tool
doesn't like that apparently.
git ls-files | \
xargs grep -l "This file is part of qutebrowser" | \
xargs grep -l SPDX-License-Identifier | \
xargs sed -i '/# This file is part of qutebrowser\./,/along with qutebrowser\. If not, see <https:\/\/www\.gnu.org\/licenses\/>./d'
Right now the version of PyQT available on PyPI is 6.5.1 and it's buggy with
Qutebrowser, this is a quick hack to get it working until it's released early next week.
Now that we moved all Qt related things out of `qutebrowser.utils.log` we can import
`qutebrowser.utils.log` in `qutebrowser.qt.machinery`, and therefore move the machinery
log where it belongs.
This resolves a temporary workaround for a circular import.
Now that we fully separated `qutebrowser.utils.log` and `qutebrowser.utils.qtlog`, we
can go back to keeping all logger definitions in the same place.
Building qutebrowser showed some warnings as the following:
```
/tmp/build-env-4jb2oh0t/lib/python3.8/site-packages/setuptools/command/build_py.py:201: _Warning: Package 'qutebrowser.html' is absent from the `packages` configuration.
!!
********************************************************************************
############################
# Package would be ignored #
############################
Python recognizes 'qutebrowser.html' as an importable package[^1],
but it is absent from setuptools' `packages` configuration.
This leads to an ambiguous overall configuration. If you want to distribute this
package, please make sure that 'qutebrowser.html' is explicitly added
to the `packages` configuration field.
Alternatively, you can also rely on setuptools' discovery methods
(for example by using `find_namespace_packages(...)`/`find_namespace:`
instead of `find_packages(...)`/`find:`).
You can read more about "package discovery" on setuptools documentation page:
- https://setuptools.pypa.io/en/latest/userguide/package_discovery.html
If you don't want 'qutebrowser.html' to be distributed and are
already explicitly excluding 'qutebrowser.html' via
`find_namespace_packages(...)/find_namespace` or `find_packages(...)/find`,
you can try to use `exclude_package_data`, or `include-package-data=False` in
combination with a more fine grained `package-data` configuration.
You can read more about "package data files" on setuptools documentation page:
- https://setuptools.pypa.io/en/latest/userguide/datafiles.html
[^1]: For Python, any directory (with suitable naming) can be imported,
even if it does not contain any `.py` files.
On the other hand, currently there is no concept of package data
directory, all directories are treated like packages.
********************************************************************************
!!
```
Using `find_namespace_packages()` as suggested in the setuptools docs[1] solved the issue.
[1] https://setuptools.pypa.io/en/latest/userguide/package_discovery.html#finding-namespace-packages
This used to be possible in some situations and was handled in somewhat
unexpected places (e.g. .to_qt()). Instead, we now assume that KeyInfo
is always "clean", and we handle the conversion from an int to a Qt.Key
elsewhere.
This only seems to affect tests, since otherwise we already made sure
we get a Qt.Key and Qt.KeyboardModifier(s) e.g. in .from_event().
We're deprecating vim modelines in favor of `.editorconfig`.
Removing vim modelines could be done using two one-liners. Most of the vim modelines
were followed by an empty line, so this one-liner took care of these ones:
```sh
rg '^# vim: .+\n\n' -l | xargs sed -i '/^# vim: /,+1d'
```
Then some of the vim modelines were followed by a pylint configuration line, so running
this one-liner afterwards took care of that:
```sh
rg '^# vim:' -l | xargs sed -i '/^# vim: /d'
```
Done by removing the existing config and doing:
tox -e mypy-pyqt6 | \
grep -F .py | \
cut -d: -f1 | \
sort | \
uniq | \
sed 's/\.py//' | \
sed 's/\//./g' | \
while read line; do \
echo "[mypy-$line]\ndisallow_untyped_defs = False\n" \
done >> .mypy.ini
This means we now enforce type annotations for all new modules.
We can still add sections for upcoming PR merges where this is a problem.
Closes#7409
The ignores needed between Qt 5 and Qt 6 differ.
We could buy into e.g. only Qt 6 linting, but apparently e.g. VS Code
also shows more errors when removing the Qt 5 type ignores.
Instead, disable this for now. We might want to re-enable it when we see
a major change in the mypy changelog and filter the results manually.
81 -> 50 errors
This means we will now get errors via the usual mechanisms
(e.g. a Tk error dialog) when all Qt wrappers failed to import.
We also add information about the picked Qt wrapper (and any errors)
to the error message.
Otherwise we run into a Python 3.12 incompatibility with pkg_resources:
https://github.com/pypa/pip/issues/11501
Also needs PIP_REQUIRE_VIRTUALENV=0 because otherwise pip seems to
falsely assume it's installing things system-wide, weirdly.
Should probably be removed once there is a newer virtualenv, which
vendors a newer pip, which vendors a newer pkg_resources...
See #7727
File "/usr/lib/python3.11/site-packages/qutebrowser/browser/webengine/notification.py", line 859, in _find_quirks
if utils.VersionNumber.parse(ver) < utils.VersionNumber(2, 0):
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/site-packages/qutebrowser/utils/utils.py", line 96, in __init__
raise ValueError(
ValueError: Refusing to construct non-normalized version from (2, 0) (normalized: (2,)).
Since Archlinux upgraded to Python 3.11, we need to downgrade Python
to 3.10 too, and install tox via pip instead.
This starts becoming somewhat questionable, but if this approach still
does indeed work, let's go for it.
These tests were failing on webkit due to cookies not getting set (they
made a change recently to fix a security issue around cookies and must
have messed something up)
tests/end2end/test_invocations.py::test_cookies_store
tests/end2end/features/test_private_bdd.py::test_make_sure_private_data_is_not_cleared_when_closing_a_private_window_but_another_remains
Although we updated the tox requirements files for tox4 in December, it
looks like the docker containers (or at least the one(s) that call this
script) are still using tox3. `.tox-config1` isn't written anymore with
tox4 (although my git-fu isn't strong enough to find the commit that
removed it).
The "Rebuild Docker CI images" run 912 was the last to have tox 3.26.0-2
and run 913 had version 4.4.12-1.
I'm not sure the new change does the same thing as whatever the old code
did. I honestly have no idea why we can't just follow the python symlink
and have to go to a config file. In what case does the python binary not
link to the system one?
In addition to this `.tox-info.json` file there is also the more
standard `pyvenv.cfg` file. I would prefer to use that over the tox
thing as generally I find tox to be a high barrier of entry for
contributors. But in that case the pyvenv one doesn't seem to be well
documented either...
Anyway, if anyone knows any cases where following the symlink of the
python in the venv isn't reliable that would be great. Because that
seems like the simpler method.
Pin Flask and Werkzeug for py3.7
They've removed compatibility upstream.
Added example to readme file because I had to go dig for one in the
commit history.
Add blinker to changelog URLs as it's a required dependency of Flask
now.
- Make sure we still test Python 3.7 and 3.8 after dropping old PyQt
versions in c5a51eb0bc
- Keep a modern Python version (3.11) with Qt 5 around, however
- Make sure we still test Python 3.10 too
- Also start testing the Python 3.12 alpha
For unknown reasons, on nightly CI we now get TWO errors:
<qutebrowser.browser.webkit.certificateerror.CertificateErrorWrapper errors=['NoPeerCertificate', 'SelfSignedCertificate'] string='The peer did not present any certificate\nThe certificate is self-signed, and untrusted'>
Looks like we have some failing smoke tests with:
[6636:16643:0329/041427.206197:ERROR:command_buffer_proxy_impl.cc(141)] ContextResult::kTransientFailure: Failed to send GpuChannelMsg_CreateCommandBuffer.
TL;DR: I think I stabilized a couple of hinting iframe tests and applied
that change to all the iframe usages in the end2end tests in the hopes
that it would resolve some other flaky tests.
I was facing some tests that hinted elements in iframes failing
intermittently recently. They were most consistently failing on the
windows runner. This is similar to the trend described in the comment
linked from #1525.
The tests failing recently where:
tests (py39-pyqt515, windows-2019, 3.9)
test_using_hintfollow_inside_an_iframe
test_using_hintfollow_inside_an_iframe_button
tests (py311-pyqt515, ubuntu-20.04, 3.11)
test_using_hintfollow_inside_an_iframe
test_using_hintfollow_inside_an_iframe_button
They are failing because hints don't get generated for the elements in
the iframe. I can see hints do get generated for the iframe itself
though.
Examining the logs in 5.15 it seems that there is a
`cur_load_finished(True) (tab *)` log line after we run :hint. I suspect
that this is the load finished signal for the iframe.
I attempted to change the bdd open_path function to wait for the current
message it's looking for and then additionally look for that
cur_load_finished signal, but then it starts failing locally when the
iframe load signal actually comes before the parent frame one. Just
looking for the cur_load_finished signal itself also always found it
immediately ("already found"), probably from the parent frame.
So instead of trying to deal with that indeterminate ordering or
trying to change the signals to say what frame they are coming from, I
added javascript to all the pages used in iframes that run on load, and
changed all the tests to watch for log messages from the JS.
It's not the most maintainable solution, perhaps if we generated our
test files with jinja we could have some "subframe loaded" boilerplate,
get a message to log from a query param, look at metadata from test
files for the open_path checking etc.
I then searched for all the iframe usages in the test data and applied
that change to all of those files and all the tests that used them. A
few of those tests also had `flaky` annotations on them. I removed those
annotations for now in the hopes that there were affected by the same
problem. And that I actually managed to work around it correctly.
ref: #7621
This was first introduced in adbdfcbad3,
most likely because we got logging from the built-in Werkzeug webserver.
It doesn't seem to be needed anymore, possibly since
41c4ee3e2f where we started using CherryPy
for the SSL server too.
This should fix nighly bleeding tests, because the before_first_request
decorator got removed in Flask:
https://github.com/pallets/flask/pull/4621https://github.com/pallets/flask/pull/4995
There's some weird issue with Qt6.4 and 6.5 where a webengine view gets
its widget swapped out when it gets history deserialized into it. While
it's swapping widgets it has no so the focus gets passed to some other
widget should never even have focus.
There's probably more comprehensive ways we could handle this, by
overriding the WebEngineView layout, or by overriding focusNextPrevChild
on the parent to put out own logic into the "child is going, pass focus
to parent" logic chain.
But all that seems like a bit too much of a headache for this very
focussed issue. We want the new tab to get focus, so lets re-focus it.
Technically we could just focus the last tab that we open, if we are
undoing multiple, but the existing logic is to open each of them in turn
as foreground tabs and this reinforces that.
Closes: #7623
Not sure why its failing on windows, but it only really needs to run in
one environment anyway.
Previously this was using chrome://gpu but that was failing on CI due to
it invoking some vulkan stuff the didn't work without a GPU. I changed
it to chrome://sandbox/ for no good reason, but it seems to be working
fine on ubuntu at least.
Follow up to 151d808940
This one is for 6.5. Interesting that it was fine on 6.4.
I haven't gone and added text to all the test file because I'm hoping
it'll be something that upstream addresses in short order. So I'm only
adding text to files used by failing tests.
See d413b87c3f#7621 and #7624
Error: qutebrowser/browser/webengine/certificateerror.py has 90.91% line and 50.00% branch coverage!
Error: qutebrowser/browser/webengine/darkmode.py has 96.88% line and 100.00% branch coverage!
Error: qutebrowser/keyinput/basekeyparser.py has 97.59% line and 100.00% branch coverage!
Error: qutebrowser/keyinput/keyutils.py has 92.64% line and 96.30% branch coverage!
Error: qutebrowser/utils/debug.py has 94.51% line and 92.19% branch coverage!
Error: qutebrowser/utils/qtutils.py has 98.67% line and 96.74% branch coverage!
Error: qutebrowser/utils/usertypes.py has 99.20% line and 96.30% branch coverage!
Error: qutebrowser/utils/version.py has 97.66% line and 97.83% branch coverage!
Maybe we should open an issue to get them back up.
On CI were were getting "Could not import sip" because link_pyqt was
looking for PyQt5.sip.
I made that look at QUTE_QT_WRAPPER since that's being set already on
tox.ini
There are probably a few other changes around link_pyqt and the makefile
etc we need to change when we switch the default wrapper.
I overrode the default `py` tox environment with py-qt6 to override
those wrapper related variables. I probably could have done something
sneaky with curly braces to make it so we don't have to add a few more
lines to the file. But in my opinion in config file is far to obfuscated
and hard to maintain already.
I changed the docker file to call the new py-qt6 env if it's a qt6
container. I'm not 100% sure that is required though since there is also
a tox invocation in the GH action definition, maybe that overrides the
container entrypoint? Also changed the indentation in the dockerfile
template a bit to make it easier to see where the conditionals start and
end.
Speaking of which I changed the matrix definition and tox invocation to
match a later one to hopefully make it so we can invoke different tox
environments in the containers without having to rebuild the containers.
Not sure I did that right, I'll see soon.
I added the unstable-qt6 container generation line so we can use it in
the future, and to match the not-qt6 one. I'm not switching to that in
CI though because the pyqt used by that is broken at the moment
(ref https://www.riverbankcomputing.com/pipermail/pyqt/2023-March/045214.html)
Also fixed the vim modeline in generate.py so my syntax highlighting
works.
On GH actions I'm seeing an "unexpected line" failure around creating a
vulkan context when loading chrome://gpu
ERROR tests/end2end/features/test_tabs_bdd.py::test_cloning_a_tab_with_a_special_url - end2end.fixtures.testprocess.InvalidLine:
1829
Error: vkCreateInstance failed with VK_ERROR_INCOMPATIBLE_DRIVER
1830
at CheckVkSuccessImpl (../../../3rdparty/chromium/third_party/dawn/src/dawn/native/vulkan/VulkanError.cpp:88)
1831
at CreateVkInstance (../../../3rdparty/chromium/third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:360)
1832
at Initialize (../../../3rdparty/chromium/third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:235)
1833
at Create (../../../3rdparty/chromium/third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:165)
1834
at operator() (../../../3rdparty/chromium/third_party/dawn/src/dawn/native/vulkan/BackendVk.cpp:420)
I'm not sure its actually failing the test, we are just seeing the error
in the logs and flagging it.
Instead of adding the logs to an ignore list I'm going to switch the
page we use so that if the error comes up for real we'll be sure to see
the logs.
6.4 tests should be passing now. 6.5 is still waiting on PyQt on PyPI
getting fixed. That goes for kde-unstable-qt6 too as that's pulling down
6.5.
We should take the opportunity to revisit the test matrix configuration
again when we add 6.5 in there. For example py3.11 on ubuntu 20.04 is a
bit odd as it only got released last year.
The ELF file structure changed in a way that it seems impossible to
support reliably. Given that we have an API to get the version nowadays,
let's not bother with this. We might need to figure this out for PySide6
support, but not now.
See #7624, #3625
In doc/help/index.asciidoc, we have links like this:
* link:../quickstart{outfilesuffix}[Quick start guide]
That is correct in e.g. the GitHub file structure, as those files are
stored in e.g. doc/quickstart.asciidoc indeed.
It's *no* longer true when we view the built files via qute://help,
however: There, this turns into a flat file structure, with those pages
being at qute://help/index.html and qute://help/quickstart.html,
respectively.
It looks like QtWebEngine < 6.5 did just ignore the
<a href="../quicktart.html"> and pointed from qute://help/index.html to
qute://help/quickstart.html anyways, weirdly.
With QtWebEngine 6.5, however, this is now interpreted as a link
pointing to qute://quickstart.html instead, which is clearly wrong.
Until we have a less messy doc generation (probably as part of #345),
let's just patch the link to be correct.
See #7624
I don't quite understand why the value changed from #00000 to #121212
there with Qt 6.3, but it looks like the change is here to stay.
Instead of keeping #000000 the default and adding an override for every
new Qt release, let's just switch over and add an override for the old
versions (5.15 and 6.2), so this won't break again for every new release.
See #7624
The fontMetrics().elidedText(...) call is copied from the base QTabBar
implementation of initStyleOption(). But here we are calling it with a
text_rect from our custom style class. In Qt6 QStyleSheetStyle no longer
calls into our custom Style class for the SE_TabBarTabText SubElement so
it ends up getting a text width from QCommonStyle which is wrong for
vertical tabs (`QCommonStylePrivate::tabLayout()` seems to transpose the
rect, maybe it thinks vertical tabs should have the text running
vertically too).
Also resolves the issue that when tabs.title.alignment is set to center
the leftmost character or two of titles wouldn't be visible when tabs
where small enough that the text got elided. Not 100% sure why but
presumably it wasn't picking up the padding etc logic in our custom
style class.
In the test I don't test the exact elided text when we don't have much
room because it might differ based on what monospace font is picked up
in the tests. So the amount elided might change. Also in the not-elided
one I'm only testing endswith() because the index should be at the start
of the string which I don't really care about.
Fixes: #7313Fixes: #7205
A quick tour through the PDF.js API:
v1.0.1040 introduced the ability to pass an object to PDFView.open()
instead of passing an URL:
https://github.com/mozilla/pdf.js/pull/5254
v1.6.210 (?) renamed PDFView to PDFViewerApplication:
ffa276a182
v3.3.122 made it mandatory to pass an object (and also made originalURL
optional when doing so):
https://github.com/mozilla/pdf.js/pull/15972
We should probably properly get rid of old version support at some point
(see #7619), but until then, I *think* this approach should work with
older versions still.
Fixes#7618
(cherry picked from commit 924a7a5907)
A quick tour through the PDF.js API:
v1.0.1040 introduced the ability to pass an object to PDFView.open()
instead of passing an URL:
https://github.com/mozilla/pdf.js/pull/5254
v1.6.210 (?) renamed PDFView to PDFViewerApplication:
ffa276a182
v3.3.122 made it mandatory to pass an object (and also made originalURL
optional when doing so):
https://github.com/mozilla/pdf.js/pull/15972
We should probably properly get rid of old version support at some point
(see #7619), but until then, I *think* this approach should work with
older versions still.
Fixes#7618
This reverts commit 47be6f3aeb.
This doesn't really make sense in test files, as the warning would point
inside some internal pytest code, which is not helpful.
Instead, let's just disable the corresponding flake8 warning in tests.
Recently the FreeBSD port of sqlite has DQS feature disabled by default.
Without this feature enabled, it's not allowed to use double quotes
for string literals. As such quoting is used in the history.py module,
qutebrowser fails to work on such configurations.
The fix is to use single quotes instead.
ref: #7596
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.
flake8-bugbear B028 No explicit stacklevel keyword argument found. The
warn method from the warnings module uses a stacklevel of 1 by default.
This will only show a stack trace for the line on which the warn method
is called. It is therefore recommended to use a stacklevel of 2 or
greater to provide more information to the user.
Semgrep helped:
semgrep --lang=py -e 'warnings.warn($ARG)' --replacement 'warnings.warn($ARG, stacklevel=2)' $FILES -a
semgrep --lang=py -e 'warnings.warn($ARG1, $ARG2)' --replacement 'warnings.warn($ARG1, $ARG2, stacklevel=2)' $FILES -a
Although it did lose the f-string on one of them.
Not a huge fan of this suggestion. I don't think it adds anything apart
from a reliance on a language feature that isn't used much in this
codebase. I don't want to have to explain to inexperienced devs that
they have to play a bit of code golf instead of focussing on what
actually matters.
Mostly pretty lazy fixes. Most of the places in the tests we were
already matching on error message, a couple of places we weren't. The
tick-tock one was the only one that wasn't being used right where it was
raised.
Some of them I just changed to RuntimeError because it was shorter than
adding the pylint directive.
Looks like the upstream bug is fixed now.
I didn't run recompile-requirements because it requires py3.7 for the
pylint file and I don't have that. The robot will run overnight anyway
(and pick up a new pylint release)
ref: 085a7956eb
The text strategy doesn't perform well with long strings.
Not really related to this PR but pushing to a branch and merging is
less stressful than pushing to master.
ref: #7553#issuecomment-1399918153
Flake8-bugbear points out some places where we use
pytest.raises(Exception). That can swallow errors with the tests that
aren't the intended ones. I most cases we are also passing
match="some-regex" to limit the exceptions that it matches against.
There is one case where that didn't look easy to do so I subclassed
exception.
ref: #7553
This is actually the renamed pep517, but I'm not removing that as
pyorama still depends on it. Even though pyproject_hooks got added to
the pyorama requirements file, huh???
This fails now with 'failed with env name pyinstaller-32 conflicting with base python C:\hostedtoolcache\windows\Python\3.10.9\x86\python.exe'
See https://github.com/tox-dev/tox/pull/2824
We are already skipping round trip tests for partial functions made in
gen_classes from compound classes (list, dict, ...). This change makes it so
we skip trying it for subclasses of those compound classes too (eg FlagList
and Padding). This is due to the test failing with the input "- A" to
FlagList.
Also switches the Dict example in the comment to be the same order as the
other two.
Some questions I don't know the answers to:
* Why are List and Dict using `json.dumps()` in `to_str()` instead of `yaml_dump()`?
* Is `to_str()` used anywhere apart from tests?
* Should we have a `to_yaml()` or just use `yaml_dump()` in the test to get back
the original value?
If we shut down qutebrowser directly, in some rare situations, we seem
to get into some trouble related to a QWidget trying to call into a
Q(Common|StyleSheet)Style at shutdown (checking the style hint
SH_Widget_ShareActivation, to figure out the palette to use when the
window is hidden, or somesuch?).
With the recent changes for #7504, those crashes suddenly started
happening much more often. After staring at the related changes and gdb
for over a day, I still have no idea what actually causes them and why
it's crashing - it smells like some kind of PyQt/Qt cleanup order bug...
However, what I *did* find out (after a lot of painful debugging and
messing around with things) is that delaying the shutdown done from
:quit seems to reliably get rid of the issue. I don't know *why*, but
I haven't seen it happen anymore with the latest changes...
Fixes#5385 and #5124, both speculatively.
Closes#7504 since this was the last change required to make it all
work.
In bleeding tests, we started to get a segfault on the second test in
tests/unit/mainwindow/test_prompt.py, with a stacktrace like:
Thread 1 "python" received signal SIGSEGV, Segmentation fault.
0x00007ffff7b55912 in _PyFunction_Vectorcall (func=<function at remote 0x7fffc4368ca0>, stack=0x7fffd74656a8, nargsf=<optimized out>, kwnames=0x0) at Objects/call.c:341
341 if (((PyCodeObject *)f->fc_code)->co_flags & CO_OPTIMIZED) {
(gdb) bt
#0 0x00007ffff7b55912 in _PyFunction_Vectorcall (func=<function at remote 0x7fffc4368ca0>, stack=0x7fffd74656a8, nargsf=<optimized out>, kwnames=0x0) at Objects/call.c:341
#1 0x00007ffff4e0b3f1 in PyQtSlot::call(_object*, _object*) const (this=0x555556c759c0, args=('/tmp/pytest-of-florian/pytest-70/test_simple_completion_1_next_0/test',), callable=<function at remote 0x7fffc4368ca0>)
at ../../qpy/QtCore/qpycore_pyqtslot.cpp:247
#2 PyQtSlot::invoke(void**, _object*, void*, bool) const (this=0x555556c759c0, qargs=<optimized out>, qargs@entry=0x7fffffff86c0, self=<optimized out>, self@entry=0x0, result=result@entry=0x0, no_receiver_check=<optimized out>)
at ../../qpy/QtCore/qpycore_pyqtslot.cpp:159
#3 0x00007ffff4e12213 in PyQtSlot::invoke(void**, bool) const (no_receiver_check=<optimized out>, qargs=0x7fffffff86c0, this=<optimized out>) at ../../qpy/QtCore/qpycore_pyqtslot.cpp:78
#4 PyQtSlotProxy::unislot(void**) (qargs=0x7fffffff86c0, this=0x555557193e20) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:205
#5 PyQtSlotProxy::unislot(void**) (qargs=0x7fffffff86c0, this=0x555557193e20) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:186
#6 PyQtSlotProxy::qt_metacall(QMetaObject::Call, int, void**) (this=0x555557193e20, _c=<optimized out>, _id=0, _a=0x7fffffff86c0) at ../../qpy/QtCore/qpycore_pyqtslotproxy.cpp:170
#7 0x00007ffff48bd91d in doActivate<false>(QObject*, int, void**) (sender=0x555556b3d680, signal_index=28, argv=0x7fffffff86c0) at kernel/qobject.cpp:3945
#8 0x00007fffeff96aca in QFileSystemModel::directoryLoaded(QString const&) (this=<optimized out>, _t1=<optimized out>) at .moc/moc_qfilesystemmodel.cpp:272
#9 0x00007ffff48b0be0 in QObject::event(QEvent*) (this=this@entry=0x555556b3d680, e=e@entry=0x7fff0c004af0) at kernel/qobject.cpp:1347
#10 0x00007fffeff962ab in QFileSystemModel::event(QEvent*) (this=this@entry=0x555556b3d680, event=event@entry=0x7fff0c004af0) at dialogs/qfilesystemmodel.cpp:1748
#11 0x00007ffff070995c in sipQFileSystemModel::event(QEvent*) (this=0x555556b3d680, a0=0x7fff0c004af0) at /usr/src/debug/pyqt5/PyQt5-5.15.7/build/QtWidgets/sipQtWidgetsQFileSystemModel.cpp:376
[...]
#23 0x00007ffff4da94ce in meth_QCoreApplication_processEvents(PyObject*, PyObject*, PyObject*) (sipArgs=<optimized out>, sipKwds=0x0) at /usr/src/debug/pyqt5/PyQt5-5.15.7/build/QtCore/sipQtCoreQCoreApplication.cpp:590
[...]
from pytest-qt in Python:
Current thread 0x00007fb366207740 (most recent call first):
File ".../pytestqt/plugin.py", line 209 in _process_events
File ".../pytestqt/plugin.py", line 171 in pytest_runtest_setup
[...]
File ".../pytest/src/_pytest/runner.py", line 115 in pytest_runtest_protocol
[...]
File ".../pytest/src/_pytest/main.py", line 317 in pytest_cmdline_main
[...]
Introduced by this change in pytest, which now deletes the temporary
directory after the test by default:
https://github.com/pytest-dev/pytest/pull/10517
It sounds like something odd like the directory removal causing (Py)Qt
to call the lambda, but the underlying Python object already being gone
or something along those lines. With a proper Qt slot (@pyqtSlot),
PyQt seems to do the right thing, so let's just use that instead...
With Qt 6.4, QtWebEngine closes/reopens the main window to switch the
RHI rendering mode when a QWebEngineView gets added:
https://github.com/qt/qtbase/blob/v6.4.1/src/widgets/kernel/qwidget.cpp#L10706-L10709
To avoid this, we need to make sure we only call .show() *after* adding
a tab, similarly to what Qt did too:
https://code.qt.io/cgit/qt/qtwebengine.git/commit/?id=d7e0fd5304ebdb12c6f809cdbcf8193b49b9ecd2
See #7504
----
This commit handles changes around app.py/mainwindow.py, which was
probably the most complex one of the whole set (and also involving the
oldest code I bet...).
This changes a couple of intertwined things:
- app._process_args() now needs to take track of the window it opened
(if any), so that it can call .show() on it *after* opening pages.
* If a session was loaded instead, sessions.py takes care of showing
it.
* The setActiveWindow call was also moved for simplicitly. Before, it
was called even with --nowindow given, but that doesn't really make
much sense.
* I'm not actually sure why the .setActiveWindow() call is there. Qt
docs say "Warning: This function does not set the keyboard focus to
the active widget. Call QWidget::activateWindow() instead.".
It was added back in 2014: ae44aa01a6
("Set initial focused window correctly."). It's possible it's not
needed anymore, but that's for a separate commit.
- app.process_pos_args() now gets a MainWindow (rather than win_id)
from mainwindow.get_window(), and is responsible for calling .show()
and .maybe_raise() on it when nothing else does (like open_url called
from it).
* To preserve existing behavior (and not fail tests), it also still
calls .maybe_raise() when a command was given. Maybe we should get
rid of this to fix#5094, but that's for a separate commit.
* Note it does *not* call .show(). That's taken care of later in
_process_args() already.
- app.open_url() can directly show the window, because it already opens
a URL. It now still returns the MainWindow object for consistency.
Also gets rid of some questionable objreg usage just to access a
tabbed browser, as a tiny step towards #640.
- Similarly, app._open_startpage() could have stayed, but now also
takes a MainWindow and uses objreg a bit less.
- open_desktopservices_url also takes care of the show/raise (as this
gets called isolated from Qt), and also avoids objreg stuff where an
attribute access would have sufficed...
- mainwindow.get_window() now doesn't show the window anymore, and
returns the mainwindow object instead of a window ID.
* However, this means it can't actually raise the window anymore
(it needs to be shown first).
- To keep the decision about whether to raise the window in a central
place, MainWindow now has a new should_raise attribute and
maybe_raise() method. mainwindow.get_window() sets should_raise
accordingly, and the caller is responsible to call .maybe_raise()
after showing.
I got an exception there while running tests with the refactored code.
I don't quite understand how/why it changed, but it seems sensible to
skip this too if we skipped calling calling .activateWindow() due to
it being deleted.
See 2fd21bb16b
Trivial to polyfill, needed by various pages in the wild,
and only natively available with Qt 6.2.
Closes#7501
Also see #7237 and 726d5e614b
(cherry picked from commit ce64e5d899)
The "Answering a question for a cancelled download" test was flaky,
due to the preceding PDF.js test sometimes not waiting properly for its
download to finish: "I wait until the download is finished" waited for
the initial PDF download above (which causes PDF.js to open), but not
for the real download after clicking the save button.
See #5390
TODO: Cherry-pick to master?
The recent pytest 7.2 upgrade leads pylint to complain:
************* Module unit.browser.test_qutescheme
tests/unit/browser/test_qutescheme.py:26:0: I0021: Useless suppression of 'no-name-in-module' (useless-suppression)
tests/unit/browser/test_qutescheme.py:226:0: I0021: Useless suppression of 'no-member' (useless-suppression)
************* Module helpers.fixtures
tests/helpers/fixtures.py:39:0: I0021: Useless suppression of 'no-name-in-module' (useless-suppression)
tests/helpers/fixtures.py:639:0: I0021: Useless suppression of 'no-member' (useless-suppression)
possibly due to it not being able to infer 'py' anymore with the weird
shenanigans pytest does now?
Also, what weird glitch in the matrix is it that those just happen to be
on lines [2]26 and [6]39 for *both* files? 🤯
Define some constants for pyright to control how it handles the imports
in qutebrowser.qt.*
This is mainly for autocompletion and definitions with VS Code, which
uses pyright via the pylance extension. If you have multiple possible
places something could be imported from, and one of them isn't
installed, the type of the thing being imported will fall back to Any,
and you wont get nice things.
So we use this file to make sure only certain imports are configured.
The most important thing to remember about this file is it'll control
where type definitions come from in VS Code. You can have multiple
backends defined as true, generally the last import will win in that
case. If any of the enabled imports aren't installed though you may not
get any type hints at all.
PyQt5 has been configured for now to match the type checking configured
in CI.
Personally I would be fine with PyQt6 configured here anyway since
that's generally what we are developing against these days.
See #7370 for more info.
The problem here is that mypy (correctly) complains that self._dialog
could be None again at the point that the lambda runs, which is a
different variable scope.
The assert can be dropped (in the show_dialog locals scope, mypy *knows*
it's never None, as we just assigned it!).
Follow-up to 1bbf75ae7d.
(Ab)using an environment variable indeed seems like the easiest way
forward here, but since it is exposed in the environment for the called
processes, let's give it a name which is less likely to clash, and more
easily identifyable.
Follow-up to c1738ca550.
The changes in requirements-mypy.txt would get overwritten on the next
dependency update. Also, it looks like we don't actually need PyQt6 (or
the PyQt6 stubs) available for checking PyQt 5 code if all Qt 6 imports
are appropriately gated by conditionals mypy knows about.
Follow-up to c1738ca550.
I only had the old way save in my bash history and this one was only
mentioned in the changelog.
Also changed the heading above the new entry to be title case, which
seems to be more consistent with the other headinfs in the file.
Also remove the one remaining mention of `QUTE_BDD_WEBENGINE` since it
does nothing anymore.
There is now some code in statusbar relying on the enabled attribute
stopping events from being processed (or at least stopping them from
showing the widget again). So add tests to make sure that behaviour
keeps working.
Also split the big test in test_backforward into a couple of smaller
ones and pull some common lines out to a (still clunky) fixture.
1. stop pretending to propagate None to the qt/python debug methods
2. handle simplewrapper in extract_enum_val
I think this stuff will need a little more cleaning up when we get to
sorting out type checking on PyQt6.
The whole key debug module seems to be a bit fuzzy about when it's going
to be passing around a simplewrapper, int and enum.
I believe we are using sip.simplerwapper as a common parent type of all
the Qt Flag/Enum values. I think it gets better on PyQt6, don't remember
though. It might just change to be sip.wrapper instead.
Mypy doesn't seem to know that QKeySequence is an iterable, so it
complains about itertools not handling it. And since we lose type
information there we have to add it back in a couple of places.
There are several cases where PyQt5 methods expects the plural flags version but
we've got the singular Enum version from accessing enum members directly.
It's not easy to turn those enums into flags and the flags don't even
exist in PyQt6.
Not sure why mypy was failing to see the inner dicts in
_PREFERRED_COLOR_SCHEME_DEFINITIONS where being seen as "object" by mypy
and not dict, I think the syntax is correct.
Add some basic type hints to help it.
They Any is because usertype.UNSET() is a sentinal object
The docs say:
> The options from QUrl::ComponentFormattingOptions are also possible.
> The FormattingOptions type is a typedef for QFlags<UrlFormattingOption>. It stores an OR combination of UrlFormattingOption values.
Maybe we should be should be definining out own types for some of the
enums that include both the QFlag, enum and any child enums.
Get rid of the per-backend classes and move the backend specific
conditionals into a common class.
Sadly it seems mypy isn't clever enough to reason ignore a class it should
know is never used. Possibly it looks at them at parse time. Probably putting
the whole class definitions into conditionals would do it but I'm not sure if
I want to go down that route for such a small example. Hopefully there aren't
too many more of these.
PyQt5 exposes Enums and their corresponding QtFlags objects seperately,
and for some reason they aren't interchangeable. ref https://github.com/python-qt-tools/PyQt5-stubs/issues/142
We could handle this by casting values back and forth between the enum
value (for working with) and the flags value (for passing to methods),
but this situation doesn't even exist with PyQt6, you can use everything
as enums just fine.
So I'm just adding ignore comments and making type definitions
conditional.
Not sure if this is the right thing to be doing or not.
for b3cdb28d044
Putting ignore[type] on the second import works fine. But if we have it
on both the pyqt5 and pyqt6 branch it'll complain about the other branch
on each run. So pull it out to a common place we can ignore.
Would be nice to have a bare `mypy` env which ran both the more specific ones
in sequence but I don't know how to do that.
Not sure if there is a way to pull the CONSTANTS_ARGS stuff out to a non-env
parameter and pass it into commands but I couldn't figure out a way. So via
the environment it is.
TODO: compare PyQt6 as-is with the WIP PyQt6-Stub
When pyright tags an object as "Unknown" (even if that is just one part of a
union of possible definitions) then VS code doesn't provide autocompletion for
that objects. Additionally, if you import that same object from multiple
modules in a file pyright (or VS Code?) will go through them in order and
prefer the last one.
1. if you import an object by name from two modules and that latter module
isn't installed the object will be marked as "Unknown"
if machinery.USE_PYQT5: # installed
from PyQt5.QtWebEngineWidgets import QWebEngineHistory
elif machinery.USE_PYSIDE2: # not installed
from PySide2.QtWebEngineWidgets import QWebEngineHistory
reveal_type(QWebEngineHistory) # class(QWebEngineHistory) | Unknown
2. if you import an object by name from a module that isn't installed and that
module isn't later overridden it'll be marked as "Unknown"
if machinery.USE_PYQT6: # installed
from from PyQt6.QtWebEngineCore import *
elif machinery.USE_PYQT5: # not installed
from PyQt5.QtWebEngineWidgets import QWebEngineHistory
reveal_type(QWebEngineHistory) # class(QWebEngineHistory) | Unknown
if machinery.USE_PYQT5: # not installed
from PyQt5.QtWebEngineWidgets import QWebEngineHistory
elif machinery.USE_PYQT6: # installed
from from PyQt6.QtWebEngineCore import *
reveal_type(QWebEngineHistory) # class(QWebEngineHistory)
So in general we want to put modules most likely to be installed at the
bottom. Except if it imports an object by name and there is a possibility it
won't be installed we want to bump it up.
So now we have:
1. PySide6: not currently supported
2. PyQt5: main supported type checking language, but not guaranteed to be
installed
3. PyQt6: quite likely to be installed but not yet supported for type checking
This means that if you have both PyQt6 and PyQt5 installed you will get
autocompletion from PyQt6. It might be better to get it from PyQt5 instead,
since that's what we will be type checking with on CI for now, but getting no
autocompletion because you only have PyQt6 installed and not PyQt5 is not
ideal.
You can confirm this by using the "go to definition" feature.
Hopefully we can use the defineConstant configuration parameter for
pyright to make this ordering less important in the future by using that
to set the proffered bindings for type checking.
We never supported PySide2 and by the time we get around to looking at
any PySide version we'll likely be near to dropping Qt5 anyway.
The main motivation however is avoiding, or being able to make use of, a
quirk of pyright. When you import the same object from two different
modules to the same attribute, and you don't have one of those modules
installed, then pyright will tag the attribute with "Unknown".
This affects us with the classes that got moved around between Qt5 and
Qt6. Since we are importing them by name for both Qt5 backends then if
you don't have one of them installed pyright will tag all those move
classes as Unkown and you won't get autocompletion with them in VS Code.
See https://github.com/qutebrowser/qutebrowser/issues/7370 for more
details.
mypy also has this issue (actually it's even worse there) but you can
shortcut the ambiguouty by setting so called "compiled time constants"
with the --always-true and --always-false command line arguments to make
it not even consider some imports. pyright has a `defineConstant`
setting for that too but it doesn't seem to apply to imports. I'll have
to raise an issue with them...
For three reasons:
- There are only 31 of them, and we don't really expect any more to
turn up (last happened in 2013, and we have a test for it happening)
- It makes for nicer debug output
- It always felt strange to only have a small subset in the enum
With Qt 6.4, it looks like the full UA disappeared from the ELF string
table - see previosly: db1382f75c
However, the full Chromium version string is still stored separately,
most likely for qWebEngineChromiumVersion() - so let's go hunt for that.
Note that this code won't be used for most situations, as with an up-to-
date PyQt, we finally have an API to access this information properly.
But it's still useful as a fallback in strange situations like running
an older PyQt against a newer Qt.
See #7314
This avoids the temptation of creating a Qt.Key() manually, which needs
to be checked for ValueError with PyQt 6.2 due to its handling of unknown enum
values.
This is exactly what happened in RegisterKeyParser, which caused such a
ValueError:
https://github.com/qutebrowser/qutebrowser/issues/7047#issuecomment-1163288560Closes#7047
Seems to be called rather infrequently (when focusing some other window
even?) but then crash with:
Traceback (most recent call last):
File ".../qutebrowser/completion/completiondelegate.py", line 323, in paint
self._draw_focus_rect()
File ".../qutebrowser/completion/completiondelegate.py", line 263, in _draw_focus_rect
o.state |= int(QStyle.StateFlag.State_KeyboardFocusChange | QStyle.StateFlag.State_Item)
TypeError: int() argument must be a string, a bytes-like object or a real number, not 'StateFlag'
The icon change doesn't seem to have any effect anymore (those environment
variables might be isolated from Chromium rendering processes since some Qt
update). The name change isn't needed anymore with QtWebEngine 5.15.2
which is now the oldest supported version.
See #3832
This raises our minimum tox version from 3.15 to 3.20 to properly
support the environment name with empty factors:
https://github.com/tox-dev/tox/issues/1636
Distribution-wise, this hopefully isn't a problem: Debian Buster
(oldstable) had tox 3.7, Debian bullseye (stable) has 3.21. Similar
story for Ubuntu: 20.04 LTS has 3.13, 21.10 (and thus 22.04 LTS) has
3.21.
We need to call .deleteLater() on the dialog when printing is done.
Otherwise, it sticks around, as confirmed by :debug-all-objects.
This might also fix the macOS segfault issue.
TODO: Verify this.
QWebEnginePage::print() changed to QWebEngineView with Qt 6, and emits a new
printFinished() signal instead of taking a callback. Adjust our API accordingly.
There also is a pdfPrintingFinished signal which we now use to get a bit more
feedback to the user.
TODO: Changelog entry for PDF feedback?
With QtWebEngine 5.15.5+, we shouldn't need the workaround anymore.
This also seems to fix flakiness in
tests/end2end/features/test_navigate_bdd.py::test_navigating_up_in_qutehelp
where sometimes no loadFinished signal was found.
Relevant commits: git log -G 65223
Partially reverts 1106d82591.
TODO: Changelog
TODO: Pick to master?
With Qt 6, the default changed to Qt handling network redirects instead of us
doing so manually. This seems like a good thing, so instead of setting the
redirect policy back to QNetworkRequest.RedirectPolicy.ManualRedirectPolicy,
let's just let Qt handle everything.
By default, Qt allows 50 redirects before giving up. That seems a tad much, so
we set it back to our former default.
This change comes with a few changes in behavior:
- Redirects to the same URL now fail (too many redirects) rather than being
ignored. I'm not sure how the previous behavior was useful. We added it in the
initial implemetation in 6856c49be9 (later
refactored a bit in 70e390a2e8) and added a test
in d13f88f0ac. But it doesn't make sense...
- We use QNetworkRequest.RedirectPolicy.NoLessSafeRedirectPolicy (no HTTPS ->
HTTP redirects allowed), while the former behavior didn't validate redirects
at all. Interestingly enough, I can't get Chromium to error out in that case
for downloads (though I only tried on localhost with a self-signed
certificate). However, it seems like a reasonable default.
Test will be added in a follow-up commit.
- Partially downloaded files aren't deleted anymore on a "too many redirect"
error. This should be solved in a more generic way, will do so in a follow-up
commit.
TODO: Pick to master?
Fixes#2679.
With scoped enums, we don't need to check whether the member belongs to the enum
class anymore, we just get it directly from there.
Also needed for PyQt 6 because we can't access the members unscoped anymore.
TODO: Pick to master?
The existing logic seems to work fine now (with some adjustments) and results in
better output compared to the repr.
Partially reverts 111e0c7f3dcccb4d0ff807854cacb6506a93e1f2.
TODO: Pick to master?
In 42e2438efb, we changed "_handle_key" to return
a QKeySequence.SequenceMatch instead of a bool.
Later in b85fe8f678, that change propagated to
"handle" too.
However, the FakeKeyParser introduced in
c1d3a94936 still returned a bool.
This wasn't a problem because we still just checked "if match:".
However, with 756db287ac4c65350aaf53d6f93b6a0a2b356a77, we now check
"if match != QKeySequence.SequenceMatch.NoMatch:", thus the comparsison
with the FakeKeyParser is now the wrong way around.
If we test for this in KeyInfo() directly (via __post_init__), all code using a
KeyInfo can be sure it has a properly split key/modifier. This is especially
important on Qt 5, where lots of Qt internals use a split value (Qt 6 introduced
QKeyCombination instead).
This however leads to different behaviour when -1 is passed as a key, that now
fails with an AssertionError. However, I believe that to be more correct:
- Testing for -1 was originally introduced in
866c758660, expecting an AssertionError.
- It was updated in d1854eddaf, still
expecting an AssertionError
- In 0ee7fac727, we changed that to KeyParseError
(in the tests only), probably in response to
78f6ad14c2.
Now that we properly check for this, it's fine to expect an AssertionError
again. I don't think we can ever get -1 from Qt anyways.
Everything in keyutils constructing a Qt.Key should catch ValueError and reraise
it as InvalidKeyError (low-level). Anything in KeySequence that calls such
methods should re-raise that as a KeyParseError to user code (high-level).
The tests skip anything that relies on constructing things with "strange" keys.
Maybe some of them could be gotten to work, but most probably won't work until
PyQt gets fixed, see e.g.:
https://www.riverbankcomputing.com/pipermail/pyqt/2022-April/044607.html
Introduce an InvalidKeyError to KeyInfo which can then also be turned
into an error in other places.
This was mainly needed to correctly handle config validation for key strings
containing characters not in Qt.Key.
Also, reduce loglevel for the message to debug, as those seem to mostly happen
with keys qutebrowser shouldn't be concerned about anyways (see #7047).
TODO: Cherry-pick to master?
Was deprecated in Qt 5.8 because a top-level QModelIndex() does not know which
model it belongs to, making .child() useless on most (flat) models:
https://codereview.qt-project.org/c/qt/qtbase/+/166180
- Make CertificateErrorWrapper responsible for accepting/rejecting certs
- Try to avoid dealing with unclear booleans
- Implement support for deferred errors (#4616) - disabled due to PyQt bug
- Implement support for Qt 6 API (#7086)
With the new approach from 54b817898f, we only
look at the values which have actually been bound by the user of the API -
however, we will miss values which have *not* been bound. Thus, we will need to
check for those seperately, with a tiny bit of version dependent code.
See https://codereview.qt-project.org/c/qt/qtbase/+/303955 for some discussion.
QApplication.desktop() (And the QDesktopWidget that returns) has been
deprecated since 5.11, we only support Qt 5.12+ (ref #3839).
The recommended alternative is to use
`QGuiApplication::screenAt(QPoint)`, but that doesn't support passing a
widget.
Stackoverflow [points out](https://stackoverflow.com/a/53490851) that
you can get to the current screen of a widget, so that's what I'm using
here.
Once you get the screen there is both `geometry()` and
`availableGeometry()` available. The later, sometimes, excludes window
manager status bars and such, which fullscreen apps should cover.
This approach works on both 5.15 and 6.2, so no version checks
necessary.
The tests now need real parent widget so that .window.WindowHandle()
works.
(cherry picked from commit 2866cae6b3)
Qt6 has switched to the global default profile being off-the-record,
which is not the default for qutebrowser.
Restore the previous default object name (although we always point it
elsewhere anyway so that name should appear in any file paths).
ref https://bugreports.qt.io/browse/QTBUG-66068
On Qt 5, .defaultProfile() is still used, to ensure the same behavior.
It's not entirely clear from the Qt 5 source what the differences are
between the two.
(cherry picked from commit b961c3b820)
The developer tools only saves preferences to disk, and reads them from
disk, if the profile of the page they are drawing to isn't
off-the-record. So if you want it to remember your dark mode preference
the page hosting the inspector needs to be linked to an on-the-record
profile. So we need to create a new page linked to a specific profile
and then set that on the view.
Options at this point:
1. (in `__init__()` always make a new page linked to the qute default
profile and add that to the view
2. delay creating the view and page until `inspect()` is called and get
a pointer to the profile of the page being inspected
(1) is actually what we have done on previous Qt versions because there
the inspector was started with WebEngine's default, global, profile,
which used to be on-the-record. I'm not sure if there are any specific
ramifications to having the inspector data persistent when the page it
is attached to isn't, but there might be stuff like per-domain
preferences saved in that case which would be an information leak.
So I've went with (2) even though it is probably going to be more
disruptive to the tests. In the future if we re-use inspectors across
different tabs/windows again we can make a new page on demand if the
profile of the inspected page differs form the initial one. For now I've
put an assert there because I am lazy.
It's a bit awkward having most of the setup in `inspect()` and not
`__init__()`. `_init_inspector()` and `inspect()` are only ever called
once right beside each other though (now) so we could just pass the page
to `__init__()` and get rid of `inspect()`.
(cherry picked from commit ec200c3486)
Based on d1eecf8b97.
Relevant parts from that commit message:
`QVariant.Type` has moved to `QMetaType.Type`[1][] and QMeta.Type
doesn't work with int().
`QtWebEngineCore.QWebEnginePage.Feature` doesn't work with int(), so add
it to the maps twice.
[1]: https://www.qt.io/blog/whats-new-in-qmetatype-qvariant
Also:
`QImage(':/icons/qutebrowser-64x64.png')` yields and empty QImage. This
is not fixed but things don't crash because of it anymore. For instance
the "Title & Body" test on https://web-push-book.gauntface.com/demos/notification-examples/
but that part seems to work fine now. I don't quite understand why the original
commit added an "or bool(icon.rect())", but I can't find anything breaking
things without.
For some reason, QtMsgType was not included and missing.
(cherry picked from commit 6afa00c465)
For completiondelegate.py, we accessed the enum members via self instead of
properly using the class.
(cherry picked from commit d37cc4ac73)
For interceptor.py, line breaks broke our script.
QKeyEvent.KeyPress was used inherited from QEvent.KeyPress, thus not
renamed.
SqlQuery.boundValues() changed in Qt6 from returning a map of
placeholders -> values to just providing a list of values.
We can either:
1. follow that change and always return a list
* this has the effect of making the return value have more items if
the caller uses a placeholder more than once in a query, like
histcategory does
2. maintain the current return type
Here I have chosen to do (2). Although (1) is an option since it looks
like no caller actually cares about the contents of the response at this
point (apart from tests), just that it is Truthy/Falsy and the right
length (if Truthy). And callers should know how many times they re-used
placeholders so should be able to adjust their length comparison themselves.
There is no API for enumerating placeholder labels anymore, if we
want to maintain our API we must either 1) store the placeholders so we
can look them up based on label later, 2) guess what they are (eg assume
the caller always used sequential integers starting at 0), or 3) always
return the dict keys as the positional indexes instead of labels (and
return a larger dict if the caller used single placeholders multiple
times).
I've chosen to do 3 which should have the same result as the 5.15
implementation, at the cost of some more list allocations.
So now we store the placeholders so we can query for them directly
later.
And the existing tests should all pass, if you can get them to run.
This approach works on both 5.15 and 6.2, so no version checks
necessary.
(cherry picked from commit 54b817898f)
With PyQt6, flags are only accessible via their flag names, not via
their enum names.
This also removes lots of cast() calls we added for mypy a while back.
We'll need to see if/how to deal with them once we can run mypy against
PyQt6.
Also fix creating empty KeyboardModifiers.
See b8f323d6a4 and
e61597ca81.
Not sure why we didn't do this initially, maybe it wasn't possible: https://webpy.org/cookbook/ssl
This means we don't need to (ab)use the Flask development server anymore.
We used to dodge these by listening on 0.0.0.0. Now they are on to us
and always show the warnings. Running flask in development mode here is
intended, so lets not have the warnings fail the tests.
Ref: https://github.com/pallets/werkzeug/issues/2480
Those commands use the config interface expecting Python objects (via
update_mutables), but we always used the raw user-supplied string as
input.
Thus far, it was very hard to trigger this bug: It would only
trigger with a List or Dict config option, with a value type which does
*not* accept a string type in to_py(). That means:
- List / FlagList / ConfirmQuit / ShellCommand
- Bool / BoolAsk
- Int
- Float
- Dict / Padding
(Notably, Perc, PercOrInt and Regex all *do* accept a string.)
That leaves only a couple of candidates:
- hints.selectors, but that's "no_autoconfig: true"
- bindings.default, ditto
- bindings.commands, but no reason to use :config-dict-* on it,
especially with fixed keys
Therefore, this got only uncovered now, after adding
content.javascript.log_message.levels, which is a Dict with FlagList
values.
Note that we still don't have any config definition with an affected
List type, thus currently making it impossible to test the changes for
:config-list-{add,remove}.
Also note the adjusted test_{dict,list}_add_invalid_value tests - they
were just plain wrong. The command functions are never going to be
called by a Python object from the user (only with a str).
Finally, test_list_remove_no_value also needed an adjustment because
the value validation now happens *before* the other validation done
by the command.
If we only clear existing mutables *after* applying, we get into an
inconsistent state if there was an error in one of the config values:
The improper value lingers around in self._mutables, and then gets
returned when get_mutable_obj() (or update_mutables()) gets called the
next time.
Reproducer:
qutebrowser --debug --temp-basedir \
':config-dict-add content.javascript.log_message.levels example.org bla' \
':later 1000 config-dict-add content.javascript.log_message.levels example.org bla'
Results in:
ERROR: Invalid value 'bla' - expected a value of type list but got str.
ERROR: example.org already exists in content.javascript.log_message - use --replace to overwrite!
Fixes the second part of #7343.
nb: As before, the mutable updating actually gets interrupted by a
failing update, instead of it e.g. collecting all errors but carrying
on. With this change, the remaining updates will thus also be discarded,
but that does not seem to be a problem with how mutables are currently
used. Ideally, we should get rid of the mutable handling entirely
anyways, at least for qutebrowser internal code - see #4344.
Those are handled separately from the other settings, but were never initialized
properly in init_settings().
For content.javascript.clipboard, this is a recent regression introduced in
4a6df3a8e8. For content.default_encoding, this has
been around since 2018: 3956f81e73.
Fixes#7281
The decision in a64c3d0dfc to return from
_verify_message() when there is a non-fatal error (later expanded to
more than just NoReply) was a poor one:
While the error signal indeed takes care of swapping out the faulty
adapter, the direct caller of _verify_message generally expects the
message to be in the verified shape (e.g. having 4 arguments, or an int
as argument, etc.). Thus, every caller would have to handle this
situation, yet none of them did!
With the restructured code, we now *always* raise an exception. It's
still the callers responsibility to deal with that happening, but that's
much less tricky than just pretending we validated the data when we did
not.
Thankfully, most caller already handle the situation, or don't need to:
- _get_server_info() and _fetch_capabilities() get called from __init__,
where any notification.Error is already handled by
NotificationBridgePresenter.
- _handle_close() and _handle_action() get called as DBus signals by Qt.
It's hard to imagine how they would ever get an error reply, as the
caller is the other side (the notification server), not us!
This only leaves present(), which now handles this case: If it gets a
fatal exception it still gets raised, but for any non-fatal ones, we now
emit the error signal there and return a dummy value.
Fixes#6931
As well as:
- replace external links with internal links for changelog and
quickstart
- remove unused class names
- use already existing logo
- replace `url` with `_url` to mark it unused
When e.g. doing:
- '?foo' (search with reverse=True -> FindBackwards)
- 'N' (prev_result -> no FindBackwards)
- 'n' (next_result -> FindBackwards again)
we need to take a copy of the flags so that we can temporarily clear
FindBackwards when pressing 'N'.
Relevant history:
- We originally did int(self._flags) in
d450257485.
- In f0da508c21, we used
QWebPage.FindFlags(int(self._flags)) instead.
- With 97bdcb8e674c8ff27ab92448effef263880ab3aa (picked from
c349fbd180) we instead do:
flags = QWebEnginePage.FindFlag(self._flags)
Using FindFlag instead of FindFlags seemed to work fine with PyQt6 and
enum.Flag. With PyQt5, however, later clearing a flag bit ends up with us
getting 0 as an integer, thus losing the type information about this being a
FindFlag instance, and resulting in a TypeError when calling into Qt.
We could use FindFlags() with PyQt 6 but FindFlag() with PyQt 5 to copy the
original flags, but that's getting rather cumbersome. Instead, let's have a
helper dataclass of bools, do away with the bit-twiddling, and only convert it
to a Qt flags when we actually need them. This solves the copying issue nicely,
and also makes the code a lot nicer.
Finally, this also adds a test case which fails when the flags are mutated in
place instead of copied.
We could do the same kind of change for QtWebKit as well, but given that it's
somewhat dead - and perhaps more importantly, won't run with Qt 6 - let's not
bother. To not break the end2end tests with QtWebKit, the output still is the
same as before.
(cherry picked from commit 96a0cc39512753445bc7a01b218b2f1290819ddd)
The fact that we need to specify this while searching rather than when
"zapping" through the results make no sense. It makes both the API as
well as our own code more complex.
When a herbe notification was right-clicked ("accepted" according to
the herbe readme), it signals that by exiting with exit status 2.
So far, we've only signalled that by emitting click_id to the bridge
(thus letting the website know about it being clicked), but we never
communicated that it was also closed by the same action.
When right-clicking a Discord notification, this meant that the
following happened:
- herbe exits with status 2
- We emit click_id, thus communicating to Discord that the notification
was clicked.
- Discord calls .close() (in JS) on the notification.
- Our on_web_closed gets called and tries to send a SIGUSR1 to herbe to dismiss
the notification.
- However, the herbe process already exited at this point, thus leading
to:
Traceback (most recent call last):
File ".../qutebrowser/browser/webengine/notification.py", line 639, in on_web_closed
os.kill(notification_id, signal.SIGUSR1)
ProcessLookupError: [Errno 3] No such process
If we make it clear that the notification is already gone, and
on_web_closed does not seem to be called at all anymore.
With qt5-webengine 5.15.9-3 on Arch Linux, there only was a rebuild:
70aa541b4f
but somehow, we now have some kind of garbage data in the .data section:
...
\x00QtWebEngine/5.15.9 Chrome/87.0.4xternalclearkey
\x00ernalclearkey.diy.differentguid
\x00Portable Documen/usr/src/debug/qtwebengine/src/core/net/proxying_restricted_cookie_manager_qt.cpp
\x00
...
the *actual* string table only seems to follow much later:
...
\x00display
\x00\x00dispatchCallbackOnIOThread
\x00/Cache
\x00DownloadInterruptReason
\x00General network failure
\x00The server has gone down
\x00General server failure
\x00Unexpected server response
\x00Download canceled by the user
\x00shutdownOnUIThread
\x00qrc://
\x00RequestQuotaPermission
\x00\x00\x00\x00/usr/src/debug/qtwebengine/src/core/quota_permission_context_qt.cpp
\x00\x00\x00\x00\x00QtWebEngine/5.15.9 Chrome/87.0.4280.144
\x00 - %04d-%02d-%02dT%0"
...
So let's include the NUL bytes in our regex, to make sure we get the full
variant, not the garbage.
(cherry picked from commit 511df8af21ed18a65c49881175814efa72329754)
Currently the session-save commande make a dump of all tabs history and stores
them in the session file. --minimal flag adds the option to store only the last
item of the history.
Signed-off-by: shirenn <shirenn@crans.org>
See https://github.com/PyCQA/astroid/pull/1550 for the signal
disconnect.
Not reporting the gen_classes() one, as we have another ignore for that,
and doing something a bit unorthodox there anyways.
This is a follow-up to 0d08e70de2. Apparently
there were some bits remaining after that, which are rather confusing.
The return values don't get read anywhere, so we can just yank it all out.
The previous solution with making AbstractInspector know about the concrete
inspector types results in Liskov issues for the backend-specific overrides:
https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
So, for now, let's just declare them as Any. Perhaps we should have a
typing.Protocol to unify the two, as they don't share a common base class.
See #7098.
For some unknown reason, those new stubs cause a *lot* of things now to be
checked by mypy which formerly probably got skipped due to Any being implied
somewhere.
The stubs themselves mainly improved, with a couple of regressions too.
In total, there were some 337 (!) new mypy errors. This commit fixes almost all
of them, and the next commit improves a fix to get things down to 0 errors
again.
Overview of the changes:
==== qutebrowser/app.py
- Drop type ignore due to improved stubs.
==== qutebrowser/browser/browsertab.py
- Specify the type of _widget members more closely than just QWidget.
This is debatable: I suppose the abstract stuff shouldn't need to know
anything about the concrete backends at all. But it seems like we cut some
corners when initially implementing things, and put some code in browsertab.py
just because the APIs of both backends happened to be compatible. Perhaps
something to reconsider once we drop QtWebKit and hopefully implement a dummy
backend.
- Add an additional assertion in AbstractAction.run_string. This is already
covered by the isinstance(member, self.action_base) above it, but that's too
dynamic for mypy to understand.
- Fix the return type of AbstractScroller.pos_px, which is a QPoint (with x
and y components), not a single int.
- Fix the return type of AbstractScroller.pos_perc, which is a Tuple (with x
and y components), not a single int.
- Fix the argument types of AbstractScroller.to_perc, as it's possible to pass
fractional percentages too.
- Specify the type for AbstractHistoryPrivate._history. See above (_widget) re
this being debatable.
- Fix the return type of AbstractTabPrivate.event_target(), which can be None
(see #3888).
- Fix the return type of AbstractTabPrivate.run_js_sync, which is Any (the JS
return value), not None.
- Fix the argument type for AbstractTabPrivate.toggle_inspector: position can
be None to use the last used position.
- Declare the type of sub-objects of AbstractTab.
- Fix the return value of AbstractTab.icon(), which is the QIcon, not None.
==== qutebrowser/browser/commands.py
- Make sure the active window is a MainWindow (with a .win_id attribute).
==== qutebrowser/browser/downloadview.py
- Add _model() which makes sure that self.model() is a DownloadModel, not None
or any other model. This is needed because other methods access a variety of
custom attributes on it, e.g. last_index().
==== qutebrowser/browser/greasemonkey.py
- Add an ignore for AbstractDownload.requested_url which we patch onto the
downloads. Probably would be nicer to add it as a proper attribute which always
gets set by the DownloadManager.
==== qutebrowser/browser/hints.py
- Remove type ignores for QUrl.toString().
- Add a new type ignore for combining different URL flags (which works, but is
not exactly type safe... still probably a regression in the stubs).
- Make sure the things we get back from self._get_keyparser are what we actually
expect. Probably should introduce a TypedDict (and/or overloads for
_get_keyparser with typing.Literal) to teach mypy about the exact return value.
See #7098.
This is needed because we access Hint/NormalKeyParser-specific attributes such
as .set_inhibited_timout() or .update_bindings().
==== qutebrowser/browser/inspector.py
- Similar changes than in browsertab.py to make some types where we share API
(e.g. .setPage()) more concrete. Didn't work out unfortunately, see next
commit.
==== qutebrowser/browser/network/pac.py
- Remove now unneeded type ignore for signal.
==== qutebrowser/browser/qtnetworkdownloads.py
- Make sure that downloads is a qtnetworkdownloads.DownloadItem (rather than an
AbstractDownload), so that we can call ._uses_nam() on it.
==== qutebrowser/browser/qutescheme.py
- Remove now unneeded type ignore for QUrl flags.
==== qutebrowser/browser/urlmarks.py
- Specify the type of UrlMarkManager._lineparser, as those only get initialized
in _init_lineparser of subclasses, so mypy doesn't know it's supposed to exist.
==== qutebrowser/browser/webelem.py
- New casts to turn single KeyboardModifier (enum) entries into
KeyboardModifiers (flags). Might not be needed anymore with Qt 6.
- With that, casting the final value is now unneeded.
==== qutebrowser/browser/webengine/notification.py
- Remove now unneeded type ignore for signal.
- Make sure the self.sender() we get in HerbeNotificationAdapter._on_finished()
is a QProcess, not just any QObject.
==== qutebrowser/browser/webengine/webenginedownloads.py
- Remove now unneeded type ignores for signals.
==== qutebrowser/browser/webengine/webengineelem.py
- Specify the type of WebEngineElement._tab.
- Remove now unneeded type ignore for mixed flags.
==== qutebrowser/browser/webengine/webengineinspector.py
- See changes to inspector.py and next commit.
- Remove now unneeded type ignore for signal.
==== qutebrowser/browser/webengine/webenginequtescheme.py
- Remove now unneeded type ignore for mixed flags.
==== qutebrowser/browser/webengine/webenginesettings.py
- Ignore access of .setter attribute which we patch onto QWebEngineProfile.
Would be nice to have a subclass or wrapper-class instead.
==== qutebrowser/browser/webengine/webenginetab.py
- Specified the type of _widget members more closely than just QWidget.
See browsertab.py changes for details.
- Remove some now-unneeded type ignores for creating FindFlags.
- Specify more concrete types for WebEngineTab members where we actually need to
access WebEngine-specific attributes.
- Make sure the page we get is our custom WebEnginePage subclass, not just any
QWebEnginePage. This is needed because we access custom attributes on it.
==== qutebrowser/browser/webengine/webview.py
- Make sure the page we get is our custom WebEnginePage subclass, not just any
QWebEnginePage. This is needed because we access custom attributes on it.
==== qutebrowser/browser/webkit/network/networkreply.py
- Remove now unneeded type ignores for signals.
==== qutebrowser/browser/webkit/webkitinspector.py
- See changes to inspector.py and next commit.
==== qutebrowser/browser/webkit/webkittab.py
- Specify the type of _widget members more closely than just QWidget.
See browsertab.py changes for details.
- Add a type ignore for WebKitAction because our workaround needs to
treat them as ints (which is allowed by PyQt, even if not type-safe).
- Add new ignores for findText calls: The text is a QString and can be None; the
flags are valid despite mypy thinking they aren't (stubs regression?).
- Specify the type for WebKitHistoryPrivate._history, because we access
WebKit-specific attributes. See above (_widget) re this being debatable.
- Make mypy aware that .currentFrame() and .frameAt() can return None (stubs
regression?).
- Make sure the .page() and .page().networkAccessManager() are our subclasses
rather than the more generic QtWebKit objects, as we use custom attributes.
- Add new type ignores for signals (stubs regression!)
==== qutebrowser/browser/webkit/webpage.py
- Make sure the .networkAccessManager() is our subclass rather than the more
generic QtWebKit object, as we use custom attributes.
- Replace a cast by a type ignore. The cast didn't work anymore.
==== qutebrowser/browser/webkit/webview.py
- Make sure the .page() is our subclass rather than the more generic QtWebKit
object, as we use custom attributes.
==== qutebrowser/commands/userscripts.py
- Remove now unneeded type ignore for signal.
==== qutebrowser/completion/completer.py
- Add a new _completion() getter (which ensures it actually gets the completion
view) rather than accessing the .parent() directly (which could be any QObject).
==== qutebrowser/completion/completiondelegate.py
- Make sure self.parent() is a CompletionView (no helper method as there is only
one instance).
- Remove a now-unneeded type ignore for adding QSizes.
==== qutebrowser/completion/completionwidget.py
- Add a ._model() getter which ensures that we get a CompletionModel (with
custom attributes) rather than Qt's .model() which can be any QAbstractItemModel
(or None).
- Removed a now-unneeded type ignore for OR-ing flags.
==== qutebrowser/completion/models/completionmodel.py
- Remove now unneeded type ignores for signals.
- Ignore a complaint about .set_pattern() not being defined. Completion
categories don't share any common parent class, so it would be good to introduce
a typing.Protocol for this. See #7098.
==== qutebrowser/components/misccommands.py
- Removed a now-unneeded type ignore for OR-ing flags.
==== qutebrowser/components/readlinecommands.py
- Make sure QApplication.instance() is a QApplication (and not just a
QCoreApplication). This includes the former "not None" check.
==== qutebrowser/components/scrollcommands.py
- Add basic annotation for "funcs" dict. Could have a callable protocol to
specify it needs a count kwarg, see #7098.
==== qutebrowser/config/stylesheet.py
- Correctly specify that stylesheet apply to QWidgets, not any QObject.
- Ignore an attr-defined for obj.STYLESHEET. Perhaps could somehow teach mypy
about this with overloads and protocols (stylesheet for set_register being None
=> STYLESHEET needs to be defined, otherwise anything goes), but perhaps not
worth the troble. See #7098.
==== qutebrowser/keyinput/keyutils.py
- Remove some now-unneeded type ignores and add a cast for using a single enum
value as flags. Might need to look at this again with Qt 6 support.
==== qutebrowser/keyinput/modeman.py
- Add a FIXME for using a TypedDict, see comments for hints.py above.
==== qutebrowser/mainwindow/mainwindow.py
- Remove now-unneeded type ignores for calling with OR-ed flags.
- Improve where we cast from WindowType to WindowFlags, no int needed
- Use new .tab_bar() getter, see below.
==== qutebrowser/mainwindow/prompt.py
- Remove now-unneeded type ignores for calling with OR-ed flags.
==== qutebrowser/mainwindow/statusbar/bar.py
- Adjust type ignores around @pyqtProperty. The fact one is still needed seems
like a stub regression.
==== qutebrowser/mainwindow/statusbar/command.py
- Fix type for setText() override (from QLineEdit): text can be None
(QString in C++).
==== qutebrowser/mainwindow/statusbar/url.py
- Adjust type ignores around @pyqtProperty. The fact one is still needed seems
like a stub regression.
==== qutebrowser/mainwindow/tabbedbrowser.py
- Specify that TabDeque manages browser tabs, not any QWidgets. It accesses
AbstractTab-specific attributes.
- Make sure that the .tabBar() we get is a tabwidget.TabBar, as we access
.maybe_hide.
- Fix the annotations for stored marks: Scroll positions are a QPoint, not int.
- Add _current_tab() and _tab_by_idx() wrappers for .currentWidget() and
.widget(), which ensures that the return values are valid AbstractTabs (or None
for _tab_by_idx). This is needed because we access AbstractTab-specific
attributes.
- For some places, where the tab can be None, continue using .currentTab() but
add asserts.
- Remove some now-unneeded [unreachable] ignores, as mypy knows about the None
possibility now.
==== qutebrowser/mainwindow/tabwidget.py
- Add new tab_bar() and _tab_by_idx() helpers which check that the .tabBar() and
.widget() are of type TabBar and AbstractTab, respectively.
- Add additional assertions where we expect ._tab_by_idx() to never be None.
- Remove dead code in get_tab_fields for handling a None y scroll position. I
was unable to find any place in the code where this could be set to None.
- Remove some now-unneeded type ignores and casts, as mypy now knows that
_type_by_idx() could be None.
- Work around a strange instance where mypy complains about not being able to
find the type of TabBar.drag_in_progress from TabWidget._toggle_visibility,
despite it clearly being shown as a bool *inside* that class without any
annotation.
- Add a ._tab_widget() getter in TabBar which ensures that the .parent() is in
fact a TabWidget.
==== qutebrowser/misc/crashsignal.py
- Remove now unneeded type ignores for signals.
==== qutebrowser/misc/editor.py
- Remove now unneeded type ignores for signals.
==== qutebrowser/misc/ipc.py
- Remove now unneeded type ignores for signals.
- Add new type ignores for .error() which is both a signal and a getter
(stub regression?). Won't be relevant for Qt 6 anymore, as the signal was
renamed to errorOccurred in 5.15.
==== qutebrowser/misc/objects.py
- Make sure mypy knows that objects.app is our custom Application (with custom
attributes) rather than any QApplication.
==== qutebrowser/utils/objreg.py
- Ignore attr-defined for .win_id attributes. Maybe could add a typing.Protocol,
but ideally, the whole objreg stuff should die one day anyways.
==== tests/unit/completion/test_completer.py
- Make CompletionWidgetStub inherit from CompletionView so that it passes the
new isinstance() asserts in completer.py (see above).
Fix qute-bitwarden userscript URL matching for multi level subdomains
The qute-bitwarden userscript is not able to autofill for a multi-level subdomain URLs (e.g. `http://my.sub.domain.local/`).
qute-bitwarden exits with status code 2 and returns the following on stderr:
```
No pass candidates for URL 'http://my.sub.domain.local/' found!
```
Joining the extracted subdomain and domain with a `.` solves the issue.
Hi, Tom from Codecov here. I noticed that you were using Codecov but weren't actually getting any notifications on pull requests. I figured it would be useful to get some idea if code being changed is being tested, but also not blocking CI/merging. Let me know if this makes sense or if we can do something that would be helpful.
Although this is just the version published to pypi. I don't know what
version sip is using. The version published on pypi is apparently stable
and mature and any further changes on github are simplifications that
the author doesn't particularly want to support.
I don't know if we actually hit the parsing code in our workflows but it
is published as a dependancy.
The latest version talks about removing optional stuff from the API a
bunch. But sip just uses `from ply import lex, yacc` so hopefully that
is a stable part of the API. Even if sip is using the newer one.
This got moved to QSslConfiguration in Qt 5.5:
https://codereview.qt-project.org/c/qt/qtbase/+/113886
(92cda9474245c79b635c21cd140c5d0a3a6d2e5b in qtbase)
(cherry picked from commit 317da1e3cf23bf40d24d186cd6d06b6bc9a09958)
The PyQt resources system is gone in 6.2 and deprecated before that. This
should be the last usage of it.
Switches icons to be read with `utils.resources.read_file_binary()` in
`notification.py` (fallback desktop notification icon) and `app.py` (icon for
the desktop window).
importlib only loads resources under a package, so the icons are moved under
the `qutebrowser/` directory.
Closes: #6062
The test case on the bug works for me on py37 but CI is failing on 3.10,
maybe this isn't quite the same issue. Anyway, we are getting rid of
webkit soon.
Then regenerate the relevant files.
Also drop dataclasses from requirements files.
TODO: should we drop the dataclasses-types requirement for mypy too?
Commits for dropping 3.5 support to copy from:
c245b7d855ccd "Initial drop of Python 3.5"
ccdfb44b85 "Drop support for Python 3.6.0"
Anything needed to update regarding OS version support in
doc/install.asciidoc?
TODO: remove 3.6/7 annotations in requirements files and
rebuild
workflows: not sure I updated it right (run 5.12 with 3.7, same 18.04 OS) but
18.04 seems to have 3.7 on it too so it should work. It'll all change when we
drop <5.15 anyway. Not sure what the minimum ubuntu version will be going
forward.
Regarding mimetype overrides (ebb3046822) the doctring says they can all go
in 3.7 but .h5 is still missing on py39, not sure if we should care.
There are a bunch of old(?) warning messages still ignored in tests/end2end/fixtures/quteprocess.py.
If we have a builtin importlib.metadata (Python 3.8+) and the importlib_metadata
backport installed, we preferred the backport. However, the version.py tests do
the opposite: They only mock the builtin if it is available. This did lead to
failing tests if the backport was installed in an environment where the builtin
was available too.
Since we don't need any specialized functionality (only reading the version), we
can prefer the builtin no matter whether a backport is available or not.
Refactor the magic tag creation thing to add python version checking support.
Makes `_check_version()` support checking plain tuples to so that I don't have
to copy the operator dict.
Now most of the branches of the if/else are the same, meh.
In 4b93da6c69 I moved a cache that was registered with the
debugcache module to be per-window. Which means they may be deleted at
some point and we shouldn't hold strong references to them.
Flake8-bugbear correctly pointed out that TabBar instances would not be
reliably cleaned up because the `self` reference would be cached in the
lru_caches on a couple of the methods. And the caches are on the class
so they last for the whole lifetime of the process.
This commit move the caches to be created per instance, instead of on
the class.
Other options:
1. get rid of the caches
From running the benchmark tests (eg `python3 -m pytest --benchmark-columns Min,Max,Median,Rounds -k test_update_tab_titles_benchmark`)
it seems like the caches can still be helpful (even though when they
were introduced in #3122 we didn't have the config cache either)
2. use cachetools to manage our own cache instead of using lru_cache in
a non-standard way
I don't feel like introducing a new dependency given this change didn't
end up being too offensive.
3. clear the cache whenever a window get closed
That would solve the "not getting deleted issue" but flake8 would still
complain :)
Possibly the cache size could be reduced now that there is going to be
one per window. But the aren't caching large objects anyway.
Flake8-bugbear change: https://github.com/PyCQA/flake8-bugbear/pull/218
Video that pointed out this way of using lru_cache: https://youtu.be/sVjtp6tGo0g
I am making the yt-dl program used in the `cast` userscript
configurable, via a pattern borrowed from some of the other
userscripts here (e.g. `password_fill` and `kodi`): we look for an
optional "cast_rc" file in the qutebrowser config directory and source
it if it exists.
(Technically this allows for overriding any variables used in `cast`,
but this is in line with how the pattern works in the other scripts
already.)
If the config file is not found, we default to yt-dlp, and if that
doesn't exist then youtube-dl. If after all this no program is
available, we emit an error message
(note, the error messaging function as currently written in the cast script
seems broken and doesn't display the full error message, but fixing this
existing bug is outside of the scope of this change. May be good for a
followup).
I recognize there's some danger of breakage for some users by switching
the default to yt-dlp, but I think it's reasonable to assume that almost
everybody who has yt-dlp installed would prefer it to be used anyway.
Those who don't will experience no difference.
The `cast` userscript hasn't worked for me in a while, because it attempts to launch youtube-dl which has been replaced by yt-dlp for some time. Switching this one command gets `cast` working for me.
I am reverting these changes, as they are causing the input buttons
to not work properly anymore.
I am adding a pointer cursor for the hidden radio button as well.
I wonder how many else will fail.
Also it would be nice if the compiled requirements files said where the
requirements come from, as in what they are dependencies of, if
anything. Like pip-compile does.
Hopefully that works. This was confusing. Grepping for 3.6 to see how
other places pin it there were a bunch of normal pins in .txt files and
then some comment-but-actually-instruction comments in the -raw files.
There is a readme in the requirements folder but it just says what
instructions are allowed. Not why you would want to add them, seems to
very high cognitive load setup.
Trying to run the recompile_requirements.py script it did some stuff
then quit because it couldn't find python3.7. Yeah I don't have that
installed. It doesn't try to use pyenv or anything, just runs venv
expecting you have everything installed?
Turns out you take the name of the requirements file you added the
instructions to (eq requirements-test.txt-raw -> test) and run it with
that as argument. The requirements file you modify should be the highest
level one that mentions the requirement you want to pin.
Anyway I edited the qutebrowser.text-raw one because that seems like the
main one and it mentions MarkupSafe (because of jinja2?) and ran
`scripts/dev/recompile_requirements qutebrowser` in my existing venv.
Lets see what the CI thinks.
Also got lots of `pkg_resources==0.0.0` in a bunch of requirements files
which seems problematic.
Show only the first line of the description of the setting. This makes all the settings the same size,
giving a better overview. Display or hide the description according to
the button click.
As delete_tab was assuming that completion column contains window ID, it was showing
exception in case of tab-focus, as it doesn't have the window ID in completion column.
So instead a new parameter named current_win_id is used in _tabs which is also passed
in all uses of the function.
This allows it to work on Wayland (my use case), but also opens up the
possibility of using the system keychain to manage the password, or any
other method the user might want.
By default numbers are interpreted as counts for bindings. Making this
behavior configurable allows for emacs-like bindings, where number keys
are passed through.
In order to fix the issue of silently using QtWebEngine when e.g.
--qute-bdd-backend=webkit is given, even though QtWebEngine is not
available, I moved the selection logic into separate functions to
clear things up a little.
I tried to avoid the duplicate imports, in case the backend is
auto-selected, but after a while of thinking I abandoned that idea in
favor of moving forward with this.
The following commit contains updates of CSS and a bit of JS:
- font update
- lists are represented as buttons
- color update
- buttons work with f hint search
One can override the auto-detection mechanism by passing
the backend via --qute-bdd-backend=<backend> or by setting the
environment variable QUTE_BDD_BACKEND=<backend>.
For now this just serves the purpose of making the information available
to the pytest_report_header() hook, where we want to report, which
backend is used when running tests.
Test originally expected "Baz" instead of "baz", which is what should
actually be found if starting at the bottom and then searching forward
(wrapping around).
Reset flags when the user searches for the same text multiple times,
instead of completely ignoring it.
This fixes an issue where searching forward, and then searching backward
for the same text, would not reset the search direction.
Add an optional boolean argument to `tabbed_browser.close_tab()` called
`transfer` which indicates whether the tab is closing as a result of
being given to another window (`tab-give`) or taken by another window
(`tab-take`). If so, the tab will always close, even if it is the last
tab in the window and `tabs.last_close` is not set to 'close'.
Seems to be still needed, at least on macOS. Otherwise, we get a
NotADirectoryError trying to load resources from a weird path under the
qutebrowser executable.
See #6792
This was originally intended as a fix for CVE-2021-41146, but it turned out the
same exploit works via e.g. https:// just as well. Still, it makes sense to
remove it.
On Windows, if an application is registered as an URL handler like this:
HKEY_CLASSES_ROOT
https
URL Protocol = ""
[...]
shell
open
command
(Default) = ".../qutebrowser.exe" "%1"
one would think that Windows takes care of making sure URLs can't inject
arguments by containing a quote. However, this is not the case, as
stated by the Microsoft docs:
https://docs.microsoft.com/en-us/previous-versions/windows/internet-explorer/ie-developer/platform-apis/aa767914(v=vs.85)
Security Warning: Applications that handle URI schemes must consider how to
respond to malicious data. Because handler applications can receive data
from untrusted sources, the URI and other parameter values passed to the
application may contain malicious data that attempts to exploit the handling
application.
and
As noted above, the string that is passed to a pluggable protocol handler
might be broken across multiple parameters. Malicious parties could use
additional quote or backslash characters to pass additional command line
parameters. For this reason, pluggable protocol handlers should assume that
any parameters on the command line could come from malicious parties, and
carefully validate them. Applications that could initiate dangerous actions
based on external data must first confirm those actions with the user. In
addition, handling applications should be tested with URIs that are overly
long or contain unexpected (or undesirable) character sequences.
Indeed it's trivial to pass a command to qutebrowser this way - given how
trivial the exploit is to recreate given the information above, here's a PoC:
https:x" ":spawn calc
(or qutebrowserurl: instead of https: if qutebrowser isn't registered as a
default browser)
Some applications do escape the quote characters before calling
qutebrowser - but others, like Outlook Desktop or .url files, do not.
As a fix, we add an --untrusted-args flag and some early validation of the raw
sys.argv, before parsing any arguments or e.g. creating a QApplication (which
might already allow injecting Qt flags there).
We assume that there's no way for an attacker to inject flags *before* the %1
placeholder in the registry, and add --untrusted-args as the last argument of
the registry entry. This way, it'd still be possible for users to customize
their invocation flags without having to remove --untrusted-args.
After --untrusted-args, however, we have some rather strict checks:
- There should be zero or one arguments, but not two (or more)
- Any argument may not start with - (flag) or : (qutebrowser command)
We also add the --untrusted-args flag to the Linux .desktop file, though it
should not be needed there, as the specification there is sane:
https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables
Implementations must take care not to expand field codes into multiple
arguments unless explicitly instructed by this specification. This means
that name fields, filenames and other replacements that can contain spaces
must be passed as a single argument to the executable program after
expansion.
There is no comparable mechanism on macOS, which opens the application without
arguments and then sends an "open" event to it:
https://doc.qt.io/qt-5/qfileopenevent.html
This issue was introduced in qutebrowser v1.7.0 which started registering it as
URL handler: baee288890 / #4086
This is by no means an issue isolated to qutebrowser. Many other projects have
had similar trouble with Windows' rather unexpected behavior:
Electron / Exodus Bitcoin wallet:
- http://web.archive.org/web/20190702112128/https://medium.com/0xcc/electrons-bug-shellexecute-to-blame-cacb433d0d62
- https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-1000006
- https://medium.com/hackernoon/exploiting-electron-rce-in-exodus-wallet-d9e6db13c374
IE/Firefox:
- https://bugzilla.mozilla.org/show_bug.cgi?id=384384
- https://bugzilla.mozilla.org/show_bug.cgi?id=1572838
Others:
- http://web.archive.org/web/20210930203632/https://www.vdoo.com/blog/exploiting-custom-protocol-handlers-in-windows
- https://parsiya.net/blog/2021-03-17-attack-surface-analysis-part-2-custom-protocol-handlers/
- etc. etc.
See CVE-2021-41146 / GHSA-vw27-fwjf-5qxm:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-41146https://github.com/qutebrowser/qutebrowser/security/advisories/GHSA-vw27-fwjf-5qxm
Thanks to Ping Fan (Zetta) Ke of Valkyrie-X Security Research Group
(VXRL/@vxresearch) for finding and responsibly disclosing this issue.
The default behaviour of qute-pass userscript will insert a password
automatically if there is only a single candidate found. This option will
force the selection prompt (i.e. dmenu) to show regardless of how many
password candidates are found.
$ .../.tox/py310/bin/python scripts/link_pyqt.py --tox .tox/py310
<string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
<string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead
See https://bugs.python.org/issue41282
If we're not waiting for the async INSERT in the SQL database, it can happen
that :debug-dump-history gets called before the history entry was addeded to the
actual database.
See #5390
For example if I type "Download" the Downloads folder will show up. Not quite fuzzyfinding. However, there is a bug where the first character typed after the / for example the 'z' in '/home/z' will remove all folders until the next character is typed.
I just added the readability package to the [AUR](https://aur.archlinux.org/packages/nodejs-readability-git/).
Not much of a difference from using npm directly, but, personally, (i) I prefer to manage all my (important) packages using yay, and (ii) like to perform all system upgrades from one place (i.e. yay --devel).
We need to check for the QtWebEngine version, not for the version of Qt.
Additionally, there's no need to re-check in DBusNotificationAdapter.__init__ as
it never gets instantiated on older versions, so it's now an assertion instead.
2c4bb064e introduced support for showing bindings in the completion menu
for commands initiated with set-cmd-text. This would crash if given a
binding for just 'set-cmd-text' with no args.
Fixes#6453.
Please report security bugs to [security@qutebrowser.org](mailto:security@qutebrowser.org).
(or if GPG encryption is desired, contact me@the-compiler.org with GPG ID [0x916EB0C8FD55A072](https://www.the-compiler.org/pubkey.asc)).
Alternatively, [report a vulnerability](https://github.com/qutebrowser/qutebrowser/security/advisories/new) via GitHub's [private reporting feature](https://docs.github.com/en/code-security/security-advisories/guidance-on-reporting-and-writing/privately-reporting-a-security-vulnerability).
* https://fanglingsu.github.io/vimb/[vimb] (C, GTK+ with WebKit2)
* https://luakit.github.io/luakit/[luakit] (C/Lua, GTK+ with WebKit2)
* https://nyxt.atlas.engineer/[Nyxt browser] (formerly "Next browser", Lisp, Emacs-like but also offers Vim bindings, QtWebKit or GTK+/WebKit2 - note there was a https://jgkamat.gitlab.io/blog/next-rce.html[critical remote code execution] which was handled quite badly)
* https://luakit.github.io/[luakit] (C/Lua, GTK+ with WebKit2)
* https://nyxt.atlas.engineer/[Nyxt browser] (formerly "Next browser", Lisp, Emacs-like but also offers Vim bindings, QtWebEngine or GTK+/WebKit2 - note there was a https://jgkamat.gitlab.io/blog/next-rce.html[critical remote code execution in 2019] which was handled quite badly)
* https://vieb.dev/[Vieb] (JavaScript, Electron)
* https://surf.suckless.org/[surf] (C, GTK+ with WebKit1/WebKit2)
* https://github.com/jun7/wyeb[wyeb] (C, GTK+ with WebKit2)
* Chrome/Chromium addons:
https://vimium.github.io/[Vimium],
https://github.com/dcchambers/vb4c[vb4c] (fork of cVim)
- chrome://conversion-internals/ (QtWebEngine >= 5.15.3 and < 6.4)
- chrome://indexeddb-internals/
- chrome://media-internals/
- chrome://network-errors/
- chrome://net-internals/ (QtWebEngine >= 5.15.4)
- chrome://process-internals/
- chrome://quota-internals/
- chrome://serviceworker-internals/
- chrome://webrtc-internals/
Crash/hang pages:
- chrome://crash/ (crashes the current renderer process!)
- chrome://kill/ (kills the current renderer process!)
- chrome://gpuclean/ (crashes the current renderer process!)
- chrome://gpucrash/ (crashes qutebrowser!)
- chrome://gpuhang/ (hangs qutebrowser!)
- chrome://gpuclean/ (crashes the current renderer process!)
- chrome://ppapiflashcrash/
- chrome://ppapiflashhang/
- chrome://quota-internals/
- chrome://taskscheduler-internals/
- chrome://sandbox/ (Linux only)
- chrome://kill/ (kills the current renderer process!)
QtWebEngine internals
~~~~~~~~~~~~~~~~~~~~~
@ -595,11 +646,14 @@ This is mostly useful for qutebrowser maintainers to work around issues in Qt -
The hierarchy of widgets when QtWebEngine is involved looks like this:
- qutebrowser has a `WebEngineTab` object, which is its abstraction over QtWebKit/QtWebEngine.
- The `WebEngineTab` has a `_widget` attribute, which is the https://doc.qt.io/qt-5/qwebengineview.html[QWebEngineView]
- That view has a https://doc.qt.io/qt-5/qwebenginepage.html[QWebEnginePage] for everything which doesn't require rendering.
- The view also has a layout with exactly one element (which also is its `focusProxy()`)
- That element is the https://code.qt.io/cgit/qt/qtwebengine.git/tree/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp[RenderWidgetHostViewQtDelegateWidget] (it inherits https://doc.qt.io/qt-5/qquickwidget.html[QQuickWidget]) - also often referred to as RWHV or RWHVQDW. It can be obtained via `sip.cast(tab._widget.focusProxy(), QQuickWidget)`.
- Calling `rootObject()` on that gives us the https://doc.qt.io/qt-5/qquickitem.html[QQuickItem] where Chromium renders into (?). With it, we can do things like `.setRotation(20)`.
- The `WebEngineTab` has a `_widget` attribute, which is the https://doc.qt.io/qt-6/qwebengineview.html[QWebEngineView]
- That view has a https://doc.qt.io/qt-6/qwebenginepage.html[QWebEnginePage] for everything which doesn't require rendering.
- The view also has a layout with exactly one element (which also is its `focusProxy()`).
- Qt 5: That element is the https://code.qt.io/cgit/qt/qtwebengine.git/tree/src/webenginewidgets/render_widget_host_view_qt_delegate_widget.cpp?h=5.15[RenderWidgetHostViewQtDelegateWidget] (it inherits https://doc.qt.io/qt-6/qquickwidget.html[QQuickWidget]) - also often referred to as RWHV or RWHVQDW.
It can be obtained via `sip.cast(tab._widget.focusProxy(), QQuickWidget)`.
- Qt 6: That element is the https://code.qt.io/cgit/qt/qtwebengine.git/tree/src/webenginewidgets/api/qwebengineview.cpp[WebEngineQuickWidget] (it inherits https://doc.qt.io/qt-6/qquickwidget.html[QQuickWidget]).
It can be obtained via `tab._widget.focusProxy()`.
- Calling `rootObject()` on that gives us the https://doc.qt.io/qt-6/qquickitem.html[QQuickItem] where Chromium renders into (?). With it, we can do things like `.setRotation(20)`.
Style conventions
-----------------
@ -657,7 +711,6 @@ Return:
+
* The layout of a module should be roughly like this:
* Type hinting: the qutebrowser codebase uses type hints liberally to enable
static type checking and autocompletion in editors.
- We use http://mypy-lang.org/[mypy] in CI jobs to perform static type
checking.
- Not all of the codebase is covered by type hints currently. We encourage
including type hints on all new code and even adding type hints to
existing code if you find yourself working on some that isn't already
covered. There are some module specific rules in the mypy config file,
`.mypy.ini`, to make type hints strictly required in some areas.
- More often than not mypy is correct when it raises issues. But don't be
afraid to add `# type: ignore[...]` statements or casts if you need to.
As an optional part of the language not all type information from third
parties is always correct. Mypy will raise a new issue if it spots an
"ignore" statement which is no longer needed because the underlying
issue has been resolved.
- One area where we have to take particular care is in code that deals
with differences between PyQt5 and PyQt6. We try to write most code in a
way that will work with either backend but when you need to deal with
differences you should use a pattern like:
+
[source,python]
----
if machinery.IS_QT5:
... # do PyQt5 specific implementation
else:
# PyQt6
... # do PyQt6 specific implementation
----
+
then you have to https://mypy.readthedocs.io/en/latest/command_line.html#cmdoption-mypy-always-true[tell]
mypy to treat `machinery.IS_QT5` as a constant value then run mypy twice to
cover both branches. There are a handful of variables in
`qutebrowser/qt/machinery.py` that mypy needs to know about. There are tox
jobs (`mypy-pyqt5` and `mypy-pyqt6`) that take care of telling mypy to use
them as constants.
Checklists
----------
@ -701,20 +789,35 @@ 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.
* Consider updating the completions for `content.headers.user_agent` in `configdata.yml`.
* Minor release: Consider updating some files from master:
* Mark the https://github.com/qutebrowser/qutebrowser/milestones[milestone] as closed.
* 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/`
* Update changelog in main branch and ensure the correct version number has `(unreleased)`
* If necessary: Update changelog in release branch from main.
**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=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
**Manual release:**
* Make sure Python is up-to-date on build machines.
* Mark the milestone at https://github.com/qutebrowser/qutebrowser/milestones as closed.
* Update changelog in master branch
* If necessary: Update changelog in release branch from master.
* Run `./.venv/bin/python3 scripts/dev/update_version.py {major,minor,patch}`.
* Run the printed instructions accordingly.
**Post release:**
* Update `qutebrowser-git` PKGBUILD if dependencies/install changed.
* Add unreleased future versions to changelog
* Update IRC topic
* Announce to qutebrowser and qutebrowser-announce mailinglist.
* +*-p*+, +*--private*+: Open in a new private window.
[[tab-close]]
=== tab-close
@ -1430,6 +1466,7 @@ If neither is given, move it to the first position.
==== positional arguments
* +'index'+: `+` or `-` to move relative to the current tab by count, or a default of 1 space.
A tab index to move to that index.
`start` and `end` to move to the start and the end.
==== count
@ -1669,6 +1706,7 @@ How many steps to zoom out.
|<<move-to-start-of-next-block,move-to-start-of-next-block>>|Move the cursor or selection to the start of next block.
|<<move-to-start-of-prev-block,move-to-start-of-prev-block>>|Move the cursor or selection to the start of previous block.
|<<prompt-accept,prompt-accept>>|Accept the current prompt.
|<<prompt-fileselect-external,prompt-fileselect-external>>|Choose a location using a configured external picker.
|<<prompt-item-focus,prompt-item-focus>>|Shift the focus of the prompt file completion menu to another item.
|<<prompt-open-download,prompt-open-download>>|Immediately open a download.
|<<prompt-yank,prompt-yank>>|Yank URL to clipboard or primary selection.
@ -1679,13 +1717,13 @@ How many steps to zoom out.
|<<rl-beginning-of-line,rl-beginning-of-line>>|Move to the start of the line.
|<<rl-delete-char,rl-delete-char>>|Delete the character after the cursor.
|<<rl-end-of-line,rl-end-of-line>>|Move to the end of the line.
|<<rl-filename-rubout,rl-filename-rubout>>|Delete backwards using the OS path separator as boundary.
|<<rl-forward-char,rl-forward-char>>|Move forward a character.
|<<rl-forward-word,rl-forward-word>>|Move forward to the end of the next word.
|<<rl-kill-line,rl-kill-line>>|Remove chars from the cursor to the end of the line.
|<<rl-kill-word,rl-kill-word>>|Remove chars from the cursor to the end of the current word.
|<<rl-unix-filename-rubout,rl-unix-filename-rubout>>|Remove chars from the cursor to the previous path separator.
|<<rl-rubout,rl-rubout>>|Delete backwards using the given characters as boundaries.
|<<rl-unix-line-discard,rl-unix-line-discard>>|Remove chars backward from the cursor to the beginning of the line.
|<<rl-unix-word-rubout,rl-unix-word-rubout>>|Remove chars from the cursor to the beginning of the word.
|<<rl-yank,rl-yank>>|Paste the most recently deleted text.
|<<selection-drop,selection-drop>>|Drop selection and keep selection mode enabled.
|<<selection-reverse,selection-reverse>>|Swap the stationary and moving end of the current selection.
@ -1857,6 +1895,12 @@ Accept the current prompt.
==== optional arguments
* +*-s*+, +*--save*+: Save the value to the config.
[[prompt-fileselect-external]]
=== prompt-fileselect-external
Choose a location using a configured external picker.
This spawns the external fileselector configured via `fileselect.folder.command`.
[[prompt-item-focus]]
=== prompt-item-focus
Syntax: +:prompt-item-focus 'which'+
@ -1937,6 +1981,12 @@ Move to the end of the line.
This acts like readline's end-of-line.
[[rl-filename-rubout]]
=== rl-filename-rubout
Delete backwards using the OS path separator as boundary.
For behavior that matches readline's `unix-filename-rubout` exactly, use `:rl-rubout "/ "` instead. This command uses the OS path separator (i.e. `\` on Windows) and ignores spaces.
[[rl-forward-char]]
=== rl-forward-char
Move forward a character.
@ -1961,11 +2011,17 @@ Remove chars from the cursor to the end of the current word.
This acts like readline's kill-word.
[[rl-unix-filename-rubout]]
=== rl-unix-filename-rubout
Remove chars from the cursor to the previous path separator.
[[rl-rubout]]
=== rl-rubout
Syntax: +:rl-rubout 'delim'+
Delete backwards using the given characters as boundaries.
With " ", this acts like readline's `unix-word-rubout`. With " /", this acts like readline's `unix-filename-rubout`, but consider using `:rl-filename-rubout` instead: It uses the OS path separator (i.e. `\` on Windows) and ignores spaces.
==== positional arguments
* +'delim'+: A string of characters (or a single character) until which text will be deleted.
This acts like readline's unix-filename-rubout.
[[rl-unix-line-discard]]
=== rl-unix-line-discard
@ -1973,12 +2029,6 @@ Remove chars backward from the cursor to the beginning of the line.
This acts like readline's unix-line-discard.
[[rl-unix-word-rubout]]
=== rl-unix-word-rubout
Remove chars from the cursor to the beginning of the word.
This acts like readline's unix-word-rubout. Whitespace is used as a word delimiter.
Alternatively, you can use `with config.pattern(...) as p:` to get a shortcut
@ -173,7 +187,7 @@ similar to `c.` which is scoped to the given domain:
[source,python]
----
with config.pattern('*://example.com/') as p:
with config.pattern('*://example.com/*') as p:
p.content.images = False
----
@ -377,8 +391,8 @@ import subprocess
def read_xresources(prefix):
props = {}
x = subprocess.run(['xrdb', '-query'], stdout=subprocess.PIPE)
lines = x.stdout.decode().split('\n')
x = subprocess.run(['xrdb', '-query'], capture_output=True, check=True, text=True)
lines = x.stdout.split('\n')
for line in filter(lambda l : l.startswith(prefix), lines):
prop, _, value = line.partition(':\t')
props[prop] = value
@ -392,11 +406,18 @@ Pre-built colorschemes
^^^^^^^^^^^^^^^^^^^^^^
- A collection of https://github.com/chriskempson/base16[base16] color-schemes can be found in https://github.com/theova/base16-qutebrowser[base16-qutebrowser] and used with https://github.com/AuditeMarlow/base16-manager[base16-manager].
qutebrowser is a keyboard-focused browser with a minimal GUI. It's based
on Python and Qt5 and is free software, licensed under the GPL.
on Python and Qt and is free software, licensed under the GPL.
It was inspired by other browsers/addons like dwb and Vimperator/Pentadactyl.
@ -34,7 +36,7 @@ show it.
*'URL'*::
URLs to open on startup (empty as a window separator).
=== optional arguments
=== options
*-h*, *--help*::
show this help message and exit
@ -62,8 +64,14 @@ show it.
*--backend* '{webkit,webengine}'::
Which backend to use.
*--qt-wrapper* '{PyQt6,PyQt5}'::
Which Qt wrapper to use. This can also be set via the QUTE_QT_WRAPPER environment variable. If both are set, the command line argument takes precedence.
*--desktop-file-name* 'DESKTOP_FILE_NAME'::
Set the base name of the desktop entry for this application. Used to set the app_id under Wayland. See https://doc.qt.io/qt-5/qguiapplication.html#desktopFileName-prop
Set the base name of the desktop entry for this application. Used to set the app_id under Wayland. See https://doc.qt.io/qt-6/qguiapplication.html#desktopFileName-prop
*--untrusted-args*::
Mark all following arguments as untrusted, which enforces that they are URLs/search terms (and not flags or commands)
@ -68,55 +68,13 @@ Then install the needed debuginfo packages:
Archlinux
^^^^^^^^^
For Archlinux, no debug information is provided. You can either compile Qt
yourself (which will take a few hours even on a modern machine) or use
debugging symbols compiled/packaged by me (x86_64 only).
To install my pre-built packages
++++++++++++++++++++++++++++++++
First download and sign the key:
For Archlinux, debug information is provided via their https://wiki.archlinux.org/title/Debuginfod[Debuginfod instance]. To use it, set the following in your environment:
style="font-size:13.8667px;line-height:1.25;font-family:sans-serif;stroke-width:1.06667">IRC: #qutebrowser on Freenode</flowPara><flowPara
style="font-size:13.8667px;line-height:1.25;font-family:sans-serif;stroke-width:1.06667">IRC: #qutebrowser on Libera Chat (irc.libera.chat)</flowPara><flowPara
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
Evan Chen
Vanessa Frank
tarneo
Julin S
arza
Eric Ericson
Philipp Albrecht
pylbrecht
Florian Stecker
Michael M
Michael M
brightonanc
Coiby Xu
Paul Mairo
Roberto Alsina
ugla
Roman Bogorodskiy
Eolien55
Takahashi, Naoki
Kian-Meng Ang
Jon Higgs
Tomasz Cebula
Tomasz Cebula
Jimmy
kt programs
Alessio Bogon
George Saker
Emanuele Torre
Mohammed Anas
Ingrid Budau
mkonig
Rijnder Wever
Mike Bannister
Markus Blöchl
shirenn
Lars Rustand
Benjamin Schnitzler
code-review-doctor
code-review-doctor
Tom Hu
Nicholas Schwab
Nicholas42
Axel Dahlberg
Nicholas42
David Vaughan
DavidRV00
Joakim Hansen
Daniel Dimijian
Marco A Parillo
Will Ruggiano
inwit
Pierre-Albéric TROUPLIN
mohite-abhi
Jonas Belouadi
tinywrkb
Eric Engestrom
Philipp Albrecht
Tim Gadanidis
harish3124
Ingrid
lyeoh
Mirko König
Eric Bower
Charlotte Van Petegem
Adrián Lattes
Reto Schnyder
Nicholas Boyd Isacsson
phesch
André Keller
jcromero
Leo S
Ander Punnar
Jason Rosenzweig
lufte
Naleo Hyde
Naleo
Yasser Kaddoura
zocker-160
jso8910
Sidharth Arya
Ryan Roden-Corrent
afreakk
frank shin
