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.
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?
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
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
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.
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.
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
Florian Bruhin
toofar
qutebrowser bot
Philipp Albrecht