From 117cb15b8db5202d7734b7b821f7d6f8dc1d9b72 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 19 May 2024 14:29:31 +1200 Subject: [PATCH 1/2] Handle selection model being None when clearing selection 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. --- qutebrowser/completion/completionwidget.py | 2 +- .../unit/completion/test_completionwidget.py | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 0f5dc0de9..27e631662 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -414,7 +414,7 @@ class CompletionView(QTreeView): def on_clear_completion_selection(self): """Clear the selection model when an item is activated.""" self.hide() - selmod = self._selection_model() + selmod = self.selectionModel() if selmod is not None: selmod.clearSelection() selmod.clearCurrentIndex() diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 4ab9dfca0..387339f4f 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -7,7 +7,7 @@ from unittest import mock import pytest -from qutebrowser.qt.core import QRect +from qutebrowser.qt.core import QRect, QItemSelectionModel from qutebrowser.completion import completionwidget from qutebrowser.completion.models import completionmodel, listcategory @@ -285,6 +285,23 @@ def test_completion_show(show, rows, quick_complete, completionview, model, assert not completionview.isVisible() +def test_completion_selection_clear_no_model(completionview): + completionview.show() + completionview.on_clear_completion_selection() + assert completionview.isVisible() is False + + +def test_completion_selection_clear_with_model(completionview, mocker): + selmod = mock.Mock(spec=QItemSelectionModel) + mocker.patch.object(completionview, "selectionModel", return_value=selmod) + completionview.show() + completionview.on_clear_completion_selection() + + assert completionview.isVisible() is False + selmod.clearSelection.assert_called_once() + selmod.clearCurrentIndex.assert_called_once() + + def test_completion_item_del(completionview, model): """Test that completion_item_del invokes delete_cur_item in the model.""" func = mock.Mock(spec=[]) From 04b0d84bda69888220f3e099b9f76b6baeb5cfb3 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 19 May 2024 14:34:23 +1200 Subject: [PATCH 2/2] update changelog for None selection model fix --- doc/changelog.asciidoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index 8dcb4cee3..e253e26a5 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -53,6 +53,8 @@ Fixed Python upgrade) should now not crash anymore. - When the QtWebEngine resources dir couldn't be found, qutebrowser now doesn't crash anymore (but QtWebEngine still might). +- Fixed a rare crash in the completion widget when there was no selection model + when we went to clear that, probably when leaving a mode. (#7901) [[v3.1.1]] v3.1.1 (unreleased)