Fix issues with the anchor selection

Don't change the selection if the anchor is invalid. This fixes
the issue that items might get selected during changing a directory.

diff --git a/src/kitemviews/kitemlistselectionmanager.cpp b/src/kitemviews/kitemlistselectionmanager.cpp
index 9aaf22f5853fbff4d37cdb724706ea3c8f0ac390..764227004528bd5dbf15941db43e92b83273e941 100644 (file)
--- a/src/kitemviews/kitemlistselectionmanager.cpp
+++ b/src/kitemviews/kitemlistselectionmanager.cpp
@@ -81,11 +81,13 @@ QSet<int> KItemListSelectionManager::selectedItems() const
 {
     QSet<int> selectedItems = m_selectedItems;
 
-    if (m_isAnchoredSelectionActive && (m_anchorItem != m_currentItem)) {
+    if (m_isAnchoredSelectionActive && m_anchorItem != m_currentItem) {
+        Q_ASSERT(m_anchorItem >= 0);
+        Q_ASSERT(m_currentItem >= 0);
         const int from = qMin(m_anchorItem, m_currentItem);
         const int to = qMax(m_anchorItem, m_currentItem);
 
-        for (int index = from; index <= to; index++) {
+        for (int index = from; index <= to; ++index) {
             selectedItems.insert(index);
         }
     }
@@ -95,7 +97,7 @@ QSet<int> KItemListSelectionManager::selectedItems() const
 
 bool KItemListSelectionManager::hasSelection() const
 {
-    return !m_selectedItems.isEmpty() || (m_isAnchoredSelectionActive && (m_anchorItem != m_currentItem));
+    return !m_selectedItems.isEmpty() || (m_isAnchoredSelectionActive && m_anchorItem != m_currentItem);
 }
 
 void KItemListSelectionManager::setSelected(int index, int count, SelectionMode mode)
@@ -156,17 +158,22 @@ void KItemListSelectionManager::clearSelection()
 
 void KItemListSelectionManager::beginAnchoredSelection(int anchor)
 {
-    m_isAnchoredSelectionActive = true;
-    setAnchorItem(anchor);
+    if (anchor >= 0 && m_model && anchor < m_model->count()) {
+        m_isAnchoredSelectionActive = true;
+        setAnchorItem(anchor);
+        Q_ASSERT(m_anchorItem >= 0);
+    }
 }
 
 void KItemListSelectionManager::endAnchoredSelection()
 {
     if (m_isAnchoredSelectionActive && (m_anchorItem != m_currentItem)) {
+        Q_ASSERT(m_anchorItem >= 0);
+        Q_ASSERT(m_currentItem >= 0);
         const int from = qMin(m_anchorItem, m_currentItem);
         const int to = qMax(m_anchorItem, m_currentItem);
 
-        for (int index = from; index <= to; index++) {
+        for (int index = from; index <= to; ++index) {
             m_selectedItems.insert(index);
         }
     }
@@ -176,14 +183,18 @@ void KItemListSelectionManager::endAnchoredSelection()
 
 void KItemListSelectionManager::setAnchorItem(int anchor)
 {
-    const int previous = m_anchorItem;
-    if (m_model && anchor < m_model->count()) {
-        m_anchorItem = anchor;
-    } else {
-        m_anchorItem = -1;
+    if (!m_isAnchoredSelectionActive) {
+        return;
+    }
+
+    if (anchor < 0 || (m_model && anchor >= m_model->count())) {
+        // Index is out of range
+        return;
     }
 
-    if (m_anchorItem != previous) {
+    if (m_anchorItem != anchor) {
+        const int previous = m_anchorItem;
+        m_anchorItem = anchor;
         emit anchorChanged(m_anchorItem, previous);
     }
 }
@@ -198,11 +209,6 @@ bool KItemListSelectionManager::isAnchoredSelectionActive() const
     return m_isAnchoredSelectionActive;
 }
 
-void KItemListSelectionManager::setAnchoredSelectionActive(bool active)
-{
-    m_isAnchoredSelectionActive = active;
-}
-
 KItemModelBase* KItemListSelectionManager::model() const
 {
     return m_model;
@@ -319,6 +325,10 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges)
             }
         }
         m_anchorItem = anchorItem;
+        if (m_anchorItem < 0) {
+            m_isAnchoredSelectionActive = false;
+        }
+
         emit anchorChanged(m_anchorItem, previousAnchor);
     }
 

diff --git a/src/kitemviews/kitemlistselectionmanager.h b/src/kitemviews/kitemlistselectionmanager.h
index dd4c3e4fca6d3d8b94080accf71e0fd4025ec53b..f8c33bb79f1b182678d7bc6caeeb3c03f73998bc 100644 (file)
--- a/src/kitemviews/kitemlistselectionmanager.h
+++ b/src/kitemviews/kitemlistselectionmanager.h
@@ -61,11 +61,17 @@ public:
 
     void beginAnchoredSelection(int anchor);
     void endAnchoredSelection();
+
+    /**
+     * Sets the anchor to \a anchor and emits anchorChanged() if the
+     * anchor differs from the current anchor value. If no anchor selection is active (see
+     * KItemListSelectionManager::beginAnchoredSelection()) or the index
+     * is not within the available model items the anchor will not be modified.
+     */
     void setAnchorItem(int anchor);
     int anchorItem() const;
 
     bool isAnchoredSelectionActive() const;
-    void setAnchoredSelectionActive(bool active);
 
     KItemModelBase* model() const;
 

diff --git a/src/tests/kitemlistselectionmanagertest.cpp b/src/tests/kitemlistselectionmanagertest.cpp
index 5a3ffd55c71247378a362cd80581559157b550a2..0e09a5dde4358cc3e98e7636ac650b71d12e9465 100644 (file)
--- a/src/tests/kitemlistselectionmanagertest.cpp
+++ b/src/tests/kitemlistselectionmanagertest.cpp
@@ -98,9 +98,6 @@ void KItemListSelectionManagerTest::testCurrentItemAnchorItem()
     QSignalSpy spyCurrent(m_selectionManager, SIGNAL(currentChanged(int,int)));
     QSignalSpy spyAnchor(m_selectionManager, SIGNAL(anchorChanged(int,int)));
 
-    m_selectionManager->setAnchoredSelectionActive(true);
-    QVERIFY(m_selectionManager->isAnchoredSelectionActive());
-
     // Set current item and check that the selection manager emits the currentChanged(int,int) signal correctly.
     m_selectionManager->setCurrentItem(4);
     QCOMPARE(m_selectionManager->currentItem(), 4);
@@ -109,7 +106,8 @@ void KItemListSelectionManagerTest::testCurrentItemAnchorItem()
     spyCurrent.takeFirst();
 
     // Set anchor item and check that the selection manager emits the anchorChanged(int,int) signal correctly.
-    m_selectionManager->setAnchorItem(3);
+    m_selectionManager->beginAnchoredSelection(3);
+    QVERIFY(m_selectionManager->isAnchoredSelectionActive());
     QCOMPARE(m_selectionManager->anchorItem(), 3);
     QCOMPARE(spyAnchor.count(), 1);
     QCOMPARE(qvariant_cast<int>(spyAnchor.at(0).at(0)), 3);
@@ -183,6 +181,9 @@ void KItemListSelectionManagerTest::testCurrentItemAnchorItem()
     m_selectionManager->clearSelection();
     QCOMPARE(m_selectionManager->selectedItems(), QSet<int>());
     QVERIFY(!m_selectionManager->hasSelection());
+
+    m_selectionManager->endAnchoredSelection();
+    QVERIFY(!m_selectionManager->isAnchoredSelectionActive());
 }
 
 void KItemListSelectionManagerTest::testSetSelected_data()
@@ -339,7 +340,7 @@ void KItemListSelectionManagerTest::testChangeSelection_data()
         << 2 << 3
         << (QSet<int>() << 2 << 3 << 5 << 6)
         << NoChange << KItemRangeList()
-        << (QSet<int>() << 2 << 3 << 5 << 6);    
+        << (QSet<int>() << 2 << 3 << 5 << 6);
 
     QTest::newRow("Insert Items")
         << (QSet<int>() << 5 << 6)