1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix problems with extended selection after changing the model contents

Browse Source 
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) <ogoffart@woboq.com>
This commit is contained in:
Frank Reininghaus 2015-03-10 23:05:22 +01:00 committed by Giuseppe D'Angelo
parent e6b97fd2c7
commit f1e9076809
4 changed files with 135 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/widgets/itemviews/qabstractitemview.cpp
View File

@ -1055,7 +1055,7 @@ void QAbstractItemView::setCurrentIndex(const QModelIndex &index)
d->selectionModel->setCurrentIndex(index, command);
d->currentIndexSet = true;
if ((command & QItemSelectionModel::Current) == 0)
d->pressedPosition = visualRect(currentIndex()).center() + d->offset();
d->currentSelectionStartIndex = index;
}
}
@ -1707,10 +1707,12 @@ void QAbstractItemView::mousePressEvent(QMouseEvent *event)
QItemSelectionModel::SelectionFlags command = selectionCommand(index, event);
d->noSelectionOnMousePress = command == QItemSelectionModel::NoUpdate || !index.isValid();
QPoint offset = d->offset();
if ((command & QItemSelectionModel::Current) == 0)
if ((command & QItemSelectionModel::Current) == 0) {
d->pressedPosition = pos + offset;
else if (!indexAt(d->pressedPosition - offset).isValid())
d->pressedPosition = visualRect(currentIndex()).center() + offset;
d->currentSelectionStartIndex = index;
}
else if (!d->currentSelectionStartIndex.isValid())
d->currentSelectionStartIndex = currentIndex();
if (edit(index, NoEditTriggers, event))
return;
@ -1722,7 +1724,7 @@ void QAbstractItemView::mousePressEvent(QMouseEvent *event)
d->autoScroll = false;
d->selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate);
d->autoScroll = autoScroll;
QRect rect(d->pressedPosition - offset, pos);
QRect rect(visualRect(d->currentSelectionStartIndex).center(), pos);
if (command.testFlag(QItemSelectionModel::Toggle)) {
command &= ~QItemSelectionModel::Toggle;
d->ctrlDragSelectionFlag = d->selectionModel->isSelected(index) ? QItemSelectionModel::Deselect : QItemSelectionModel::Select;
@ -2319,16 +2321,16 @@ void QAbstractItemView::keyPressEvent(QKeyEvent *event)
// note that we don't check if the new current index is enabled because moveCursor() makes sure it is
if (command & QItemSelectionModel::Current) {
d->selectionModel->setCurrentIndex(newCurrent, QItemSelectionModel::NoUpdate);
if (!indexAt(d->pressedPosition - d->offset()).isValid())
d->pressedPosition = visualRect(oldCurrent).center() + d->offset();
QRect rect(d->pressedPosition - d->offset(), visualRect(newCurrent).center());
if (!d->currentSelectionStartIndex.isValid())
d->currentSelectionStartIndex = oldCurrent;
QRect rect(visualRect(d->currentSelectionStartIndex).center(), visualRect(newCurrent).center());
setSelection(rect, command);
} else {
d->selectionModel->setCurrentIndex(newCurrent, command);
d->pressedPosition = visualRect(newCurrent).center() + d->offset();
d->currentSelectionStartIndex = newCurrent;
if (newCurrent.isValid()) {
// We copy the same behaviour as for mousePressEvent().
QRect rect(d->pressedPosition - d->offset(), QSize(1, 1));
QRect rect(visualRect(newCurrent).center(), QSize(1, 1));
setSelection(rect, command);
}
}

1
src/widgets/itemviews/qabstractitemview_p.h
View File

@ -370,6 +370,7 @@ public:
QPersistentModelIndex enteredIndex;
QPersistentModelIndex pressedIndex;
QPersistentModelIndex currentSelectionStartIndex;
Qt::KeyboardModifiers pressedModifiers;
QPoint pressedPosition;
bool pressedAlreadySelected;

119
tests/auto/widgets/itemviews/qabstractitemview/tst_qabstractitemview.cpp
View File

@ -53,6 +53,8 @@
#include <qscreen.h>
#include <qscopedpointer.h>
#include <qstyleditemdelegate.h>
#include <qstringlistmodel.h>
#include <qsortfilterproxymodel.h>
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"

3
tests/auto/widgets/itemviews/qtableview/tst_qtableview.cpp
View File

@ -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));