From 9e8abb63ba4609887d988ee15ba6daee0b01380e Mon Sep 17 00:00:00 2001 From: Alexis Menard Date: Thu, 10 Sep 2009 11:40:42 +0200 Subject: Fix random selection when the order is descending. This commit fix the random selection that appear when the sort is not ascending. The problem is that sometimes the sort is not yet made (timer is not yet fired) so the visible children list contains both sorted items and non sorted items (at the end). translateVisibleLocation was buggy assuming that the list is always sorted. We have now a dirty index that indicate where the dirty items start. And then when the sort is made the dirty index is reset. I have added auto-test for that and fix one that was broken for Mac. The new version of the auto-test showed a crash because of this broken selection. Task-number:258751 Reviewed-by:thierry --- src/gui/dialogs/qfilesystemmodel.cpp | 6 ++ src/gui/dialogs/qfilesystemmodel_p.h | 13 ++++- tests/auto/qfiledialog/tst_qfiledialog.cpp | 30 +++++++++- .../auto/qfilesystemmodel/tst_qfilesystemmodel.cpp | 64 ++++++++++++++++------ 4 files changed, 91 insertions(+), 22 deletions(-) diff --git a/src/gui/dialogs/qfilesystemmodel.cpp b/src/gui/dialogs/qfilesystemmodel.cpp index 66d9bb0..b9012c7 100644 --- a/src/gui/dialogs/qfilesystemmodel.cpp +++ b/src/gui/dialogs/qfilesystemmodel.cpp @@ -1104,6 +1104,8 @@ void QFileSystemModelPrivate::sortChildren(int column, const QModelIndex &parent qStableSort(values.begin(), values.end(), ms); // First update the new visible list indexNode->visibleChildren.clear(); + //No more dirty item we reset our internal dirty index + indexNode->dirtyChildrenIndex = -1; for (int i = 0; i < values.count(); ++i) { indexNode->visibleChildren.append(values.at(i).first->fileName); values.at(i).first->isVisible = true; @@ -1706,6 +1708,10 @@ void QFileSystemModelPrivate::addVisibleFiles(QFileSystemNode *parentNode, const if (!indexHidden) { q->beginInsertRows(parent, parentNode->visibleChildren.count() , parentNode->visibleChildren.count() + newFiles.count() - 1); } + + if (parentNode->dirtyChildrenIndex == -1) + parentNode->dirtyChildrenIndex = parentNode->visibleChildren.count(); + for (int i = 0; i < newFiles.count(); ++i) { parentNode->visibleChildren.append(newFiles.at(i)); parentNode->children[newFiles.at(i)]->isVisible = true; diff --git a/src/gui/dialogs/qfilesystemmodel_p.h b/src/gui/dialogs/qfilesystemmodel_p.h index 9734f1c..175159f 100644 --- a/src/gui/dialogs/qfilesystemmodel_p.h +++ b/src/gui/dialogs/qfilesystemmodel_p.h @@ -84,7 +84,7 @@ public: { public: QFileSystemNode(const QString &filename = QString(), QFileSystemNode *p = 0) - : fileName(filename), populatedChildren(false), isVisible(false), parent(p), info(0) {} + : fileName(filename), populatedChildren(false), isVisible(false), dirtyChildrenIndex(-1), parent(p), info(0) {} ~QFileSystemNode() { QHash::const_iterator i = children.constBegin(); while (i != children.constEnd()) { @@ -194,6 +194,7 @@ public: bool isVisible; QHash children; QList visibleChildren; + int dirtyChildrenIndex; QFileSystemNode *parent; @@ -237,7 +238,15 @@ public: void sortChildren(int column, const QModelIndex &parent); inline int translateVisibleLocation(QFileSystemNode *parent, int row) const { - return (sortOrder == Qt::AscendingOrder) ? row : parent->visibleChildren.count() - row - 1; + if (sortOrder == Qt::AscendingOrder) + return row; + if (parent->dirtyChildrenIndex == -1 || row < parent->dirtyChildrenIndex) + if (parent->dirtyChildrenIndex != -1) + return parent->dirtyChildrenIndex - row - 1; + else + return parent->visibleChildren.count() - row - 1; + else + return row; } inline static QString myComputer() { diff --git a/tests/auto/qfiledialog/tst_qfiledialog.cpp b/tests/auto/qfiledialog/tst_qfiledialog.cpp index 6890610..18875e7 100644 --- a/tests/auto/qfiledialog/tst_qfiledialog.cpp +++ b/tests/auto/qfiledialog/tst_qfiledialog.cpp @@ -1814,6 +1814,8 @@ void tst_QFiledialog::task228844_ensurePreviousSorting() current.mkdir("e"); current.mkdir("f"); current.mkdir("g"); + QTemporaryFile *tempFile = new QTemporaryFile(current.absolutePath() + "/rXXXXXX"); + tempFile->open(); current.cdUp(); QNonNativeFileDialog fd; @@ -1825,25 +1827,47 @@ void tst_QFiledialog::task228844_ensurePreviousSorting() tree->header()->setSortIndicator(3,Qt::DescendingOrder); QTest::qWait(200); QDialogButtonBox *buttonBox = qFindChild(&fd, "buttonBox"); - QPushButton *button = buttonBox->button(QDialogButtonBox::Cancel); + QPushButton *button = buttonBox->button(QDialogButtonBox::Open); QTest::mouseClick(button, Qt::LeftButton); QTest::qWait(500); QNonNativeFileDialog fd2; + fd2.setFileMode(QFileDialog::Directory); fd2.restoreState(fd.saveState()); current.cd("aaaaaaaaaaaaaaaaaa"); fd2.setDirectory(current.absolutePath()); fd2.show(); QTest::qWait(500); QTreeView *tree2 = qFindChild(&fd2, "treeView"); + tree2->setFocus(); QCOMPARE(tree2->rootIndex().data(QFileSystemModel::FilePathRole).toString(),current.absolutePath()); QDialogButtonBox *buttonBox2 = qFindChild(&fd2, "buttonBox"); - QPushButton *button2 = buttonBox2->button(QDialogButtonBox::Cancel); + QPushButton *button2 = buttonBox2->button(QDialogButtonBox::Open); + fd2.selectFile("g"); QTest::mouseClick(button2, Qt::LeftButton); QTest::qWait(500); + QCOMPARE(fd2.selectedFiles().first(), current.absolutePath() + QChar('/') + QLatin1String("g")); + + QNonNativeFileDialog fd3(0, "This is a third file dialog", tempFile->fileName()); + fd3.restoreState(fd.saveState()); + fd3.setFileMode(QFileDialog::Directory); + fd3.show(); + QTest::qWait(500); + QTreeView *tree3 = qFindChild(&fd3, "treeView"); + tree3->setFocus(); + + QCOMPARE(tree3->rootIndex().data(QFileSystemModel::FilePathRole).toString(), current.absolutePath()); + + QDialogButtonBox *buttonBox3 = qFindChild(&fd3, "buttonBox"); + QPushButton *button3 = buttonBox3->button(QDialogButtonBox::Open); + QTest::mouseClick(button3, Qt::LeftButton); + QTest::qWait(500); + + QCOMPARE(fd3.selectedFiles().first(), tempFile->fileName()); + current.cd("aaaaaaaaaaaaaaaaaa"); current.rmdir("a"); current.rmdir("b"); @@ -1852,6 +1876,8 @@ void tst_QFiledialog::task228844_ensurePreviousSorting() current.rmdir("e"); current.rmdir("f"); current.rmdir("g"); + tempFile->close(); + delete tempFile; current.cdUp(); current.rmdir("aaaaaaaaaaaaaaaaaa"); } diff --git a/tests/auto/qfilesystemmodel/tst_qfilesystemmodel.cpp b/tests/auto/qfilesystemmodel/tst_qfilesystemmodel.cpp index 04e60c4..a388f0a 100644 --- a/tests/auto/qfilesystemmodel/tst_qfilesystemmodel.cpp +++ b/tests/auto/qfilesystemmodel/tst_qfilesystemmodel.cpp @@ -44,6 +44,7 @@ #include "../../../src/gui/dialogs/qfilesystemmodel_p.h" #include #include +#include #include "../../shared/util.h" #include #include @@ -802,38 +803,65 @@ void tst_QFileSystemModel::sort() if (fileDialogMode) myModel->d_func()->disableRecursiveSort = true; - const QString dirPath = QString("%1/sortTemp").arg(QDir::tempPath()); - QDir dir(dirPath); - dir.mkpath(dirPath); + QDir dir(QDir::tempPath()); + dir.mkdir("sortTemp"); + dir.cd("sortTemp"); + const QString dirPath = dir.absolutePath(); QVERIFY(dir.exists()); - dir.mkdir("a"); - dir.mkdir("b"); - dir.mkdir("c"); - dir.mkdir("d"); - dir.mkdir("e"); - dir.mkdir("f"); - dir.mkdir("g"); - QTemporaryFile tempFile(dirPath + "/rXXXXXX"); - tempFile.open(); + + //Create a file that will be at the end when sorting by name (For Mac, the default) + //but if we sort by size descending it will be the first + QFile tempFile(dirPath + "/plop2.txt"); + tempFile.open(QIODevice::WriteOnly | QIODevice::Text); + QTextStream out(&tempFile); + out << "The magic number is: " << 49 << "\n"; + tempFile.close(); + + QFile tempFile2(dirPath + "/plop.txt"); + tempFile2.open(QIODevice::WriteOnly | QIODevice::Text); + QTextStream out2(&tempFile2); + out2 << "The magic number is : " << 49 << " but i write some stuff in the file \n"; + tempFile2.close(); + myModel->setRootPath(QDir::rootPath()); + myModel->setFilter(QDir::AllEntries | QDir::System | QDir::Hidden); + tree->setSortingEnabled(true); tree->setModel(myModel); tree->show(); + tree->resize(800, 800); QTest::qWait(500); - tree->expand(myModel->index(dir.absolutePath(), 0)); - while (dir.cdUp()) + tree->header()->setSortIndicator(1,Qt::DescendingOrder); + tree->header()->setResizeMode(0, QHeaderView::ResizeToContents); + QStringList dirsToOpen; + do { - tree->expand(myModel->index(dir.absolutePath(), 0)); + dirsToOpen< 0 ; --i) { + QString path = dirsToOpen[i]; + QTest::qWait(500); + tree->expand(myModel->index(path, 0)); } - QTest::qWait(250); + tree->expand(myModel->index(dirPath, 0)); + QTest::qWait(500); + QModelIndex parent = myModel->index(dirPath, 0); //File dialog Mode means sub trees are not sorted, only the current root if (fileDialogMode) - QVERIFY(myModel->index(0, 1, myModel->index(dirPath, 0)).data(QFileSystemModel::FilePathRole).toString() != dirPath + QLatin1String("/a")); + QVERIFY(dirPath + QChar('/') + myModel->index(0, 1, parent).data(QFileSystemModel::FileNameRole).toString() != tempFile2.fileName()); else - QCOMPARE(myModel->index(0, 1, myModel->index(dirPath, 0)).data(QFileSystemModel::FilePathRole).toString(), dirPath + QLatin1String("/a")); + QCOMPARE(dirPath + QChar('/') + myModel->index(0, 1, parent).data(QFileSystemModel::FileNameRole).toString(), tempFile2.fileName()); delete tree; delete myModel; + dir.setPath(QDir::tempPath()); + dir.cd("sortTemp"); + tempFile.remove(); + tempFile2.remove(); + dir.cdUp(); + dir.rmdir("sortTemp"); + } void tst_QFileSystemModel::mkdir() -- cgit v0.12