| Age | Commit message (Collapse) | Author |
|---|
|
Two issues here:
1. transplanted trees could get rendered upside down depending on the value of the
tab.new_position.tree.new_child setting, I think. The parent and child
attributes of the nodes looked correct but it was being rendered upside
down in the tab bar, weird. Anyway, we are adding new tabs at the top level
and them setting their parents based on the mapping. We shouldn't be
opening them as related to the currently focused tab
2. moving a subtree was crashing because the parent of the top node wasn't in
the mapping. Which is fine because that's not being moved over. I changed
it to not explicitly change the parent of such nodes (so they remain under
the tree root).
Two other tidy ups:
* the initialization of "uid_map" as it didn't seem to need the 1:1 in there.
* remove tabs explicitly in reverse order rather than repeatedly
removing the current tab. This avoids re-work in the tree because it
does a bunch of stuff to re-parent children if you delete the parent.
Also I have debug logging in notree currently warning of children being
"orphaned" that was going off
TODO items around tab moving:
* collapsed state is lost
* moved tab isn't focused in destination window (I think that's a thing that
happens normally? Could be wrong)
* this method might be a bit simpler and consistent with other methods
(like undo) if it did more stuff in one go using traverse order to
keep things consistent, instead of the map. It seems to work fine as
is but not re-using patterns makes it harder to see opportunities for
refactoring
The structure I was testing with was:
1. one
2. two
3. three
4. four
5. five
1. six
Then focusing "three" and `:tab-give -r 1`.
|
|
Little bit of tidy up. This was already being used in the conditional branch
for collapsed nodes, I have no idea why it wasn't being used for the
un-collapsed case.
|
|
When a collapsed tab group was being deleted the nodes were being removed
from the tree by a sneaky line in `_add_undo_entry()`, which made it difficult
to reason about the code. The nodes are removed from the tree by setting
`node.parent = None`, the setting goes and calls `parent.disown()`, which is a
bit magical but that's a job for another day.
This commit moves the line where the node is removed from the tree to be up into
the same block as where the tab is deleted and adjusts the undo entry to
remove the child uids from it instead. The problem there is that
`UndoEntry.from_node()` iterates over a nodes children, previously
children were being removed from the tree first so they weren't showing
up here. We don't want children uids in the undo nodes because the
restore code will look for an existing child to re-attach when
re-creating a node. But tabs are restored parent first. And since we do
have a parent uid in the undo entry that is enough to restore the full
structure.
|
|
This resolves an issue in update_tab_titles() when deleting a tab with
collapsed children. Since we were doing all of the super() action before
removing the node from the tree the super() method was calling
on_current_changed() before we had done our bit of updating the tree.
update_tab_titles() was getting called from that which was calling into
get_tab_fields() in TreeTabWidget which was complaining that the tree and
widget didn't match up.
We don't have any automated tests covering this behaviour currently but moving
the super() call to the end works so far. Possibly it was that way due to some
earlier refactoring.
I've changed a local variable name since it was shadowing a method arg that is
now used further down.
TODO:
* are there other pieces of code where we should look whether we are doing
tree stuff or widget stuff first and enshrine that as a pattern?
* the note about `_add_undo_entry()` points to unfortunate code that we should
look into. It doesn't seem correct to be manipulating the tree in that
method and the explanatory comment is too vague to explain anything
|
|
This is the third case where errors have been raised from update_tab_titles()
due to a mismatch between the tab widget and node tree:
1. on startup when the tabs are being added to the widget but the tree hasn't
grown yet - identified with heuristic
2. when hiding and showing tabs - changed logic to not fire updates
mid-operation
3. when shutting down tabs get removed from the widget before they are removed
from the tree - changed logic to not fire updates mid-operation
Now there is a fourth issue: loading a session with hidden tabs. In this case
tabs will be in the widget but not in the rendered tree, because that doesn't
include hidden nodes.
I'm using "all the URLs are empty" as a proxy for "a session is loading into
this window". That combined with there being a hidden tab in the tree should
hopefully be a good enough heuristic for this case. Although it would be good
to check with 100% certainty that a session was being loaded. Or just
having something to see if the current window was still being
initialized would do it, as sessions are always loaded into new windows.
I've refactored the logic in this method so that we are matching on exact tab
value, instead of having two lists and assuming the indexes are going to line
up. This way seems a bit more deliberate.
|
|
TreeTabWidget.get_tab_fields() was getting called with indexes that didn't
match what it had in its internal data structure on shutdown and was printing
errors. With this change tab titles no longer get updated during shutdown,
avoiding those errors.
Unfortunately we couldn't connect to the `shutting_down` signal on the parent
object because that is only emitted after the tabs are removed, and it seems
it needs to stay that way for the sake of the window close undo functionality.
So I plumbed that parent object through to a member variable, hopefully mypy
is happy with that. For some reason `self.parent()` in that later on function
was returning a MainWindow instead of a TabbedBrowser? Weird.
That does beg the question of why the nodes aren't getting removed from the
tree when they are getting removed from the widget. Should
`TreeTabbedBrowser._remove_tab()` be doing things in a different order? That
says "node will already be removed from tree", so where are they getting
removed? Or is that meaning widget instead of tree?
TODO: get more certainty on alternate logic fixes
|
|
When we are hiding or showing a tree group `render()` will reflect the
`collapsed` state right away. But while `update_tree_tab_visibility()` goes and
adds and removes the tabs from the tab widget one by one we get called on each
tab removal (via `tabRemoved()` -> `update_tab_title()` ->
`get_tab_fields()`). While each of those tab removals is happening there are
more tabs in the widget than are returned by `render()` which can cause issues
in this code due to the mismatch.
So add an attribute and context manager to avoid tab title updates while
`update_tree_tab_visibility()` is doing its things, since it goes ahead and
updates all the tab titles right afterwards anyway.
|
|
Previously there was a `tree_prefix is empty!` error being printed in a couple
of situations. 1) at startup 2) when hiding a tree group with multiple tabs in
it.
The main problem with the existing code is that it was catching a generic
IndexError without having a strong indication of what the actual cause of this
error was. We get called from `update_tab_titles()` in the base TabWidget with
a valid tab index. If we don't have information for that tab index in our tree
structure that could be an indication of a real error and we should go to
some effort to identify and prevent the cause.
This commit addresses cause 1 (at startup) by checking if the tree is empty
and returning early if so. Ideally we would have an even more specific
indication of being in startup (eg self.initialised == False) but we don't
have have a variable indicating a startup is in progress currently (we
do have one for shutting down!).
TODO: figure out when the tree is constructed vs when the tab widgets are
inserted and figure out if we can construct the tree earlier.
The second part of the commit identifies and documents the cause of 2
(collapsing a group) but leaves the error messages in for now because I want
to do a more proper fix that would let us continue to identify this
situation if we got here via another call path in the future.
This error was only being printed when hiding a group, not showing it,
because when showing a group there was more entries in the tree than in
the widget. So the function was still returning wrong data, it just
wasn't being verified at all.
The two remaining error messages are using a {foo=} f-string feature that I
don't think is available in all the python versions we support.
|
|
Update dependencies
|
|
|
|
Update dependencies
|
|
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).
|
|
qutebrowser/dependabot/github_actions/actions/upload-artifact-4
build(deps): bump actions/upload-artifact from 3 to 4
|
|
qutebrowser/dependabot/github_actions/github/codeql-action-3
build(deps): bump github/codeql-action from 2 to 3
|
|
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 3 to 4.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](https://github.com/actions/upload-artifact/compare/v3...v4)
---
updated-dependencies:
- dependency-name: actions/upload-artifact
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
|
|
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 2 to 3.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](https://github.com/github/codeql-action/compare/v2...v3)
---
updated-dependencies:
- dependency-name: github/codeql-action
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
|
|
Update dependencies
|
|
|
|
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?
|
|
qutebrowser/dependabot/github_actions/actions/setup-python-5
build(deps): bump actions/setup-python from 4 to 5
|
|
Update dependencies
|
|
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 4 to 5.
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](https://github.com/actions/setup-python/compare/v4...v5)
---
updated-dependencies:
- dependency-name: actions/setup-python
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
|
|
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 #2236
Fixes #7885
|
|
See #7854
|
|
See #7837
|
|
|
|
Fix docstrings
|
|
|
|
Closes #7814
|
|
|
|
|
|
|
|
|
|
See #7646
|
|
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
|
|
Closes #7929
|
|
|
|
|
|
|
