From 8278879c199ecc9b17a1fd603ecb4a31f5fd6c68 Mon Sep 17 00:00:00 2001 From: Andreas Buhr Date: Fri, 30 Apr 2021 09:07:06 +0200 Subject: Fix QItemSelectionModel::selectionChanged emission QItemSelectionModel has the property selectedIndexes with the notification signal selectionChanged. When a row is deleted or inserted above the current selection, the row number of the current selection changes and thus the return value of selectedIndexes changes. This should trigger its notification signal. This signal was not emitted. This patch fixes this and adds a unit test to verify this. [ChangeLog][Important Behavior Changes][QtCore] QItemSelectionModel now emits the selectionChanged signal if only the indexes of the selected items change. Fixes: QTBUG-93305 Change-Id: Ia5fb5ca32d658c9c0e1d7093c57cc08a966b9402 Reviewed-by: Ivan Solovev Reviewed-by: David Faure --- src/corelib/itemmodels/qitemselectionmodel.cpp | 40 ++++++++++++++++++++------ 1 file changed, 32 insertions(+), 8 deletions(-) (limited to 'src/corelib') diff --git a/src/corelib/itemmodels/qitemselectionmodel.cpp b/src/corelib/itemmodels/qitemselectionmodel.cpp index bccf02bb0b..1b8b8f57ff 100644 --- a/src/corelib/itemmodels/qitemselectionmodel.cpp +++ b/src/corelib/itemmodels/qitemselectionmodel.cpp @@ -675,6 +675,7 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare int start, int end) { Q_Q(QItemSelectionModel); + Q_ASSERT(start <= end); finalize(); // update current index @@ -699,6 +700,7 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare QItemSelection deselected; QItemSelection newParts; + bool indexesOfSelectionChanged = false; QItemSelection::iterator it = ranges.begin(); while (it != ranges.end()) { if (it->topLeft().parent() != parent) { // Check parents until reaching root or contained in range @@ -710,6 +712,8 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare deselected.append(*it); it = ranges.erase(it); } else { + if (itParent.isValid() && end < itParent.row()) + indexesOfSelectionChanged = true; ++it; } } else if (start <= it->bottom() && it->bottom() <= end // Full inclusion @@ -734,12 +738,16 @@ void QItemSelectionModelPrivate::_q_rowsAboutToBeRemoved(const QModelIndex &pare deselected.append(removedRange); QItemSelection::split(*it, removedRange, &newParts); it = ranges.erase(it); - } else + } else if (end < it->top()) { // deleted row before selection + indexesOfSelectionChanged = true; ++it; + } else { + ++it; + } } ranges.append(newParts); - if (!deselected.isEmpty()) + if (!deselected.isEmpty() || indexesOfSelectionChanged) emit q->selectionChanged(QItemSelection(), deselected); } @@ -791,11 +799,12 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex & QList split; QList::iterator it = ranges.begin(); for (; it != ranges.end(); ) { - if ((*it).isValid() && (*it).parent() == parent + const QModelIndex &itParent = it->parent(); + if ((*it).isValid() && itParent == parent && (*it).left() < start && (*it).right() >= start) { - QModelIndex bottomMiddle = model->index((*it).bottom(), start - 1, (*it).parent()); + QModelIndex bottomMiddle = model->index((*it).bottom(), start - 1, itParent); QItemSelectionRange left((*it).topLeft(), bottomMiddle); - QModelIndex topMiddle = model->index((*it).top(), start, (*it).parent()); + QModelIndex topMiddle = model->index((*it).top(), start, itParent); QItemSelectionRange right(topMiddle, (*it).bottomRight()); it = ranges.erase(it); split.append(left); @@ -815,25 +824,35 @@ void QItemSelectionModelPrivate::_q_columnsAboutToBeInserted(const QModelIndex & void QItemSelectionModelPrivate::_q_rowsAboutToBeInserted(const QModelIndex &parent, int start, int end) { + Q_Q(QItemSelectionModel); Q_UNUSED(end); finalize(); QList split; QList::iterator it = ranges.begin(); + bool indexesOfSelectionChanged = false; for (; it != ranges.end(); ) { - if ((*it).isValid() && (*it).parent() == parent + const QModelIndex &itParent = it->parent(); + if ((*it).isValid() && itParent == parent && (*it).top() < start && (*it).bottom() >= start) { - QModelIndex middleRight = model->index(start - 1, (*it).right(), (*it).parent()); + QModelIndex middleRight = model->index(start - 1, (*it).right(), itParent); QItemSelectionRange top((*it).topLeft(), middleRight); - QModelIndex middleLeft = model->index(start, (*it).left(), (*it).parent()); + QModelIndex middleLeft = model->index(start, (*it).left(), itParent); QItemSelectionRange bottom(middleLeft, (*it).bottomRight()); it = ranges.erase(it); split.append(top); split.append(bottom); + } else if ((*it).isValid() && itParent == parent // insertion before selection + && (*it).top() >= start) { + indexesOfSelectionChanged = true; + ++it; } else { ++it; } } ranges += split; + + if (indexesOfSelectionChanged) + emit q->selectionChanged(QItemSelection(), QItemSelection()); } /*! @@ -1227,6 +1246,11 @@ void QItemSelectionModel::select(const QModelIndex &index, QItemSelectionModel:: Note the that the current index changes independently from the selection. Also note that this signal will not be emitted when the item model is reset. + Items which stay selected but change their index are not included in + \a selected and \a deselected. Thus, this signal might be emitted with both + \a selected and \a deselected empty, if only the indices of selected items + change. + \sa select(), currentChanged() */ -- cgit v1.2.3