From 4660ea3a48822a08bcfd8c518a168c344885bb65 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Wed, 30 Apr 2014 09:34:32 +0200 Subject: [PATCH 1/4] Update the "Paste" action only if it is necessary Updating this action is only required if the clipboard contents change, or if the "is writable" state of the current location changes. In all other cases, an update of this action should be prevented because it can be very expensive if many files are in the clipboard. In particular, the update when the selection changes could make changing the current item in the view very slow. BUG: 333903 REVIEW: 117782 FIXED-IN: 4.13.1 --- src/dolphinmainwindow.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dolphinmainwindow.cpp b/src/dolphinmainwindow.cpp index 28169cfb5..7117c09ba 100644 --- a/src/dolphinmainwindow.cpp +++ b/src/dolphinmainwindow.cpp @@ -217,6 +217,7 @@ DolphinMainWindow::DolphinMainWindow() : toggleSplitView(); } updateEditActions(); + updatePasteAction(); updateViewActions(); updateGoActions(); @@ -356,6 +357,7 @@ void DolphinMainWindow::changeUrl(const KUrl& url) if (view) { view->setUrl(url); updateEditActions(); + updatePasteAction(); updateViewActions(); updateGoActions(); setUrlAsCaption(url); @@ -1455,6 +1457,7 @@ void DolphinMainWindow::setActiveViewContainer(DolphinViewContainer* viewContain updateHistory(); updateEditActions(); + updatePasteAction(); updateViewActions(); updateGoActions(); @@ -1817,7 +1820,6 @@ void DolphinMainWindow::updateEditActions() deleteWithTrashShortcut->setEnabled(capabilities.supportsDeleting() && !enableMoveToTrash); cutAction->setEnabled(capabilities.supportsMoving()); } - updatePasteAction(); } void DolphinMainWindow::updateViewActions() From 24d0ade8e3badf665ed80eaa7d57d1c3099546c9 Mon Sep 17 00:00:00 2001 From: Uzair Shamim Date: Tue, 13 May 2014 18:56:28 +0200 Subject: [PATCH 2/4] Allow the widget in the "Additional Information" dialog to resize BUG: 334355 REVIEW: 118088 FIXED-IN: 4.13.2 --- src/settings/additionalinfodialog.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/settings/additionalinfodialog.cpp b/src/settings/additionalinfodialog.cpp index e9d5f606d..0de639540 100644 --- a/src/settings/additionalinfodialog.cpp +++ b/src/settings/additionalinfodialog.cpp @@ -75,7 +75,6 @@ AdditionalInfoDialog::AdditionalInfoDialog(QWidget* parent, QVBoxLayout* layout = new QVBoxLayout(mainWidget); layout->addWidget(header); layout->addWidget(m_listWidget); - layout->addStretch(1); setMainWidget(mainWidget); From 66f1759b6f993c237b9652b9aa51ac9df52e5238 Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Tue, 13 May 2014 19:04:09 +0200 Subject: [PATCH 3/4] Ensure that all children of a collapsed folder are removed Before this patch, any (direct or indirect) children that might have been in m_pendingItemsToInsert, i.e., that were not inserted into the model yet because KDirLister had not finished listing the directory yet, would be added to the model later without a proper parent. This could cause a crash later on. CCBUG: 332102 FIXED-IN: 4.13.2 --- src/kitemviews/kfileitemmodel.cpp | 12 +++++ src/tests/kfileitemmodeltest.cpp | 78 +++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index a0f9305cb..34a97de4c 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -486,6 +486,18 @@ bool KFileItemModel::setExpanded(int index, bool expanded) m_urlsToExpand.insert(url); } } else { + // Note that there might be (indirect) children of the folder which is to be collapsed in + // m_pendingItemsToInsert. To prevent that they will be inserted into the model later, + // possibly without a parent, which might result in a crash, we insert all pending items + // right now. All new items which would be without a parent will then be removed. + dispatchPendingItemsToInsert(); + + // Check if the index of the collapsed folder has changed. If that is the case, then items + // were inserted before the collapsed folder, and its index needs to be updated. + if (m_itemData.at(index)->item != item) { + index = this->index(item); + } + m_expandedDirs.remove(targetUrl); m_dirLister->stop(url); diff --git a/src/tests/kfileitemmodeltest.cpp b/src/tests/kfileitemmodeltest.cpp index 99ee3368e..48e72e83f 100644 --- a/src/tests/kfileitemmodeltest.cpp +++ b/src/tests/kfileitemmodeltest.cpp @@ -93,6 +93,7 @@ private slots: void testChangeRolesForFilteredItems(); void testChangeSortRoleWhileFiltering(); void testRefreshFilteredItems(); + void testCollapseFolderWhileLoading(); void testCreateMimeData(); private: @@ -1619,6 +1620,83 @@ void KFileItemModelTest::testCreateMimeData() delete mimeData; } +void KFileItemModelTest::testCollapseFolderWhileLoading() +{ + QSet modelRoles = m_model->roles(); + modelRoles << "isExpanded" << "isExpandable" << "expandedParentsCount"; + m_model->setRoles(modelRoles); + + QStringList files; + files << "a2/b/c1.txt"; + m_testDir->createFiles(files); + + m_model->loadDirectory(m_testDir->url()); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "a2"); + + // Expand "a2/". + m_model->setExpanded(0, true); + QVERIFY(m_model->isExpanded(0)); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "a2" << "b"); + + // Expand "a2/b/". + m_model->setExpanded(1, true); + QVERIFY(m_model->isExpanded(1)); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "a2" << "b" << "c1.txt"); + + // Simulate that a new item "c2.txt" appears, but that the dir lister's completed() + // signal is not emitted yet. + const KFileItem fileItemC1 = m_model->fileItem(2); + KFileItem fileItemC2 = fileItemC1; + KUrl urlC2 = fileItemC2.url(); + urlC2.setFileName("c2.txt"); + fileItemC2.setUrl(urlC2); + + const KUrl urlB = m_model->fileItem(1).url(); + m_model->slotItemsAdded(urlB, KFileItemList() << fileItemC2); + QCOMPARE(itemsInModel(), QStringList() << "a2" << "b" << "c1.txt"); + + // Collapse "a2/". This should also remove all its (indirect) children from + // the model and from the model's m_pendingItemsToInsert member. + m_model->setExpanded(0, false); + QCOMPARE(itemsInModel(), QStringList() << "a2"); + + // Simulate that the dir lister's completed() signal is emitted. If "c2.txt" + // is still in m_pendingItemsToInsert, then we might get a crash, see + // https://bugs.kde.org/show_bug.cgi?id=332102. Even if the crash is not + // reproducible here, Valgrind will complain, and the item "c2.txt" will appear + // without parent in the model. + m_model->slotCompleted(); + QCOMPARE(itemsInModel(), QStringList() << "a2"); + + // Expand "a2/" again. + m_model->setExpanded(0, true); + QVERIFY(m_model->isExpanded(0)); + QVERIFY(QTest::kWaitForSignal(m_model, SIGNAL(itemsInserted(KItemRangeList)), DefaultTimeout)); + QCOMPARE(itemsInModel(), QStringList() << "a2" << "b"); + + // Now simulate that a new folder "a1/" is appears, but that the dir lister's + // completed() signal is not emitted yet. + const KFileItem fileItemA2 = m_model->fileItem(0); + KFileItem fileItemA1 = fileItemA2; + KUrl urlA1 = fileItemA1.url(); + urlA1.setFileName("a1"); + fileItemA1.setUrl(urlA1); + + m_model->slotItemsAdded(m_model->directory(), KFileItemList() << fileItemA1); + QCOMPARE(itemsInModel(), QStringList() << "a2" << "b"); + + // Collapse "a2/". Note that this will cause "a1/" to be added to the model, + // i.e., the index of "a2/" will change from 0 to 1. Check that this does not + // confuse the code which collapses the folder. + m_model->setExpanded(0, false); + QCOMPARE(itemsInModel(), QStringList() << "a1" << "a2"); + QVERIFY(!m_model->isExpanded(0)); + QVERIFY(!m_model->isExpanded(1)); +} + QStringList KFileItemModelTest::itemsInModel() const { QStringList items; From 99e8f8e2e6b6982d1a58185d0980be203d6061ec Mon Sep 17 00:00:00 2001 From: Frank Reininghaus Date: Tue, 13 May 2014 19:06:42 +0200 Subject: [PATCH 4/4] Stop listing children of collapsed folders Before this patch, KDirLister would continue listing any children of collapsed folders, even though the children themselves were removed from the model. This could lead to new items being inserted as top-level items at some later point, because no parent could be found for them. This inconsistent model state could lead to a crash later on. Many thanks to Martin Koller for helping to debug this problem! BUG: 332102 REVIEW: 118055 FIXED-IN: 4.13.2 --- src/kitemviews/kfileitemmodel.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp index 34a97de4c..de3c3eb22 100644 --- a/src/kitemviews/kfileitemmodel.cpp +++ b/src/kitemviews/kfileitemmodel.cpp @@ -512,7 +512,9 @@ bool KFileItemModel::setExpanded(int index, bool expanded) ItemData* itemData = m_itemData.at(childIndex); if (itemData->values.value("isExpanded").toBool()) { const KUrl targetUrl = itemData->item.targetUrl(); + const KUrl url = itemData->item.url(); m_expandedDirs.remove(targetUrl); + m_dirLister->stop(url); // TODO: try to unit-test this, see https://bugs.kde.org/show_bug.cgi?id=332102#c11 expandedChildren.append(targetUrl); } ++childIndex;