From a6b4348ddbc8fdf64ddb2c454e7fa2179e82c61b Mon Sep 17 00:00:00 2001 From: Ilia Kats Date: Mon, 9 Dec 2024 11:31:44 +0000 Subject: [PATCH] Fix inline renaming multiple files when renaming changes sorting order Suppose we are renaming file i and the new name will be sorted after file i+1. We are now pressing ArrowDown to immediately start renaming file i+1. However, because of the sorting we would not actually end up renaming what used to be file i+1. What would happen is that editing would be started in dolphinview.cpp:2065. However, after 100 ms the timer in KFileItemModel would fire, resulting in the model emitting itemsMoved(). This would trigger doLayout() in KItemListView::slotItemsMoved(). doLayout() resizes the KItemListWidgets, wich causes the renaming to be canceled in KStandardItemListWidget::resizeEvent(). Now, we start a new renaming operation for the correct widget after the relayouting is complete. --- src/kitemviews/kitemlistview.cpp | 18 +++++++ src/kitemviews/kitemlistview.h | 2 + src/kitemviews/kstandarditemlistwidget.cpp | 11 +---- src/tests/dolphinmainwindowtest.cpp | 56 ++++++++++++++++++++++ 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp index d3caa5560..42c5e25c2 100644 --- a/src/kitemviews/kitemlistview.cpp +++ b/src/kitemviews/kitemlistview.cpp @@ -25,6 +25,8 @@ #include "private/kitemlistsizehintresolver.h" #include "private/kitemlistviewlayouter.h" +#include + #include #include #include @@ -743,6 +745,7 @@ void KItemListView::editRole(int index, const QByteArray &role) } m_editingRole = true; + m_controller->selectionManager()->setCurrentItem(index); widget->setEditedRole(role); connect(widget, &KItemListWidget::roleEditingCanceled, this, &KItemListView::slotRoleEditingCanceled); @@ -1376,9 +1379,20 @@ void KItemListView::slotItemsMoved(const KItemRange &itemRange, const QList const int firstVisibleMovedIndex = qMax(firstVisibleIndex(), itemRange.index); const int lastVisibleMovedIndex = qMin(lastVisibleIndex(), itemRange.index + itemRange.count - 1); + /// Represents an item that was moved while being edited. + struct MovedEditedItem { + int movedToIndex; + QByteArray editedRole; + }; + std::optional movedEditedItem; for (int index = firstVisibleMovedIndex; index <= lastVisibleMovedIndex; ++index) { KItemListWidget *widget = m_visibleItems.value(index); if (widget) { + if (m_editingRole && !widget->editedRole().isEmpty()) { + movedEditedItem = {movedToIndexes[index - itemRange.index], widget->editedRole()}; + disconnectRoleEditingSignals(index); + m_editingRole = false; + } updateWidgetProperties(widget, index); initializeItemListWidget(widget); } @@ -1386,6 +1400,10 @@ void KItemListView::slotItemsMoved(const KItemRange &itemRange, const QList doLayout(NoAnimation); updateSiblingsInformation(); + + if (movedEditedItem) { + editRole(movedEditedItem->movedToIndex, movedEditedItem->editedRole); + } } void KItemListView::slotItemsChanged(const KItemRangeList &itemRanges, const QSet &roles) diff --git a/src/kitemviews/kitemlistview.h b/src/kitemviews/kitemlistview.h index 7be08302c..30ce4d871 100644 --- a/src/kitemviews/kitemlistview.h +++ b/src/kitemviews/kitemlistview.h @@ -794,6 +794,8 @@ private: friend class KItemListControllerTest; friend class KItemListViewAccessible; friend class KItemListDelegateAccessible; + + friend class DolphinMainWindowTest; }; /** diff --git a/src/kitemviews/kstandarditemlistwidget.cpp b/src/kitemviews/kstandarditemlistwidget.cpp index a8fee6244..bc7023e12 100644 --- a/src/kitemviews/kstandarditemlistwidget.cpp +++ b/src/kitemviews/kstandarditemlistwidget.cpp @@ -845,16 +845,7 @@ void KStandardItemListWidget::editedRoleChanged(const QByteArray ¤t, const if (current.isEmpty() || !parent || current != "text") { if (m_roleEditor) { Q_EMIT roleEditingCanceled(index(), current, data().value(current)); - - disconnect(m_roleEditor, &KItemListRoleEditor::roleEditingCanceled, this, &KStandardItemListWidget::slotRoleEditingCanceled); - disconnect(m_roleEditor, &KItemListRoleEditor::roleEditingFinished, this, &KStandardItemListWidget::slotRoleEditingFinished); - - if (m_oldRoleEditor) { - m_oldRoleEditor->deleteLater(); - } - m_oldRoleEditor = m_roleEditor; - m_roleEditor->hide(); - m_roleEditor = nullptr; + closeRoleEditor(); } return; } diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp index 6ac85e4f0..f620cb6ba 100644 --- a/src/tests/dolphinmainwindowtest.cpp +++ b/src/tests/dolphinmainwindowtest.cpp @@ -13,7 +13,9 @@ #include "kitemviews/kitemlistcontainer.h" #include "kitemviews/kitemlistcontroller.h" #include "kitemviews/kitemlistselectionmanager.h" +#include "kitemviews/kitemlistwidget.h" #include "testdir.h" +#include "views/dolphinitemlistview.h" #include #include @@ -55,6 +57,7 @@ private Q_SLOTS: void testOpenFiles(); void testAccessibilityTree(); void testAutoSaveSession(); + void testInlineRename(); void cleanupTestCase(); private: @@ -881,6 +884,59 @@ void DolphinMainWindowTest::testAutoSaveSession() m_mainWindow->setSessionAutoSaveEnabled(false); } +void DolphinMainWindowTest::testInlineRename() +{ + QScopedPointer testDir{new TestDir()}; + testDir->createFiles({"aaaa", "bbbb", "cccc", "dddd"}); + m_mainWindow->openDirectories({testDir->url()}, false); + m_mainWindow->show(); + QVERIFY(QTest::qWaitForWindowExposed(m_mainWindow.data())); + QVERIFY(m_mainWindow->isVisible()); + + DolphinView *view = m_mainWindow->activeViewContainer()->view(); + QSignalSpy viewDirectoryLoadingCompletedSpy(view, &DolphinView::directoryLoadingCompleted); + QSignalSpy itemsReorderedSpy(view->m_model, &KFileItemModel::itemsMoved); + QSignalSpy modelDirectoryLoadingCompletedSpy(view->m_model, &KFileItemModel::directoryLoadingCompleted); + + QVERIFY(viewDirectoryLoadingCompletedSpy.wait()); + QTest::qWait(500); // we need to wait for the file widgets to become visible + view->markUrlsAsSelected({QUrl(testDir->url().toString() + "/aaaa")}); + view->updateViewState(); + view->renameSelectedItems(); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Left); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_E); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Down); + + QVERIFY(itemsReorderedSpy.wait()); + QVERIFY(view->m_view->m_editingRole); + KItemListWidget *widget = view->m_view->m_visibleItems.value(view->m_view->firstVisibleIndex()); + QVERIFY(!widget->editedRole().isEmpty()); + + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Left); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_A); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Down); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Down); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Left); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_A); + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Down); + + QVERIFY(itemsReorderedSpy.wait()); + QVERIFY(view->m_view->m_editingRole); + widget = view->m_view->m_visibleItems.value(view->m_view->lastVisibleIndex()); + QVERIFY(!widget->editedRole().isEmpty()); + + QTest::keyClick(QApplication::focusWidget(), Qt::Key_Escape); + QVERIFY(widget->isCurrent()); + view->m_model->refreshDirectory(testDir->url()); + QVERIFY(modelDirectoryLoadingCompletedSpy.wait()); + + QCOMPARE(view->m_model->fileItem(0).name(), "abbbb"); + QCOMPARE(view->m_model->fileItem(1).name(), "adddd"); + QCOMPARE(view->m_model->fileItem(2).name(), "cccc"); + QCOMPARE(view->m_model->fileItem(3).name(), "eaaaa"); + QCOMPARE(view->m_model->count(), 4); +} + void DolphinMainWindowTest::cleanupTestCase() { m_mainWindow->showNormal(); -- 2.47.3