Fix inconsistency in selection manager

When a selection has been done with non-linear ranges, it was possible that
the anchor item pointed to an invalid index that resulted into an invalid
selection.

As part of this fix the sorting for DolphinView::selectedItems() has been
disabled (if the caller assumes a sorted selection he must manually adjust it).

BUG: 288908
FIXED-IN: 4.8.0

diff --git a/src/kitemviews/kitemlistselectionmanager.cpp b/src/kitemviews/kitemlistselectionmanager.cpp
index 448e792930699f8d0258317073077856c5a0208b..79c3370b47016980d0d950c8184eacbe3055458f 100644 (file)
--- a/src/kitemviews/kitemlistselectionmanager.cpp
+++ b/src/kitemviews/kitemlistselectionmanager.cpp
@@ -288,70 +288,33 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges)
     // Update the current item
     if (m_currentItem >= 0) {
         const int previousCurrent = m_currentItem;
-        int currentItem = m_currentItem;
-        foreach (const KItemRange& itemRange, itemRanges) {
-            if (currentItem < itemRange.index) {
-                break;
-            }
-
-            if (currentItem >= itemRange.index + itemRange.count) {
-                currentItem -= itemRange.count;
-            }
-
-            if (currentItem >= m_model->count()) {
-                currentItem = m_model->count() - 1;
-            }
-        }
-        // Calling setCurrentItem would trigger the selectionChanged signal, but we want to
+        // Calling setCurrentItem() would trigger the selectionChanged signal, but we want to
         // emit it only once in this function -> change the current item manually and emit currentChanged
-        m_currentItem = currentItem;
-        Q_ASSERT(m_currentItem < m_model->count());
-        emit currentChanged(m_currentItem, previousCurrent);
+        m_currentItem = indexAfterRangesRemoving(m_currentItem, itemRanges);
+        if (m_currentItem < 0) {
+            m_currentItem = qMin(previousCurrent, m_model->count() - 1);
+        }
+        if (m_currentItem != previousCurrent) {
+            emit currentChanged(m_currentItem, previousCurrent);
+        }
     }
 
     // Update the anchor item
     if (m_anchorItem >= 0) {
-        int anchorItem = m_anchorItem;
-        foreach (const KItemRange& itemRange, itemRanges) {
-            if (anchorItem < itemRange.index) {
-                break;
-            }
-            if (anchorItem >= itemRange.index + itemRange.count) {
-                anchorItem -= itemRange.count;
-            } else if (anchorItem >= m_model->count()) {
-                anchorItem = m_model->count() - 1;
-            }
-        }
-        m_anchorItem = anchorItem;
+        m_anchorItem = indexAfterRangesRemoving(m_anchorItem, itemRanges);
         if (m_anchorItem < 0) {
             m_isAnchoredSelectionActive = false;
         }
     }
 
-    // Update the selections
+    // Update the selections and the anchor item
     if (!m_selectedItems.isEmpty()) {
         const QSet<int> previous = m_selectedItems;
         m_selectedItems.clear();
         m_selectedItems.reserve(previous.count());
         QSetIterator<int> it(previous);
         while (it.hasNext()) {
-            int index = it.next();
-            int dec = 0;
-            foreach (const KItemRange& itemRange, itemRanges) {
-                if (index < itemRange.index) {
-                    break;
-                }
-
-                if (index < itemRange.index + itemRange.count) {
-                    // The selection is part of the removed range
-                    // and will get deleted
-                    index = -1;
-                    break;
-                }
-
-                dec += itemRange.count;
-            }
-            index -= dec;
+            const int index = indexAfterRangesRemoving(it.next(), itemRanges);
             if (index >= 0)  {
                 m_selectedItems.insert(index);
             }
@@ -362,6 +325,9 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges)
     if (selection != previousSelection) {
         emit selectionChanged(selection, previousSelection);
     }
+
+    Q_ASSERT(m_currentItem < m_model->count());
+    Q_ASSERT(m_anchorItem < m_model->count());
 }
 
 void KItemListSelectionManager::itemsMoved(const KItemRange& itemRange, const QList<int>& movedToIndexes)
@@ -408,4 +374,21 @@ void KItemListSelectionManager::itemsMoved(const KItemRange& itemRange, const QL
     }
 }
 
+int KItemListSelectionManager::indexAfterRangesRemoving(int index, const KItemRangeList& itemRanges) const
+{
+    int dec = 0;
+    foreach (const KItemRange& itemRange, itemRanges) {
+        if (index < itemRange.index) {
+            break;
+        }
+
+        if (index < itemRange.index + itemRange.count) {
+            // The index is part of the removed range
+            return -1;
+        }
+
+        dec += itemRange.count;
+    }
+    return index - dec;
+}
 #include "kitemlistselectionmanager.moc"

diff --git a/src/kitemviews/kitemlistselectionmanager.h b/src/kitemviews/kitemlistselectionmanager.h
index 82ae13abb552bd66694102d73c5926a85c9d812a..4afad1f8b811dfb58b09876fa44a5d9d67f4058d 100644 (file)
--- a/src/kitemviews/kitemlistselectionmanager.h
+++ b/src/kitemviews/kitemlistselectionmanager.h
@@ -76,6 +76,13 @@ private:
     void itemsRemoved(const KItemRangeList& itemRanges);
     void itemsMoved(const KItemRange& itemRange, const QList<int>& movedToIndexes);
 
+
+    /**
+     * Helper method for itemsRemoved. Returns the changed index after removing
+     * the given range. If the index is part of the range, -1 will be returned.
+     */
+    int indexAfterRangesRemoving(int index, const KItemRangeList& itemRanges) const;
+
 private:
     int m_currentItem;
     int m_anchorItem;

diff --git a/src/views/dolphinview.cpp b/src/views/dolphinview.cpp
index f17749b8318ea2e01fcaa2a1134ce99b2de264c8..a31bf566dc920e9684cc099138c5bf65825fa3cc 100644 (file)
--- a/src/views/dolphinview.cpp
+++ b/src/views/dolphinview.cpp
@@ -311,11 +311,8 @@ KFileItemList DolphinView::selectedItems() const
     const KItemListSelectionManager* selectionManager = m_container->controller()->selectionManager();
     const QSet<int> selectedIndexes = selectionManager->selectedItems();
 
-    QList<int> sortedIndexes = selectedIndexes.toList();
-    qSort(sortedIndexes);
-
     KFileItemList selectedItems;
-    QListIterator<int> it(sortedIndexes);
+    QSetIterator<int> it(selectedIndexes);
     while (it.hasNext()) {
         const int index = it.next();
         selectedItems.append(model->fileItem(index));