diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index d0fd3d77c..60fc27546 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -1,5 +1,6 @@ /*************************************************************************** * Copyright (C) 2011 by Peter Penz * + * Copyright (C) 2013 by Frank Reininghaus * * * * This program is free software; you can redistribute it and/or modify * * it under the terms of the GNU General Public License as published by * @@ -1338,11 +1339,34 @@ bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b) const { int result = 0; - if (m_expandedParentsCountRoot >= 0) { - result = expandedParentsCountCompare(a, b); - if (result != 0) { - // The items have parents with different expansion levels - return (sortOrder() == Qt::AscendingOrder) ? result < 0 : result > 0; + if (m_expandedParentsCountRoot >= 0 && a->parent != b->parent) { + const int expansionLevelA = a->values.value("expandedParentsCount").toInt(); + const int expansionLevelB = b->values.value("expandedParentsCount").toInt(); + + // If b has a higher expansion level than a, check if a is a parent + // of b, and make sure that both expansion levels are equal otherwise. + for (int i = expansionLevelB; i > expansionLevelA; --i) { + if (b->parent == a) { + return true; + } + b = b->parent; + } + + // If a has a higher expansion level than a, check if b is a parent + // of a, and make sure that both expansion levels are equal otherwise. + for (int i = expansionLevelA; i > expansionLevelB; --i) { + if (a->parent == b) { + return false; + } + a = a->parent; + } + + Q_ASSERT(a->values.value("expandedParentsCount").toInt() == b->values.value("expandedParentsCount").toInt()); + + // Compare the last parents of a and b which are different. + while (a->parent != b->parent) { + a = a->parent; + b = b->parent; } } @@ -1523,88 +1547,6 @@ int KFileItemModel::stringCompare(const QString& a, const QString& b) const : QString::compare(a, b, Qt::CaseSensitive); } -int KFileItemModel::expandedParentsCountCompare(const ItemData* a, const ItemData* b) const -{ - const KUrl urlA = a->item.url(); - const KUrl urlB = b->item.url(); - if (urlA.directory() == urlB.directory()) { - // Both items have the same directory as parent - return 0; - } - - // Check whether one item is the parent of the other item - if (urlA.isParentOf(urlB)) { - return (sortOrder() == Qt::AscendingOrder) ? -1 : +1; - } else if (urlB.isParentOf(urlA)) { - return (sortOrder() == Qt::AscendingOrder) ? +1 : -1; - } - - // Determine the maximum common path of both items and - // remember the index in 'index' - const QString pathA = urlA.path(); - const QString pathB = urlB.path(); - - const int maxIndex = qMin(pathA.length(), pathB.length()) - 1; - int index = 0; - while (index <= maxIndex && pathA.at(index) == pathB.at(index)) { - ++index; - } - if (index > maxIndex) { - index = maxIndex; - } - while (index > 0 && (pathA.at(index) != QLatin1Char('/') || pathB.at(index) != QLatin1Char('/'))) { - --index; - } - - // Determine the first sub-path after the common path and - // check whether it represents a directory or already a file - bool isDirA = true; - const QString subPathA = subPath(a->item, pathA, index, &isDirA); - bool isDirB = true; - const QString subPathB = subPath(b->item, pathB, index, &isDirB); - - if (m_sortDirsFirst || m_sortRole == SizeRole) { - if (isDirA && !isDirB) { - return (sortOrder() == Qt::AscendingOrder) ? -1 : +1; - } else if (!isDirA && isDirB) { - return (sortOrder() == Qt::AscendingOrder) ? +1 : -1; - } - } - - // Compare the items of the parents that represent the first - // different path after the common path. - const QString parentPathA = pathA.left(index) + subPathA; - const QString parentPathB = pathB.left(index) + subPathB; - - const ItemData* parentA = a; - while (parentA && parentA->item.url().path() != parentPathA) { - parentA = parentA->parent; - } - - const ItemData* parentB = b; - while (parentB && parentB->item.url().path() != parentPathB) { - parentB = parentB->parent; - } - - if (parentA && parentB) { - return sortRoleCompare(parentA, parentB); - } - - kWarning() << "Child items without parent detected:" << a->item.url() << b->item.url(); - return QString::compare(urlA.url(), urlB.url(), Qt::CaseSensitive); -} - -QString KFileItemModel::subPath(const KFileItem& item, - const QString& itemPath, - int start, - bool* isDir) const -{ - Q_ASSERT(isDir); - const int pathIndex = itemPath.indexOf('/', start + 1); - *isDir = (pathIndex > 0) || item.isDir(); - return itemPath.mid(start, pathIndex - start); -} - bool KFileItemModel::useMaximumUpdateInterval() const { return !m_dirLister->url().isLocalFile(); diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h index 903291a4c..a05d1f9d8 100644 --- a/src/kitemviews/kfileitemmodel.h +++ b/src/kitemviews/kfileitemmodel.h @@ -356,22 +356,6 @@ private: int stringCompare(const QString& a, const QString& b) const; - /** - * Compares the expansion level of both items. The "expansion level" is defined - * by the number of parent directories. However simply comparing just the numbers - * is not sufficient, it is also important to check the hierarchy for having - * a correct order like shown in a tree. - */ - int expandedParentsCountCompare(const ItemData* a, const ItemData* b) const; - - /** - * Helper method for expandedParentsCountCompare(). - */ - QString subPath(const KFileItem& item, - const QString& itemPath, - int start, - bool* isDir) const; - bool useMaximumUpdateInterval() const; QList > nameRoleGroups() const; diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 85a46488f..c9f8a3468 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -21,6 +21,8 @@ #include #include +#include + #include "kitemviews/kfileitemmodel.h" #include "kitemviews/private/kfileitemmodeldirlister.h" #include "testdir.h" @@ -71,6 +73,7 @@ private slots: void testItemRangeConsistencyWhenInsertingItems(); void testExpandItems(); void testExpandParentItems(); + void testMakeExpandedItemHidden(); void testSorting(); void testIndexForKeyboardSearch(); void testNameFilter(); @@ -569,6 +572,55 @@ void KFileItemModelTest::testExpandParentItems() QVERIFY(m_model->isConsistent()); } +/** + * Renaming an expanded folder by prepending its name with a dot makes it + * hidden. Verify that this does not cause an inconsistent model state and + * a crash later on, see https://bugs.kde.org/show_bug.cgi?id=311947 + */ +void KFileItemModelTest::testMakeExpandedItemHidden() +{ + QSet modelRoles = m_model->roles(); + modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount"; + m_model->setRoles(modelRoles); + + QStringList files; + m_testDir->createFile("1a/2a/3a"); + m_testDir->createFile("1a/2a/3b"); + m_testDir->createFile("1a/2b"); + m_testDir->createFile("1b"); + + m_model->loadDirectory(m_testDir->url()); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + + // So far, the model contains only "1a/" and "1b". + QCOMPARE(m_model->count(), 2); + m_model->setExpanded(0, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + + // Now "1a/2a" and "1a/2b" have appeared. + QCOMPARE(m_model->count(), 4); + m_model->setExpanded(1, true); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(m_model->count(), 6); + + // Rename "1a/2" and make it hidden. + const QString oldPath = m_model->fileItem(0).url().path() + "/2a"; + const QString newPath = m_model->fileItem(0).url().path() + "/.2a"; + + KIO::SimpleJob* job = KIO::rename(oldPath, newPath, KIO::HideProgressInfo); + bool ok = job->exec(); + QVERIFY(ok); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsRemoved(KItemRangeList)), DefaultTimeout)); + + // "1a/2" and its subfolders have disappeared now. + QVERIFY(m_model->isConsistent()); + QCOMPARE(m_model->count(), 3); + + m_model->setExpanded(0, false); + QCOMPARE(m_model->count(), 2); + +} + void KFileItemModelTest::testSorting() { // Create some files with different sizes and modification times to check the different sorting options