From: Frank Reininghaus <frank78ac@googlemail.com>
Date: Wed, 14 Aug 2013 22:15:51 +0000 (+0200)
Subject: Merge remote-tracking branch 'origin/KDE/4.11'
X-Git-Url: https://cloud.milkyroute.net/gitweb/dolphin.git/commitdiff_plain/6bfa0ccfc701df5af5f043fce3168b3840858212?hp=-c

Merge remote-tracking branch 'origin/KDE/4.11'
---

6bfa0ccfc701df5af5f043fce3168b3840858212
diff --combined src/kitemviews/kfileitemmodel.cpp
index c7e1c8600,70e8834a6..eb7b16461
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@@ -192,8 -192,30 +192,30 @@@ bool KFileItemModel::setData(int index
  
      emit itemsChanged(KItemRangeList() << KItemRange(index, 1), changedRoles);
  
-     if (changedRoles.contains(sortRole())) {
-         m_resortAllItemsTimer->start();
+     // Trigger a resorting if the item's correct position has changed. Note
+     // that this can happen even if the sort role has not changed at all
+     // because the file name can be used as a fallback.
+     if (changedRoles.contains(sortRole()) || changedRoles.contains(roleForType(NameRole))) {
+         // Compare the changed item with its neighbors to see
+         // if an expensive resorting is needed at all.
+         const ItemData* changedItem = m_itemData.at(index);
+         const ItemData* previousItem = (index == 0) ? 0 : m_itemData.at(index - 1);
+         const ItemData* nextItem = (index == m_itemData.count() - 1) ? 0 : m_itemData.at(index + 1);
+ 
+         if ((previousItem && lessThan(changedItem, previousItem))
+             || (nextItem && lessThan(nextItem, changedItem))) {
+             m_resortAllItemsTimer->start();
+         } else if (groupedSorting() && changedRoles.contains(sortRole())) {
+             // The position is still correct, but the groups might have changed
+             // if the changed item is either the first or the last item in a
+             // group.
+             // In principle, we could try to find out if the item really is the
+             // first or last one in its group and then update the groups
+             // (possibly with a delayed timer to make sure that we don't
+             // re-calculate the groups very often if items are updated one by
+             // one), but starting m_resortAllItemsTimer is easier.
+             m_resortAllItemsTimer->start();
+         }
      }
  
      return true;
@@@ -689,6 -711,7 +711,6 @@@ void KFileItemModel::resortAllItems(
          oldUrls.append(itemData->item.url());
      }
  
 -    m_groups.clear();
      m_items.clear();
  
      // Resort the items
@@@ -697,45 -720,20 +719,45 @@@
          m_items.insert(m_itemData.at(i)->item.url(), i);
      }
  
 -    // Determine the indexes that have been moved
 -    QList<int> movedToIndexes;
 -    movedToIndexes.reserve(itemCount);
 -    for (int i = 0; i < itemCount; i++) {
 -        const int newIndex = m_items.value(oldUrls.at(i));
 -        movedToIndexes.append(newIndex);
 +    // Determine the first index that has been moved.
 +    int firstMovedIndex = 0;
 +    while (firstMovedIndex < itemCount
 +           && firstMovedIndex == m_items.value(oldUrls.at(firstMovedIndex))) {
 +        ++firstMovedIndex;
      }
  
 -    // Don't check whether items have really been moved and always emit a
 -    // itemsMoved() signal after resorting: In case of grouped items
 -    // the groups might change even if the items themselves don't change their
 -    // position. Let the receiver of the signal decide whether a check for moved
 -    // items makes sense.
 -    emit itemsMoved(KItemRange(0, itemCount), movedToIndexes);
 +    const bool itemsHaveMoved = firstMovedIndex < itemCount;
 +    if (itemsHaveMoved) {
 +        m_groups.clear();
 +
 +        int lastMovedIndex = itemCount - 1;
 +        while (lastMovedIndex > firstMovedIndex
 +               && lastMovedIndex == m_items.value(oldUrls.at(lastMovedIndex))) {
 +            --lastMovedIndex;
 +        }
 +
 +        Q_ASSERT(firstMovedIndex <= lastMovedIndex);
 +
 +        // Create a list movedToIndexes, which has the property that
 +        // movedToIndexes[i] is the new index of the item with the old index
 +        // firstMovedIndex + i.
 +        const int movedItemsCount = lastMovedIndex - firstMovedIndex + 1;
 +        QList<int> movedToIndexes;
 +        movedToIndexes.reserve(movedItemsCount);
 +        for (int i = firstMovedIndex; i <= lastMovedIndex; ++i) {
 +            const int newIndex = m_items.value(oldUrls.at(i));
 +            movedToIndexes.append(newIndex);
 +        }
 +
 +        emit itemsMoved(KItemRange(firstMovedIndex, movedItemsCount), movedToIndexes);
 +    } else if (groupedSorting()) {
 +        // The groups might have changed even if the order of the items has not.
 +        const QList<QPair<int, QVariant> > oldGroups = m_groups;
 +        m_groups.clear();
 +        if (groups() != oldGroups) {
 +            emit groupsChanged();
 +        }
 +    }
  
  #ifdef KFILEITEMMODEL_DEBUG
      kDebug() << "[TIME] Resorting of" << itemCount << "items:" << timer.elapsed();
@@@ -1299,25 -1297,20 +1321,20 @@@ QHash<QByteArray, QVariant> KFileItemMo
      data.insert(sharedValue("url"), item.url());
  
      const bool isDir = item.isDir();
-     if (m_requestRole[IsDirRole]) {
-         data.insert(sharedValue("isDir"), isDir);
+     if (m_requestRole[IsDirRole] && isDir) {
+         data.insert(sharedValue("isDir"), true);
      }
  
-     if (m_requestRole[IsLinkRole]) {
-         const bool isLink = item.isLink();
-         data.insert(sharedValue("isLink"), isLink);
+     if (m_requestRole[IsLinkRole] && item.isLink()) {
+         data.insert(sharedValue("isLink"), true);
      }
  
      if (m_requestRole[NameRole]) {
          data.insert(sharedValue("text"), item.text());
      }
  
-     if (m_requestRole[SizeRole]) {
-         if (isDir) {
-             data.insert(sharedValue("size"), QVariant());
-         } else {
-             data.insert(sharedValue("size"), item.size());
-         }
+     if (m_requestRole[SizeRole] && !isDir) {
+         data.insert(sharedValue("size"), item.size());
      }
  
      if (m_requestRole[DateRole]) {
@@@ -1371,17 -1364,15 +1388,15 @@@
          data.insert(sharedValue("path"), path);
      }
  
-     if (m_requestRole[IsExpandableRole]) {
-         data.insert(sharedValue("isExpandable"), item.isDir());
+     if (m_requestRole[IsExpandableRole] && isDir) {
+         data.insert(sharedValue("isExpandable"), true);
      }
  
      if (m_requestRole[ExpandedParentsCountRole]) {
-         int level = 0;
          if (parent) {
-             level = parent->values["expandedParentsCount"].toInt() + 1;
+             const int level = parent->values["expandedParentsCount"].toInt() + 1;
+             data.insert(sharedValue("expandedParentsCount"), level);
          }
- 
-         data.insert(sharedValue("expandedParentsCount"), level);
      }
  
      if (item.isMimeTypeKnown()) {
diff --combined src/kitemviews/kitemlistview.cpp
index 80a4ba7e3,7f1526151..d8edcfc50
--- a/src/kitemviews/kitemlistview.cpp
+++ b/src/kitemviews/kitemlistview.cpp
@@@ -1229,13 -1229,6 +1229,13 @@@ void KItemListView::slotItemsChanged(co
      QAccessible::updateAccessibility(this, 0, QAccessible::TableModelChanged);
  }
  
 +void KItemListView::slotGroupsChanged()
 +{
 +    updateVisibleGroupHeaders();
 +    doLayout(NoAnimation);
 +    updateSiblingsInformation();
 +}
 +
  void KItemListView::slotGroupedSortingChanged(bool current)
  {
      m_grouped = current;
@@@ -1244,8 -1237,10 +1244,10 @@@
      if (m_grouped) {
          updateGroupHeaderHeight();
      } else {
-         // Clear all visible headers
-         QMutableHashIterator<KItemListWidget*, KItemListGroupHeader*> it (m_visibleGroups);
+         // Clear all visible headers. Note that the QHashIterator takes a copy of
+         // m_visibleGroups. Therefore, it remains valid even if items are removed
+         // from m_visibleGroups in recycleGroupHeaderForWidget().
+         QHashIterator<KItemListWidget*, KItemListGroupHeader*> it(m_visibleGroups);
          while (it.hasNext()) {
              it.next();
              recycleGroupHeaderForWidget(it.key());
@@@ -1528,8 -1523,6 +1530,8 @@@ void KItemListView::setModel(KItemModel
                     this,    SLOT(slotItemsRemoved(KItemRangeList)));
          disconnect(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)),
                     this,    SLOT(slotItemsMoved(KItemRange,QList<int>)));
 +        disconnect(m_model, SIGNAL(groupsChanged()),
 +                   this,    SLOT(slotGroupsChanged()));
          disconnect(m_model, SIGNAL(groupedSortingChanged(bool)),
                     this,    SLOT(slotGroupedSortingChanged(bool)));
          disconnect(m_model, SIGNAL(sortOrderChanged(Qt::SortOrder,Qt::SortOrder)),
@@@ -1553,8 -1546,6 +1555,8 @@@
                  this,    SLOT(slotItemsRemoved(KItemRangeList)));
          connect(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)),
                  this,    SLOT(slotItemsMoved(KItemRange,QList<int>)));
 +        connect(m_model, SIGNAL(groupsChanged()),
 +                this,    SLOT(slotGroupsChanged()));
          connect(m_model, SIGNAL(groupedSortingChanged(bool)),
                  this,    SLOT(slotGroupedSortingChanged(bool)));
          connect(m_model, SIGNAL(sortOrderChanged(Qt::SortOrder,Qt::SortOrder)),
diff --combined src/tests/kfileitemmodeltest.cpp
index 6d5d4b77c,f8439789b..e55e3eb33
--- a/src/tests/kfileitemmodeltest.cpp
+++ b/src/tests/kfileitemmodeltest.cpp
@@@ -49,7 -49,6 +49,7 @@@ namespace 
      const int DefaultTimeout = 5000;
  };
  
 +Q_DECLARE_METATYPE(KItemRange)
  Q_DECLARE_METATYPE(KItemRangeList)
  Q_DECLARE_METATYPE(QList<int>)
  
@@@ -71,6 -70,7 +71,7 @@@ private slots
      void testSetDataWithModifiedSortRole_data();
      void testSetDataWithModifiedSortRole();
      void testChangeSortRole();
+     void testResortAfterChangingName();
      void testModelConsistencyWhenInsertingItems();
      void testItemRangeConsistencyWhenInsertingItems();
      void testExpandItems();
@@@ -369,6 -369,30 +370,30 @@@ void KFileItemModelTest::testChangeSort
      QVERIFY(ok1 || ok2);
  }
  
+ void KFileItemModelTest::testResortAfterChangingName()
+ {
+     // We sort by size in a directory where all files have the same size.
+     // Therefore, the files are sorted by their names.
+     m_model->setSortRole("size");
+ 
+     QStringList files;
+     files << "a.txt" << "b.txt" << "c.txt";
+     m_testDir->createFiles(files);
+ 
+     m_model->loadDirectory(m_testDir->url());
+     QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout));
+     QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "c.txt");
+ 
+     // We rename a.txt to d.txt. Even though the size has not changed at all,
+     // the model must re-sort the items.
+     QHash<QByteArray, QVariant> data;
+     data.insert("text", "d.txt");
+     m_model->setData(0, data);
+ 
+     QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)), DefaultTimeout));
+     QCOMPARE(itemsInModel(), QStringList() << "b.txt" << "c.txt" << "d.txt");
+ }
+ 
  void KFileItemModelTest::testModelConsistencyWhenInsertingItems()
  {
      //QSKIP("Temporary disabled", SkipSingle);
@@@ -720,8 -744,7 +745,8 @@@ void KFileItemModelTest::testSorting(
      QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder);
      QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c" << "c-1" << "c-2" << "c-3" << "d" << "e");
      QCOMPARE(spyItemsMoved.count(), 1);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 2 << 4 << 5 << 3 << 0 << 1 << 6 << 7);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(0, 6));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 2 << 4 << 5 << 3 << 0 << 1);
  
      // Sort by Name, descending
      m_model->setSortDirectoriesFirst(true);
@@@ -730,10 -753,8 +755,10 @@@
      QCOMPARE(m_model->sortOrder(), Qt::DescendingOrder);
      QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "e" << "d" << "b" << "a");
      QCOMPARE(spyItemsMoved.count(), 2);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 4 << 5 << 0 << 3 << 1 << 2 << 6 << 7);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 0 << 1 << 2 << 3 << 7 << 6 << 5 << 4);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(0, 6));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 4 << 5 << 0 << 3 << 1 << 2);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(4, 4));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 7 << 6 << 5 << 4);
  
      // Sort by Date, descending
      m_model->setSortDirectoriesFirst(true);
@@@ -742,8 -763,7 +767,8 @@@
      QCOMPARE(m_model->sortOrder(), Qt::DescendingOrder);
      QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "b" << "d" << "a" << "e");
      QCOMPARE(spyItemsMoved.count(), 1);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 0 << 1 << 2 << 3 << 7 << 5 << 4 << 6);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(4, 4));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 7 << 5 << 4 << 6);
  
      // Sort by Date, ascending
      m_model->setSortOrder(Qt::AscendingOrder);
@@@ -751,8 -771,7 +776,8 @@@
      QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder);
      QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "e" << "a" << "d" << "b");
      QCOMPARE(spyItemsMoved.count(), 1);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 0 << 1 << 2 << 3 << 7 << 6 << 5 << 4);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(4, 4));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 7 << 6 << 5 << 4);
  
      // Sort by Date, ascending, 'Sort Folders First' disabled
      m_model->setSortDirectoriesFirst(false);
@@@ -761,8 -780,7 +786,8 @@@
      QVERIFY(!m_model->sortDirectoriesFirst());
      QCOMPARE(itemsInModel(), QStringList() << "e" << "a" << "c" << "c-1" << "c-2" << "c-3" << "d" << "b");
      QCOMPARE(spyItemsMoved.count(), 1);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 2 << 4 << 5 << 3 << 0 << 1 << 6 << 7);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(0, 6));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 2 << 4 << 5 << 3 << 0 << 1);
  
      // Sort by Name, ascending, 'Sort Folders First' disabled
      m_model->setSortRole("text");
@@@ -770,7 -788,6 +795,7 @@@
      QVERIFY(!m_model->sortDirectoriesFirst());
      QCOMPARE(itemsInModel(), QStringList() << "a" << "b" << "c" << "c-1" << "c-2" << "c-3" << "d" << "e");
      QCOMPARE(spyItemsMoved.count(), 1);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(0, 8));
      QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 7 << 0 << 2 << 3 << 4 << 5 << 6 << 1);
  
      // Sort by Size, ascending, 'Sort Folders First' disabled
@@@ -780,15 -797,19 +805,15 @@@
      QVERIFY(!m_model->sortDirectoriesFirst());
      QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "a" << "b" << "e" << "d");
      QCOMPARE(spyItemsMoved.count(), 1);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(0, 8));
      QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 4 << 5 << 0 << 3 << 1 << 2 << 7 << 6);
  
 -    QSKIP("2 tests of testSorting() are temporary deactivated as in KFileItemModel resortAllItems() "
 -          "always emits a itemsMoved() signal. Before adjusting the tests think about probably introducing "
 -          "another signal", SkipSingle);
 -    // Internal note: Check comment in KFileItemModel::resortAllItems() for details.
 -
      // In 'Sort by Size' mode, folders are always first -> changing 'Sort Folders First' does not resort the model
      m_model->setSortDirectoriesFirst(true);
      QCOMPARE(m_model->sortRole(), QByteArray("size"));
      QCOMPARE(m_model->sortOrder(), Qt::AscendingOrder);
      QVERIFY(m_model->sortDirectoriesFirst());
 -    QCOMPARE(itemsInModel(), QStringList() << "c" << "a" << "b" << "e" << "d");
 +    QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "a" << "b" << "e" << "d");
      QCOMPARE(spyItemsMoved.count(), 0);
  
      // Sort by Size, descending, 'Sort Folders First' enabled
