From 6e2d78b826863d9d810980fcd977475fbb0d8e41 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 1 Aug 2016 21:04:23 -0400 Subject: [PATCH 1/2] Don't crash on tab with no completions. first_item and last_item return an invalid index when there are no items in the completion, and the completionwidget will throw on an invalid index. However, setting an invalid index on the selection view is fine, so just remove the assertion. Resolves #1731. --- qutebrowser/completion/completionwidget.py | 5 ++--- tests/unit/completion/test_completionwidget.py | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 9fc1ef5d7..41ed65d74 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -193,9 +193,8 @@ class CompletionView(QTreeView): prev: True for prev item, False for next one. """ idx = self._next_idx(prev) - qtutils.ensure_valid(idx) - self.selectionModel().setCurrentIndex( - idx, QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) + self.selectionModel().setCurrentIndex(idx, + QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) def set_model(self, model): """Switch completion to a new model. diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 9e7bf04bf..fb1241b2c 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -119,6 +119,8 @@ def test_maybe_resize_completion(completionview, config_stub, qtbot): ([['Aa'], []], -1, 'Aa'), ([['Aa'], [], []], 1, 'Aa'), ([['Aa'], [], []], -1, 'Aa'), + ([[]], 1, None), + ([[]], -1, None), ]) def test_completion_item_next_prev(tree, count, expected, completionview): """Test that on_next_prev_item moves the selection properly. From a393adc6c7a40f25c1191bebb87067f74acda952 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 1 Aug 2016 22:12:13 -0400 Subject: [PATCH 2/2] Don't crash completion with auto-open/show False. If both are false, the selectionModel may be None. In this case, don't try to move the index. Resolves #1722. --- qutebrowser/completion/completionwidget.py | 10 +++++++--- tests/unit/completion/test_completionwidget.py | 10 ++++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 41ed65d74..364d783f6 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -192,9 +192,13 @@ class CompletionView(QTreeView): Args: prev: True for prev item, False for next one. """ - idx = self._next_idx(prev) - self.selectionModel().setCurrentIndex(idx, - QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) + # selmodel can be None if 'show' and 'auto-open' are set to False + # https://github.com/The-Compiler/qutebrowser/issues/1731 + selmodel = self.selectionModel() + if (selmodel is not None): + idx = self._next_idx(prev) + selmodel.setCurrentIndex(idx, + QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) def set_model(self, model): """Switch completion to a new model. diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index fb1241b2c..badc1fc89 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -148,3 +148,13 @@ def test_completion_item_next_prev(tree, count, expected, completionview): completionview.completion_item_next() idx = completionview.selectionModel().currentIndex() assert filtermodel.data(idx) == expected + + +def test_completion_item_next_prev_no_model(completionview): + """Test that next/prev won't crash with no model set. + + This can happen if completion.show and completion.auto-open are False. + Regression test for issue #1722. + """ + completionview.completion_item_prev() + completionview.completion_item_next()