Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add qute:settings handler #2

Merged
merged 3 commits into from
Nov 25, 2014
Merged

Add qute:settings handler #2

merged 3 commits into from
Nov 25, 2014

Conversation

iggy
Copy link

@iggy iggy commented Sep 30, 2014

Add a page to make finding/setting configuration settings a little easier on
users. Tested changing different types of settings to make sure they all change
and retain the changes over a close/open cycle.


This change is Reviewable

@@ -25,6 +25,7 @@
</script>
</head>
<body>
<header><h1>{{ title }}</h1></header>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's a good idea to display the title as <h1> on every qute:... page? IMHO it should be enough to display titles in the tab title.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary. It helped put some space in the page for my eyes, and it's kind of a habit from Django. I'll pull it out.

@The-Compiler
Copy link
Member

I've quickly tried it now, some UI suggestions

  • It'd be nice to show config section and value descriptions. Check _get_setting_quickref and generate_settings in scripts/src2asciidoc.py for some inspiration.
  • If the config type has valid_values defined to something else than None, a select-box of valid values could be presented.
  • Color pickers maybe should have some plaintext/color toggle so people could also use gradients? (though that one probably isn't that important)

Add the ability to view/edit the browser's config settings via a special
browser page.

It's very simplistic for now, but a good starting point.

Future possibilities:
* Matching config types to html input types
  * colors = html colorpicker
  * options with valid_values = select/multi-select
  * plain text fallbacks where appropriate
  * multi-line text edits for long options (i.e. host-block-lists)

* Javascript option verification
  * switch from submitting changes onblur to onchange if an option passes basic
    verification, etc.
Brian Jackson added 2 commits November 25, 2014 11:19
The message class needs a win_id to know where to send error messages. Just
pass it through the jinja->js->qtwebbridge as it's just a simple int.
Fix to make sure the js bridge code is only enabled when qute: pages are shown.
Previously it would only be available to the first page (and before that it
was available to all pages).
@The-Compiler The-Compiler merged commit 9cfb4b3 into qutebrowser:master Nov 25, 2014
The-Compiler added a commit that referenced this pull request Dec 5, 2014
The-Compiler added a commit that referenced this pull request Feb 26, 2015
We already attempted this in c5a2039, but
having the directories as module attributes means they'll be created on start
(rather than when they're actually used), and it'd also be impossible to change
them after init for some reason in the future.

To still have a nice short API, we simply change the attributes to functions.
The-Compiler added a commit that referenced this pull request Apr 13, 2015
Fixes #614.

We only added the long variant of the flag, but not the short one.
The-Compiler added a commit that referenced this pull request Jun 10, 2015
The-Compiler added a commit that referenced this pull request Jun 23, 2015
Kingdread added a commit to Kingdread/qutebrowser that referenced this pull request Oct 2, 2015
Lint fixes round qutebrowser#2 as the "unused arguments" are now fine.
The-Compiler added a commit that referenced this pull request Oct 14, 2015
hcpl added a commit to hcpl/qutebrowser that referenced this pull request Oct 8, 2016
The-Compiler pushed a commit that referenced this pull request Oct 2, 2017
JimenezJC pushed a commit to JimenezJC/qutebrowser that referenced this pull request Apr 10, 2018
@pinpox pinpox mentioned this pull request Jan 21, 2019
The-Compiler added a commit that referenced this pull request Mar 29, 2021
This reverts commit c6cf306.

Seems to cause segfaults:

  #0  0x00007ffff5cecbcc in void doActivate<false>(QObject*, int, void**) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #1  0x00007ffff5be4e31 in QIODevice::channelReadyRead(int) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #2  0x00007fffeffccb54 in QAbstractSocketPrivate::canReadNotification() () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Network.so.5
  #3  0x00007fffeffdf061 in QReadNotifier::event(QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Network.so.5
  #4  0x00007ffff269e43c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Widgets.so.5
  #5  0x00007ffff26a4f20 in QApplication::notify(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Widgets.so.5
  #6  0x00007ffff318d0d6 in sipQApplication::notify(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/QtWidgets.abi3.so
  #7  0x00007ffff5cb4808 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #8  0x00007ffff5d10d98 in socketNotifierSourceDispatch(_GSource*, int (*)(void*), void*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #9  0x00007ffff691df9c in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
  #10 0x00007ffff6971a49 in ?? () from /usr/lib/libglib-2.0.so.0
  #11 0x00007ffff691b6f1 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
  #12 0x00007ffff5d101cc in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #13 0x00007ffff5cb321a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #14 0x00007ffff5cbc1d3 in QCoreApplication::exec() () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
The-Compiler added a commit that referenced this pull request Mar 29, 2021
This reverts commit c6cf306.

Seems to cause segfaults:

  #0  0x00007ffff5cecbcc in void doActivate<false>(QObject*, int, void**) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #1  0x00007ffff5be4e31 in QIODevice::channelReadyRead(int) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #2  0x00007fffeffccb54 in QAbstractSocketPrivate::canReadNotification() () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Network.so.5
  #3  0x00007fffeffdf061 in QReadNotifier::event(QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Network.so.5
  #4  0x00007ffff269e43c in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Widgets.so.5
  #5  0x00007ffff26a4f20 in QApplication::notify(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Widgets.so.5
  #6  0x00007ffff318d0d6 in sipQApplication::notify(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/QtWidgets.abi3.so
  #7  0x00007ffff5cb4808 in QCoreApplication::notifyInternal2(QObject*, QEvent*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #8  0x00007ffff5d10d98 in socketNotifierSourceDispatch(_GSource*, int (*)(void*), void*) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #9  0x00007ffff691df9c in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
  #10 0x00007ffff6971a49 in ?? () from /usr/lib/libglib-2.0.so.0
  #11 0x00007ffff691b6f1 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
  #12 0x00007ffff5d101cc in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #13 0x00007ffff5cb321a in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
  #14 0x00007ffff5cbc1d3 in QCoreApplication::exec() () from /home/florian/proj/qutebrowser/git/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/lib/libQt5Core.so.5
The-Compiler pushed a commit that referenced this pull request Apr 6, 2021
Add debug lines for lib_file
@franzhuang franzhuang mentioned this pull request Oct 6, 2021
@rien333 rien333 mentioned this pull request Jun 7, 2022
The-Compiler added a commit that referenced this pull request Dec 6, 2022
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:

pytest-dev/pytest#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...
toofar added a commit that referenced this pull request Oct 27, 2023
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.
The-Compiler added a commit that referenced this pull request Dec 7, 2023
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: #2236 (comment)

    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

jome 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
The-Compiler added a commit that referenced this pull request Dec 7, 2023
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: #2236 (comment)

> 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
@jnglg jnglg mentioned this pull request Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants