From f1e90768095be37419ba4bf3c69ec5c860bdbcb6 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Tue, 10 Mar 2015 23:05:22 +0100 Subject: Fix problems with extended selection after changing the model contents Storing the position of the first selected item in the view can lead to wrong extended selections if the contents of the model change. Future Shift-clicks will always use the previous position of the first selected item, which may not be correct any more, to calculate the current selection. To fix this problem, a QPersistentModelIndex is used to keep track of the first selected item. A new unit test is added. Moreover, one function of the QTableView unit test is changed such that it shows the view prior to performing the test. Without this change, this test may fail. That the test, which simulates mouse presses without showing the view, worked at all seems to be a coincidence, as pointed out in QTBUG-18009. Task-number: QTBUG-18009 Change-Id: I0d844fbd1a994c279a7c8ee5d9b5b9fccecd25bf Reviewed-by: Olivier Goffart (Woboq GmbH) --- .../qabstractitemview/tst_qabstractitemview.cpp | 119 +++++++++++++++++++++ .../itemviews/qtableview/tst_qtableview.cpp | 3 + 2 files changed, 122 insertions(+) (limited to 'tests') diff --git a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp index 79f51d27c7..90580aea05 100644 --- a/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp +++ b/tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp @@ -53,6 +53,8 @@ #include #include #include +#include +#include static inline void setFrameless(QWidget *w) { @@ -252,6 +254,7 @@ private slots: void QTBUG31411_noSelection(); void QTBUG39324_settingSameInstanceOfIndexWidget(); void sizeHintChangeTriggersLayout(); + void shiftSelectionAfterChangingModelContents(); }; class MyAbstractItemDelegate : public QAbstractItemDelegate @@ -1884,5 +1887,121 @@ void tst_QAbstractItemView::QTBUG39324_settingSameInstanceOfIndexWidget() table->show(); } +void tst_QAbstractItemView::shiftSelectionAfterChangingModelContents() +{ + QStringList list; + list << "A" << "B" << "C" << "D" << "E" << "F"; + QStringListModel model(list); + QSortFilterProxyModel proxyModel; + proxyModel.setSourceModel(&model); + proxyModel.sort(0, Qt::AscendingOrder); + proxyModel.setDynamicSortFilter(true); + + QPersistentModelIndex indexA(proxyModel.index(0, 0)); + QPersistentModelIndex indexB(proxyModel.index(1, 0)); + QPersistentModelIndex indexC(proxyModel.index(2, 0)); + QPersistentModelIndex indexD(proxyModel.index(3, 0)); + QPersistentModelIndex indexE(proxyModel.index(4, 0)); + QPersistentModelIndex indexF(proxyModel.index(5, 0)); + + QListView view; + view.setModel(&proxyModel); + view.setSelectionMode(QAbstractItemView::ExtendedSelection); + view.show(); + QTest::qWaitForWindowExposed(&view); + + // Click "C" + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, view.visualRect(indexC).center()); + QModelIndexList selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 1); + QVERIFY(selected.contains(indexC)); + + // Insert new item "B1" + model.insertRow(0); + model.setData(model.index(0, 0), "B1"); + + // Shift-click "D" -> we expect that "C" and "D" are selected + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ShiftModifier, view.visualRect(indexD).center()); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 2); + QVERIFY(selected.contains(indexC)); + QVERIFY(selected.contains(indexD)); + + // Click "D" + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, view.visualRect(indexD).center()); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 1); + QVERIFY(selected.contains(indexD)); + + // Remove items "B" and "C" + model.removeRows(proxyModel.mapToSource(indexB).row(), 1); + model.removeRows(proxyModel.mapToSource(indexC).row(), 1); + QVERIFY(!indexB.isValid()); + QVERIFY(!indexC.isValid()); + + // Shift-click "F" -> we expect that "D", "E", and "F" are selected + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ShiftModifier, view.visualRect(indexF).center()); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 3); + QVERIFY(selected.contains(indexD)); + QVERIFY(selected.contains(indexE)); + QVERIFY(selected.contains(indexF)); + + // Move to "A" by pressing "Up" repeatedly + while (view.currentIndex() != indexA) { + QTest::keyClick(&view, Qt::Key_Up); + } + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 1); + QVERIFY(selected.contains(indexA)); + + // Change the sort order + proxyModel.sort(0, Qt::DescendingOrder); + + // Shift-click "F" -> All items should be selected + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::ShiftModifier, view.visualRect(indexF).center()); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), model.rowCount()); + + // Restore the old sort order + proxyModel.sort(0, Qt::AscendingOrder); + + // Click "D" + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, view.visualRect(indexD).center()); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 1); + QVERIFY(selected.contains(indexD)); + + // Insert new item "B2" + model.insertRow(0); + model.setData(model.index(0, 0), "B2"); + + // Press Shift+Down -> "D" and "E" should be selected. + QTest::keyClick(&view, Qt::Key_Down, Qt::ShiftModifier); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 2); + QVERIFY(selected.contains(indexD)); + QVERIFY(selected.contains(indexE)); + + // Click "A" to reset the current selection starting point; + //then select "D" via QAbstractItemView::setCurrentIndex(const QModelIndex&). + QTest::mouseClick(view.viewport(), Qt::LeftButton, Qt::NoModifier, view.visualRect(indexA).center()); + view.setCurrentIndex(indexD); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 1); + QVERIFY(selected.contains(indexD)); + + // Insert new item "B3" + model.insertRow(0); + model.setData(model.index(0, 0), "B3"); + + // Press Shift+Down -> "D" and "E" should be selected. + QTest::keyClick(&view, Qt::Key_Down, Qt::ShiftModifier); + selected = view.selectionModel()->selectedIndexes(); + QCOMPARE(selected.count(), 2); + QVERIFY(selected.contains(indexD)); + QVERIFY(selected.contains(indexE)); +} + QTEST_MAIN(tst_QAbstractItemView) #include "tst_qabstractitemview.moc" diff --git a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp index a11d423896..75f77f8107 100644 --- a/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp +++ b/tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp @@ -4288,6 +4288,9 @@ void tst_QTableView::taskQTBUG_4516_clickOnRichTextLabel() QLabel label("rich text"); label.setTextFormat(Qt::RichText); view.setIndexWidget(model.index(1,1), &label); + view.show(); + QVERIFY(QTest::qWaitForWindowExposed(&view)); + view.setCurrentIndex(model.index(0,0)); QCOMPARE(view.currentIndex(), model.index(0,0)); -- cgit v1.2.3