From 5c94c5a4efd41fc9b9a60136a239214b9f2d5c6c Mon Sep 17 00:00:00 2001 From: Kaloyan Chehlarski Date: Wed, 14 Aug 2024 11:36:49 +0200 Subject: [PATCH] QAbstractItemView: Fix shift-select after changing index through model When setting the current index through the related model's QItemSelectionModel::setCurrentIndex(), the selection can also change. However, attempting to shift-select after doing so would produce an unexpected selection, because the internal variable that keeps track of the start index of the current selection would still have its old value. This change moves where said variable is set: instead of doing that in QAbstractItemView::setCurrentIndex(), it is now done inside the currentChanged() slot instead. This slot will get called both when the selection is modified through the QAbstractItemView class itself, as well as when it's modified from the outside (e.g. from the model). Pick-to: 6.7 6.5 6.2 Fixes: QTBUG-127381 Change-Id: I6d38320e656aa5a102ce079604590672c88ecad1 Reviewed-by: Volker Hilsheimer (cherry picked from commit fb2d64bc57aadf5bf140c72cf7eb2a5f391b7d55) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/itemviews/qabstractitemview.cpp | 6 ++-- src/widgets/itemviews/qtableview.cpp | 18 ++++++++-- .../tst_qabstractitemview.cpp | 33 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/widgets/itemviews/qabstractitemview.cpp b/src/widgets/itemviews/qabstractitemview.cpp index 85e478a71e2..695f1c8c7f5 100644 --- a/src/widgets/itemviews/qabstractitemview.cpp +++ b/src/widgets/itemviews/qabstractitemview.cpp @@ -1102,8 +1102,6 @@ void QAbstractItemView::setCurrentIndex(const QModelIndex &index) QItemSelectionModel::SelectionFlags command = selectionCommand(index, nullptr); d->selectionModel->setCurrentIndex(index, command); d->currentIndexSet = true; - if ((command & QItemSelectionModel::Current) == 0) - d->currentSelectionStartIndex = index; } } @@ -3819,6 +3817,10 @@ void QAbstractItemView::currentChanged(const QModelIndex ¤t, const QModelI } } + QItemSelectionModel::SelectionFlags command = selectionCommand(current, nullptr); + if ((command & QItemSelectionModel::Current) == 0) + d->currentSelectionStartIndex = current; + if (current.isValid() && !d->autoScrollTimer.isActive()) { if (isVisible()) { if (d->autoScroll) diff --git a/src/widgets/itemviews/qtableview.cpp b/src/widgets/itemviews/qtableview.cpp index 6b69f380f05..6ac85650ba6 100644 --- a/src/widgets/itemviews/qtableview.cpp +++ b/src/widgets/itemviews/qtableview.cpp @@ -3426,7 +3426,14 @@ void QTableViewPrivate::selectRow(int row, bool anchor) int column = horizontalHeader->logicalIndexAt(q->isRightToLeft() ? viewport->width() : 0); QModelIndex index = model->index(row, column, root); QItemSelectionModel::SelectionFlags command = q->selectionCommand(index); - selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate); + + { + // currentSelectionStartIndex gets modified inside QAbstractItemView::currentChanged() + const auto startIndex = currentSelectionStartIndex; + selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate); + currentSelectionStartIndex = startIndex; + } + if ((anchor && !(command & QItemSelectionModel::Current)) || (q->selectionMode() == QTableView::SingleSelection)) currentSelectionStartIndex = model->index(row, column, root); @@ -3466,7 +3473,14 @@ void QTableViewPrivate::selectColumn(int column, bool anchor) int row = verticalHeader->logicalIndexAt(0); QModelIndex index = model->index(row, column, root); QItemSelectionModel::SelectionFlags command = q->selectionCommand(index); - selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate); + + { + // currentSelectionStartIndex gets modified inside QAbstractItemView::currentChanged() + const auto startIndex = currentSelectionStartIndex; + selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate); + currentSelectionStartIndex = startIndex; + } + if ((anchor && !(command & QItemSelectionModel::Current)) || (q->selectionMode() == QTableView::SingleSelection)) currentSelectionStartIndex = model->index(row, column, root); diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 4f2495375d9..2cbb1279581 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -152,6 +152,7 @@ private slots: void testSpinBoxAsEditor(); void removeIndexWhileEditing(); void focusNextOnHide(); + void shiftSelectionAfterModelSetCurrentIndex(); private: static QAbstractItemView *viewFromString(const QByteArray &viewType, QWidget *parent = nullptr) @@ -3561,5 +3562,37 @@ void tst_QAbstractItemView::focusNextOnHide() QVERIFY(lineEdit.hasFocus()); } +// Tests for QTBUG-127381, where shift-selecting after calling +// QItemSelectionModel::setCurrentIndex() would produce unexpected results. +void tst_QAbstractItemView::shiftSelectionAfterModelSetCurrentIndex() +{ + QStandardItemModel model(5, 1); + QTreeView view; + view.setSelectionBehavior(QAbstractItemView::SelectRows); + view.setSelectionMode(QAbstractItemView::ExtendedSelection); + view.setModel(&model); + + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + + QModelIndex index = model.index(0, 0); + view.setCurrentIndex(index); + index = model.index(2, 0); + view.selectionModel()->setCurrentIndex(index, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows); + + QTest::keyPress(&view, Qt::Key_Down, Qt::ShiftModifier); + + QItemSelection selection = view.selectionModel()->selection(); + QCOMPARE(selection.size(), 1); + QCOMPARE(selection.first().top(), 2); + QCOMPARE(selection.first().bottom(), 3); + + QTest::keyPress(&view, Qt::Key_Up, Qt::ShiftModifier); + + selection = view.selectionModel()->selection(); + QCOMPARE(selection.first().top(), 2); + QCOMPARE(selection.first().bottom(), 2); +} + QTEST_MAIN(tst_QAbstractItemView) #include "tst_qabstractitemview.moc"