include/svl/undo.hxx | 2 svl/source/undo/undo.cxx | 4 - sw/qa/extras/tiledrendering/tiledrendering.cxx | 56 +++++++++++++++++++++++++ sw/source/core/undo/docundo.cxx | 21 ++++++++- 4 files changed, 77 insertions(+), 6 deletions(-)
New commits: commit 60665dc4a2af238939b1a5056ae4a4ce2c083159 Author: Miklos Vajna <[email protected]> AuthorDate: Thu Nov 11 08:41:50 2021 +0100 Commit: Mike Kaganski <[email protected]> CommitDate: Thu Nov 11 18:04:18 2021 +0100 sw, out of order undo: allow a subset of a non-empty redo list Specifically, we used to not allow out of order undo at all if the redo list was non-empty. This relaxes that condition a bit. Out of order undo is OK with a non-empty redo list, in case all undo actions in the redo list are either 1) owned by the current view or 2) independent from the undo action to be executed I.e. if view1 has lots of type undo actions and an view2 adds a single type undo action on top of it, then allow view 1 to execute multiple of its typing undo actions, not just a single one. (cherry picked from commit 2875c65946e59f5dd7968155463bf00bd71d440b) Conflicts: sw/source/core/undo/docundo.cxx Change-Id: I2f5d9404a9994ed74b65233d2a315976c27b28b2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125039 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Mike Kaganski <[email protected]> diff --git a/include/svl/undo.hxx b/include/svl/undo.hxx index e0d064b27987..eff352298a5c 100644 --- a/include/svl/undo.hxx +++ b/include/svl/undo.hxx @@ -206,7 +206,7 @@ public: OUString GetUndoActionsInfo() const; virtual size_t GetRedoActionCount( bool const i_currentLevel = CurrentLevel ) const; OUString GetRedoActionComment( size_t nNo=0, bool const i_currentLevel = CurrentLevel ) const; - SfxUndoAction* GetRedoAction() const; + SfxUndoAction* GetRedoAction(size_t nNo = 0) const; /// Get info about all redo actions (comment, view shell id, etc.) OUString GetRedoActionsInfo() const; virtual bool Undo(); diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx index 5889d59a064b..5dca45082fce 100644 --- a/svl/source/undo/undo.cxx +++ b/svl/source/undo/undo.cxx @@ -752,7 +752,7 @@ size_t SfxUndoManager::ImplGetRedoActionCount_Lock( bool const i_currentLevel ) } -SfxUndoAction* SfxUndoManager::GetRedoAction() const +SfxUndoAction* SfxUndoManager::GetRedoAction(size_t nNo) const { UndoManagerGuard aGuard( *m_xData ); @@ -761,7 +761,7 @@ SfxUndoAction* SfxUndoManager::GetRedoAction() const { return nullptr; } - return pUndoArray->maUndoActions[ pUndoArray->nCurUndoAction ].pAction.get(); + return pUndoArray->maUndoActions[pUndoArray->nCurUndoAction + nNo].pAction.get(); } diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx b/sw/qa/extras/tiledrendering/tiledrendering.cxx index d596f7ea7fb7..9b64b64c78e8 100644 --- a/sw/qa/extras/tiledrendering/tiledrendering.cxx +++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx @@ -108,6 +108,7 @@ public: void testUndoInvalidations(); void testUndoLimiting(); void testUndoReordering(); + void testUndoReorderingRedo(); void testUndoShapeLimiting(); void testUndoDispatch(); void testUndoRepairDispatch(); @@ -189,6 +190,7 @@ public: CPPUNIT_TEST(testUndoInvalidations); CPPUNIT_TEST(testUndoLimiting); CPPUNIT_TEST(testUndoReordering); + CPPUNIT_TEST(testUndoReorderingRedo); CPPUNIT_TEST(testUndoShapeLimiting); CPPUNIT_TEST(testUndoDispatch); CPPUNIT_TEST(testUndoRepairDispatch); @@ -1346,6 +1348,60 @@ void SwTiledRenderingTest::testUndoReordering() SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr); } +void SwTiledRenderingTest::testUndoReorderingRedo() +{ + // Create two views and a document of 2 paragraphs. + SwXTextDocument* pXTextDocument = createDoc(); + SwWrtShell* pWrtShell1 = pXTextDocument->GetDocShell()->GetWrtShell(); + int nView1 = SfxLokHelper::getView(); + int nView2 = SfxLokHelper::createView(); + pXTextDocument->initializeForTiledRendering(uno::Sequence<beans::PropertyValue>()); + SwWrtShell* pWrtShell2 = pXTextDocument->GetDocShell()->GetWrtShell(); + pWrtShell2->SplitNode(); + SfxLokHelper::setView(nView1); + pWrtShell1->SttEndDoc(/*bStt=*/true); + SwTextNode* pTextNode1 = pWrtShell1->GetCursor()->GetNode().GetTextNode(); + // View 1 types into the first paragraph, twice. + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'f', 0); + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'f', 0); + Scheduler::ProcessEventsToIdle(); + // Go to the start of the paragraph, to avoid grouping. + pWrtShell1->SttEndDoc(/*bStt=*/true); + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 's', 0); + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 's', 0); + Scheduler::ProcessEventsToIdle(); + SfxLokHelper::setView(nView2); + pWrtShell2->SttEndDoc(/*bStt=*/false); + SwTextNode* pTextNode2 = pWrtShell2->GetCursor()->GetNode().GetTextNode(); + // View 2 types into the second paragraph. + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'z', 0); + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'z', 0); + Scheduler::ProcessEventsToIdle(); + CPPUNIT_ASSERT_EQUAL(OUString("sf"), pTextNode1->GetText()); + CPPUNIT_ASSERT_EQUAL(OUString("z"), pTextNode2->GetText()); + + // When view 1 presses undo, twice: + SfxLokHelper::setView(nView1); + dispatchCommand(mxComponent, ".uno:Undo", {}); + Scheduler::ProcessEventsToIdle(); + // First just s(econd) is erased: + CPPUNIT_ASSERT_EQUAL(OUString("f"), pTextNode1->GetText()); + dispatchCommand(mxComponent, ".uno:Undo", {}); + Scheduler::ProcessEventsToIdle(); + + // Then make sure view 1's undo actions are invoked, out of order: + // Without the accompanying fix in place, this test would have failed with: + // - Expression: pTextNode1->GetText().isEmpty() + // i.e. out of order undo was executed only once, not twice. + CPPUNIT_ASSERT(pTextNode1->GetText().isEmpty()); + // The top undo action is not invoked, as it belongs to view 2. + CPPUNIT_ASSERT_EQUAL(OUString("z"), pTextNode2->GetText()); + SfxLokHelper::setView(nView1); + SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr); + SfxLokHelper::setView(nView2); + SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr); +} + void SwTiledRenderingTest::testUndoShapeLimiting() { // Load a document and create a view. diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx index 8c46f659cf06..16266e6a4f85 100644 --- a/sw/source/core/undo/docundo.cxx +++ b/sw/source/core/undo/docundo.cxx @@ -359,10 +359,9 @@ UndoManager::EndUndo(SwUndoId eUndoId, SwRewriter const*const pRewriter) */ bool UndoManager::IsViewUndoActionIndependent(const SwView* pView) const { - if (GetUndoActionCount() <= 1 || SdrUndoManager::GetRedoActionCount() > 0) + if (GetUndoActionCount() <= 1) { - // Single or less undo, owned by an other view; or redo actions that might depend on the - // current undo order. + // Single or less undo, owned by another view. return false; } @@ -405,6 +404,22 @@ bool UndoManager::IsViewUndoActionIndependent(const SwView* pView) const const auto& rTopInsert = *static_cast<const SwUndoInsert*>(pTopSwAction); const auto& rViewInsert = *static_cast<const SwUndoInsert*>(pViewSwAction); + for (size_t i = 0; i < GetRedoActionCount(); ++i) + { + auto pRedoAction = dynamic_cast<const SwUndo*>(GetRedoAction(i)); + if (!pRedoAction || pViewSwAction->GetId() != SwUndoId::TYPING) + { + return false; + } + + const auto& rRedoInsert = *static_cast<const SwUndoInsert*>(pRedoAction); + if (!rViewInsert.IsIndependent(rRedoInsert) && rRedoInsert.GetViewShellId() != nViewId) + { + // Dependent redo action and owned by an other view. + return false; + } + } + return rViewInsert.IsIndependent(rTopInsert); }