@@@ -796,10 -817,9 +821,10 @@@
      QCOMPARE(m_model->sortRole(), QByteArray("size"));
      QCOMPARE(m_model->sortOrder(), Qt::DescendingOrder);
      QVERIFY(m_model->sortDirectoriesFirst());
 -    QCOMPARE(itemsInModel(), QStringList() << "c" << "d" << "e" << "b" << "a");
 +    QCOMPARE(itemsInModel(), QStringList() << "c" << "c-2" << "c-3" << "c-1" << "d" << "e" << "b" << "a");
      QCOMPARE(spyItemsMoved.count(), 1);
 -    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 0 << 4 << 3 << 2 << 1);
 +    QCOMPARE(spyItemsMoved.first().at(0).value<KItemRange>(), KItemRange(4, 4));
 +    QCOMPARE(spyItemsMoved.takeFirst().at(1).value<QList<int> >(), QList<int>() << 7 << 6 << 5 << 4);
  
      // TODO: Sort by other roles; show/hide hidden files
  }
@@@ -1217,7 -1237,7 +1242,7 @@@ void KFileItemModelTest::testNameRoleGr
      // Rename c.txt to d.txt.
      data.insert("text", "d.txt");
      m_model->setData(2, data);
 -    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsMoved(KItemRange,QList<int>)), DefaultTimeout));
 +    QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(groupsChanged()), DefaultTimeout));
      QCOMPARE(itemsInModel(), QStringList() << "a.txt" << "b.txt" << "d.txt" << "e.txt");
  
      expectedGroups.clear();