From 64df402f8471189ac96240493f6b4a8ba2fbbb80 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Fri, 25 Mar 2016 20:10:12 +0100 Subject: [PATCH] KSPM: Ensure proper signalling when removing last selected KSelectionProxyModel listens to two delegate objects for changes: the source model of the proxy, and the selectionModel. When a QModelIndex which is selected gets removed, slots connected to signals for both clients are executed. Whichever slot is executed first updates the KSelectionProxyModel, and when the other slot is executed it ignores the already-handled event. Until Qt 5.5. QItemSelectionModel required the model to be passed in its constructor. Prior to commit v5.9.0~21 (KSPM: Make the setSelectionModel method public., 2015-03-08) the KSelectionProxyModel required the selectionModel to be passed in its constructor. Because the order of slot execution follows the order of signal slot connection, the KSelectionProxyModel slot for handling deselection was always executed first to process the removal. Since Qt 5.5 and KItemModels 5.9.0, it is possible to set the model on the QItemSelectionModel and the selectionModel on the KSelectionProxyModel in a different order, meaning that the (before/after) slots for handling removal of a row from the source model is executed first. When the before slot is executed the selectionModel still has a selection, but by the time the after slot is executed the selection has already been removed (because QItemSelectionModel updates its selection on rowsAboutToBeRemoved). Remove the check for the selection being empty in the after slot to handle this case. --- autotests/kselectionproxymodeltest.cpp | 22 ++++++++++++++++++++++ src/kselectionproxymodel.cpp | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/autotests/kselectionproxymodeltest.cpp b/autotests/kselectionproxymodeltest.cpp index b029964..77ca5e1 100644 --- a/autotests/kselectionproxymodeltest.cpp +++ b/autotests/kselectionproxymodeltest.cpp @@ -48,6 +48,7 @@ private Q_SLOTS: void columnCountShouldBeStable(); void selectOnSourceReset(); void selectionMapping(); + void removeSelected(); #if QT_VERSION >= QT_VERSION_CHECK(5, 5, 0) void selectionModelModelChange(); @@ -169,6 +170,27 @@ void KSelectionProxyModelTest::selectionModelModelChange() QCOMPARE(proxy.rowCount(), 1); QCOMPARE(proxy.index(0, 0).data().toString(), numbers.at(0)); } + +void KSelectionProxyModelTest::removeSelected() +{ + QStringListModel strings(days); + QItemSelectionModel selectionModel; + KSelectionProxyModel proxy(&selectionModel); + proxy.setFilterBehavior(KSelectionProxyModel::ExactSelection); + + proxy.setSourceModel(&strings); + selectionModel.setModel(&strings); + + QSignalSpy beforeSpy(&proxy, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int))); + QSignalSpy afterSpy(&proxy, SIGNAL(rowsRemoved(QModelIndex,int,int))); + + + selectionModel.select(strings.index(0, 0), QItemSelectionModel::Select); + strings.removeRow(0); + + QCOMPARE(beforeSpy.count(), 1); + QCOMPARE(afterSpy.count(), 1); +} #endif void KSelectionProxyModelTest::selectionMapping() diff --git a/src/kselectionproxymodel.cpp b/src/kselectionproxymodel.cpp index a542a17..5169cec 100644 --- a/src/kselectionproxymodel.cpp +++ b/src/kselectionproxymodel.cpp @@ -1291,7 +1291,7 @@ void KSelectionProxyModelPrivate::sourceRowsRemoved(const QModelIndex &parent, i Q_ASSERT(parent.isValid() ? parent.model() == q->sourceModel() : true); - if (!m_selectionModel || !m_selectionModel->hasSelection()) { + if (!m_selectionModel) { return; }