diff --git a/include/svl/undo.hxx b/include/svl/undo.hxx index 2757967aaee4..e0d064b27987 100644 --- a/include/svl/undo.hxx +++ b/include/svl/undo.hxx @@ -42,6 +42,12 @@ public: class SVL_DLLPUBLIC SfxUndoContext { public: + /** + * Don't undo the top undo action, but an earlier one. It's the caller's responsibility to + * ensure that the earlier undo action is independent from the following ones. + */ + virtual size_t GetUndoOffset() { return 0; } + virtual ~SfxUndoContext() = 0; }; diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx index 630bb9deea5b..0c081b152c3a 100644 --- a/svl/source/undo/undo.cxx +++ b/svl/source/undo/undo.cxx @@ -683,6 +683,16 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext* i_contextOrNull ) return false; } + if (i_contextOrNull && i_contextOrNull->GetUndoOffset() == 1) + { + if (m_xData->pActUndoArray->nCurUndoAction >= 2) + { + std::swap( + m_xData->pActUndoArray->maUndoActions[m_xData->pActUndoArray->nCurUndoAction - 1], + m_xData->pActUndoArray->maUndoActions[m_xData->pActUndoArray->nCurUndoAction - 2]); + } + } + SfxUndoAction* pAction = m_xData->pActUndoArray->maUndoActions[ --m_xData->pActUndoArray->nCurUndoAction ].pAction.get(); const OUString sActionComment = pAction->GetComment(); try diff --git a/sw/inc/IDocumentUndoRedo.hxx b/sw/inc/IDocumentUndoRedo.hxx index b533a7e647c2..dae73a333360 100644 --- a/sw/inc/IDocumentUndoRedo.hxx +++ b/sw/inc/IDocumentUndoRedo.hxx @@ -212,6 +212,9 @@ public: */ virtual void SetView(SwView* pView) = 0; + /// Zero offset means undoing the top undo action. + virtual bool UndoWithOffset(size_t nUndoOffset) = 0; + protected: virtual ~IDocumentUndoRedo() {}; }; diff --git a/sw/inc/editsh.hxx b/sw/inc/editsh.hxx index f3f645580135..076cbbf5b3bb 100644 --- a/sw/inc/editsh.hxx +++ b/sw/inc/editsh.hxx @@ -602,7 +602,7 @@ public: /// should only be called by sw::UndoManager! void HandleUndoRedoContext(::sw::UndoRedoContext & rContext); - void Undo(sal_uInt16 const nCount = 1); + void Undo(sal_uInt16 const nCount = 1, sal_uInt16 nOffset = 0); void Redo(sal_uInt16 const nCount = 1); void Repeat(sal_uInt16 const nCount); diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx b/sw/qa/extras/tiledrendering/tiledrendering.cxx index 4ad4159a295c..46f1c781be02 100644 --- a/sw/qa/extras/tiledrendering/tiledrendering.cxx +++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx @@ -108,6 +108,7 @@ public: void testTextEditViewInvalidations(); void testUndoInvalidations(); void testUndoLimiting(); + void testUndoReordering(); void testUndoShapeLimiting(); void testUndoDispatch(); void testUndoRepairDispatch(); @@ -189,6 +190,7 @@ public: CPPUNIT_TEST(testTextEditViewInvalidations); CPPUNIT_TEST(testUndoInvalidations); CPPUNIT_TEST(testUndoLimiting); + CPPUNIT_TEST(testUndoReordering); CPPUNIT_TEST(testUndoShapeLimiting); CPPUNIT_TEST(testUndoDispatch); CPPUNIT_TEST(testUndoRepairDispatch); @@ -1305,6 +1307,51 @@ void SwTiledRenderingTest::testUndoLimiting() SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr); } +void SwTiledRenderingTest::testUndoReordering() +{ + // 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()); + 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. + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'a', 0); + pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'a', 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("a"), pTextNode1->GetText()); + CPPUNIT_ASSERT_EQUAL(OUString("z"), pTextNode2->GetText()); + + // When view 1 presses undo: + SfxLokHelper::setView(nView1); + dispatchCommand(mxComponent, ".uno:Undo", {}); + Scheduler::ProcessEventsToIdle(); + + // Then make sure view 1's last undo action is invoked, out of order: + // Without the accompanying fix in place, this test would have failed with: + // - Expression: pTextNode1->GetText().isEmpty() + // i.e. the "a" in the first paragraph was not removed. + CPPUNIT_ASSERT(pTextNode1->GetText().isEmpty()); + // Last 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/edit/edundo.cxx b/sw/source/core/edit/edundo.cxx index bfadd01aaffd..826d3154ff78 100644 --- a/sw/source/core/edit/edundo.cxx +++ b/sw/source/core/edit/edundo.cxx @@ -96,7 +96,7 @@ void SwEditShell::HandleUndoRedoContext(::sw::UndoRedoContext & rContext) } } -void SwEditShell::Undo(sal_uInt16 const nCount) +void SwEditShell::Undo(sal_uInt16 const nCount, sal_uInt16 nOffset) { MakeAllOutlineContentTemporarilyVisible a(GetDoc()); @@ -132,8 +132,7 @@ void SwEditShell::Undo(sal_uInt16 const nCount) try { for (sal_uInt16 i = 0; i < nCount; ++i) { - bRet = GetDoc()->GetIDocumentUndoRedo().Undo() - || bRet; + bRet = GetDoc()->GetIDocumentUndoRedo().UndoWithOffset(nOffset) || bRet; } } catch (const css::uno::Exception &) { TOOLS_WARN_EXCEPTION("sw.core", "SwEditShell::Undo()"); diff --git a/sw/source/core/inc/UndoCore.hxx b/sw/source/core/inc/UndoCore.hxx index bb60ddeedb3e..af5a6b7f04da 100644 --- a/sw/source/core/inc/UndoCore.hxx +++ b/sw/source/core/inc/UndoCore.hxx @@ -107,11 +107,16 @@ public: o_rpMarkList = m_pMarkList; } + void SetUndoOffset(size_t nUndoOffset) { m_nUndoOffset = nUndoOffset; } + + size_t GetUndoOffset() override { return m_nUndoOffset; } + private: SwDoc & m_rDoc; IShellCursorSupplier & m_rCursorSupplier; SwFrameFormat * m_pSelFormat; SdrMarkList * m_pMarkList; + size_t m_nUndoOffset = 0; }; class RepeatContext final diff --git a/sw/source/core/inc/UndoInsert.hxx b/sw/source/core/inc/UndoInsert.hxx index 8caaf00a3e3e..ae7b6c02dbd5 100644 --- a/sw/source/core/inc/UndoInsert.hxx +++ b/sw/source/core/inc/UndoInsert.hxx @@ -87,6 +87,8 @@ public: virtual SwRewriter GetRewriter() const override; void SetWithRsid() { m_bWithRsid = true; } + + bool IsIndependent(const SwUndoInsert& rOther) const; }; SwRewriter diff --git a/sw/source/core/inc/UndoManager.hxx b/sw/source/core/inc/UndoManager.hxx index 4690fd171492..b8b130875d90 100644 --- a/sw/source/core/inc/UndoManager.hxx +++ b/sw/source/core/inc/UndoManager.hxx @@ -80,6 +80,7 @@ public: virtual size_t GetUndoActionCount(const bool bCurrentLevel = true) const override; size_t GetRedoActionCount(const bool bCurrentLevel = true) const override; void SetView(SwView* pView) override; + bool UndoWithOffset(size_t nUndoOffset) override; // SfxUndoManager virtual void AddUndoAction(std::unique_ptr pAction, @@ -95,6 +96,12 @@ public: SwNodes & GetUndoNodes(); void SetDocShell(SwDocShell* pDocShell); + /** + * Checks if the topmost undo action owned by pView is independent from the topmost action undo + * action. + */ + bool IsViewUndoActionIndependent(const SwView* pView) const; + private: virtual void EmptyActionsChanged() override; @@ -118,7 +125,7 @@ private: SwView* m_pView; enum class UndoOrRedoType { Undo, Redo }; - bool impl_DoUndoRedo(UndoOrRedoType undoOrRedo); + bool impl_DoUndoRedo(UndoOrRedoType undoOrRedo, size_t nUndoOffset); // UGLY: should not be called using SdrUndoManager::Repeat; diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx index 3a5b459df880..20c6c6c46c95 100644 --- a/sw/source/core/undo/docundo.cxx +++ b/sw/source/core/undo/docundo.cxx @@ -41,6 +41,8 @@ #include #include +#include + using namespace ::com::sun::star; // the undo array should never grow beyond this limit: @@ -352,6 +354,61 @@ UndoManager::EndUndo(SwUndoId eUndoId, SwRewriter const*const pRewriter) return eUndoId; } +/** + * Checks if the topmost undo action owned by pView is independent from the topmost action undo + * action. + */ +bool UndoManager::IsViewUndoActionIndependent(const SwView* pView) const +{ + if (GetUndoActionCount() <= 1 || SdrUndoManager::GetRedoActionCount() > 0) + { + // Single or less undo, owned by an other view; or redo actions that might depend on the + // current undo order. + return false; + } + + if (!pView) + { + return false; + } + + // Last undo action that doesn't belong to the view. + const SfxUndoAction* pTopAction = GetUndoAction(); + + ViewShellId nViewId = pView->GetViewShellId(); + + // Earlier undo action that belongs to the view, but is not the top one. + const SfxUndoAction* pViewAction = nullptr; + const SfxUndoAction* pAction = GetUndoAction(1); + if (pAction->GetViewShellId() == nViewId) + { + pViewAction = pAction; + } + + if (!pViewAction) + { + // Found no earlier undo action that belongs to the view. + return false; + } + + auto pTopSwAction = dynamic_cast(pTopAction); + if (!pTopSwAction || pTopSwAction->GetId() != SwUndoId::TYPING) + { + return false; + } + + auto pViewSwAction = dynamic_cast(pViewAction); + if (!pViewSwAction || pViewSwAction->GetId() != SwUndoId::TYPING) + { + return false; + } + + const auto& rTopInsert = *static_cast(pTopSwAction); + const auto& rViewInsert = *static_cast(pViewSwAction); + + return rViewInsert.IsIndependent(rTopInsert); +} + bool UndoManager::GetLastUndoInfo( OUString *const o_pStr, SwUndoId *const o_pId, const SwView* pView) const @@ -369,7 +426,8 @@ UndoManager::GetLastUndoInfo( { // If another view created the undo action, prevent undoing it from this view. ViewShellId nViewShellId = pView ? pView->GetViewShellId() : m_pDocShell->GetView()->GetViewShellId(); - if (pAction->GetViewShellId() != nViewShellId) + // Unless we know that the other view's undo action is independent from us. + if (pAction->GetViewShellId() != nViewShellId && !IsViewUndoActionIndependent(pView)) { if (o_pId) { @@ -572,7 +630,7 @@ private: } -bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo) +bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo, size_t nUndoOffset) { SwDoc & rDoc(GetUndoNodes().GetDoc()); @@ -601,6 +659,7 @@ bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo) bool bRet(false); ::sw::UndoRedoContext context(rDoc, *pEditShell); + context.SetUndoOffset(nUndoOffset); // N.B. these may throw! if (UndoOrRedoType::Undo == undoOrRedo) @@ -630,7 +689,9 @@ bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo) return bRet; } -bool UndoManager::Undo() +bool UndoManager::Undo() { return UndoWithOffset(0); } + +bool UndoManager::UndoWithOffset(size_t nUndoOffset) { if(isTextEditActive()) { @@ -638,7 +699,7 @@ bool UndoManager::Undo() } else { - return impl_DoUndoRedo(UndoOrRedoType::Undo); + return impl_DoUndoRedo(UndoOrRedoType::Undo, nUndoOffset); } } @@ -650,7 +711,7 @@ bool UndoManager::Redo() } else { - return impl_DoUndoRedo(UndoOrRedoType::Redo); + return impl_DoUndoRedo(UndoOrRedoType::Redo, /*nUndoOffset=*/0); } } diff --git a/sw/source/core/undo/unins.cxx b/sw/source/core/undo/unins.cxx index 44800d1c132a..c18f6406e20d 100644 --- a/sw/source/core/undo/unins.cxx +++ b/sw/source/core/undo/unins.cxx @@ -476,6 +476,11 @@ SwRewriter SwUndoInsert::GetRewriter() const return aResult; } +bool SwUndoInsert::IsIndependent(const SwUndoInsert& rOther) const +{ + return m_nNode != rOther.m_nNode; +} + class SwUndoReplace::Impl : private SwUndoSaveContent { diff --git a/sw/source/uibase/inc/wrtsh.hxx b/sw/source/uibase/inc/wrtsh.hxx index 45b3451cf560..c86de9ef55de 100644 --- a/sw/source/uibase/inc/wrtsh.hxx +++ b/sw/source/uibase/inc/wrtsh.hxx @@ -374,7 +374,7 @@ typedef bool (SwWrtShell::*FNSimpleMove)(); enum class FieldDialogPressedButton { NONE, Previous, Next }; - void Do( DoType eDoType, sal_uInt16 nCnt = 1 ); + void Do(DoType eDoType, sal_uInt16 nCnt = 1, sal_uInt16 nOffset = 0); OUString GetDoString( DoType eDoType ) const; OUString GetRepeatString() const; void GetDoStrings( DoType eDoType, SfxStringListItem& rStrLstItem ) const; diff --git a/sw/source/uibase/shells/basesh.cxx b/sw/source/uibase/shells/basesh.cxx index dd47986401f8..f492123f9b47 100644 --- a/sw/source/uibase/shells/basesh.cxx +++ b/sw/source/uibase/shells/basesh.cxx @@ -101,6 +101,7 @@ #include #include +#include FlyMode SwBaseShell::eFrameMode = FLY_DRAG_END; @@ -618,7 +619,25 @@ void SwBaseShell::ExecUndo(SfxRequest &rReq) { for (SwViewShell& rShell : rWrtShell.GetRingContainer()) rShell.LockPaint(); - rWrtShell.Do( SwWrtShell::UNDO, nCnt ); + + sal_uInt16 nUndoOffset = 0; + if (comphelper::LibreOfficeKit::isActive() && !bRepair && nCnt == 1) + { + sw::UndoManager& rManager = rWrtShell.GetDoc()->GetUndoManager(); + const SfxUndoAction* pAction = rManager.GetUndoAction(); + SwView& rView = rWrtShell.GetView(); + ViewShellId nViewShellId = rView.GetViewShellId(); + if (pAction->GetViewShellId() != nViewShellId + && rManager.IsViewUndoActionIndependent(&rView)) + { + // Execute the undo with an offset: don't undo the top action, but an + // earlier one, since it's independent and that belongs to our view. + nUndoOffset = 1; + } + } + + rWrtShell.Do(SwWrtShell::UNDO, nCnt, nUndoOffset); + for (SwViewShell& rShell : rWrtShell.GetRingContainer()) rShell.UnlockPaint(); diff --git a/sw/source/uibase/wrtsh/wrtundo.cxx b/sw/source/uibase/wrtsh/wrtundo.cxx index 82bb28109c60..770c59367a37 100644 --- a/sw/source/uibase/wrtsh/wrtundo.cxx +++ b/sw/source/uibase/wrtsh/wrtundo.cxx @@ -30,7 +30,7 @@ // Undo ends all modes. If a selection is emerged by the Undo, // this must be considered for further action. -void SwWrtShell::Do( DoType eDoType, sal_uInt16 nCnt ) +void SwWrtShell::Do(DoType eDoType, sal_uInt16 nCnt, sal_uInt16 nOffset) { // #105332# save current state of DoesUndo() bool bSaveDoesUndo = DoesUndo(); @@ -42,7 +42,7 @@ void SwWrtShell::Do( DoType eDoType, sal_uInt16 nCnt ) DoUndo(false); // #i21739# // Reset modes EnterStdMode(); - SwEditShell::Undo(nCnt); + SwEditShell::Undo(nCnt, nOffset); break; case REDO: DoUndo(false); // #i21739#