Fix fetch/delete sql category bug.

Fixes #2868, where pressing <shift-tab> then <ctrl-d> in history
completion (with > 256 items) would cause later items to disappear (and
cause a crash if you try to delete again).

Cause:
Scrolling to the bottom would fetch an additional 256 items (in addition
to the 256 that are fetched at first). Deleting causes the query to
re-run, but it only fetches the initial 256 items, so the current index
is now invalid.

Fix:
After deleting from the history category, call fetchMore until it has
enough rows populated that the current index is valid.
This commit is contained in:
Ryan Roden-Corrent 2017-07-28 08:14:38 -04:00
parent 8f63bb1edc
commit c6cb6ccd07
2 changed files with 21 additions and 4 deletions

View File

@ -93,10 +93,12 @@ class HistoryCategory(QSqlQueryModel):
self._query.run(pat=pattern)
self.setQuery(self._query)
def removeRows(self, _row, _count, _parent=None):
def removeRows(self, row, _count, _parent=None):
"""Override QAbstractItemModel::removeRows to re-run sql query."""
# re-run query to reload updated table
with debug.log_time('sql', 'Re-running completion query post-delete'):
self._query.run()
self.setQuery(self._query)
while self.rowCount() < row:
self.fetchMore()
return True

View File

@ -22,7 +22,6 @@
import datetime
import pytest
from PyQt5.QtCore import QModelIndex
from qutebrowser.misc import sql
from qutebrowser.completion.models import histcategory
@ -147,6 +146,22 @@ def test_remove_rows(hist, model_validator):
model_validator.set_model(cat)
cat.set_pattern('')
hist.delete('url', 'foo')
# histcategory does not care which index was removed, it just regenerates
cat.removeRows(QModelIndex(), 1)
cat.removeRows(0, 1)
model_validator.validate([('bar', 'Bar', '')])
def test_remove_rows_fetch(hist):
"""removeRows should fetch enough data to make the current index valid."""
# we cannot use model_validator as it will fetch everything up front
hist.insert_batch({'url': [str(i) for i in range(300)]})
cat = histcategory.HistoryCategory()
cat.set_pattern('')
# sanity check that we didn't fetch everything up front
assert cat.rowCount() < 300
cat.fetchMore()
assert cat.rowCount() == 300
hist.delete('url', '298')
cat.removeRows(297, 1)
assert cat.rowCount() == 299