svl/source/undo/undo.cxx | 18 ++++++--
sw/qa/extras/tiledrendering/tiledrendering.cxx | 52 +++++++++++++++++++++++++
sw/source/core/inc/UndoManager.hxx | 4 -
sw/source/core/undo/docundo.cxx | 26 +++++++++---
sw/source/uibase/shells/basesh.cxx | 5 +-
5 files changed, 90 insertions(+), 15 deletions(-)
New commits:
commit 39f231360013e944a8713248359662b9f282d902
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Fri Nov 12 08:39:35 2021 +0100
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Nov 12 12:46:22 2021 +0100
sw, out of order undo: allow multiple actions from other views
Previously we assumed that the action to be executed is always exactly
the top of the undo stack minus 1 element.
Extend this, so that in case an other view appends two or more elements
to the undo stack, we still find our undo action. Obviously only do this
if all those undo actions are independent from us.
This requires replacing the swap in svl/ with a move-out + move a range
down + move in construct.
Change-Id: Ic12d32d6eb5e77618d99eddb4fa096802f32d655
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125076
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
Tested-by: Jenkins
diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx
index f84d2cda185a..e820a3cbb9d1 100644
--- a/svl/source/undo/undo.cxx
+++ b/svl/source/undo/undo.cxx
@@ -683,13 +683,21 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext*
i_contextOrNull )
return false;
}
- if (i_contextOrNull && i_contextOrNull->GetUndoOffset() == 1)
+ if (i_contextOrNull && i_contextOrNull->GetUndoOffset() > 0)
{
- if (m_xData->pActUndoArray->nCurUndoAction >= 2)
+ size_t nCurrent = m_xData->pActUndoArray->nCurUndoAction;
+ size_t nOffset = i_contextOrNull->GetUndoOffset();
+ if (nCurrent >= nOffset + 1)
{
- std::swap(
-
m_xData->pActUndoArray->maUndoActions[m_xData->pActUndoArray->nCurUndoAction -
1],
-
m_xData->pActUndoArray->maUndoActions[m_xData->pActUndoArray->nCurUndoAction -
2]);
+ // Move out the action we want to execute.
+ MarkedUndoAction aAction
+ = std::move(m_xData->pActUndoArray->maUndoActions[nCurrent - 1
- nOffset]);
+ // Move the actions after aAction down by one.
+ std::move(m_xData->pActUndoArray->maUndoActions.data() + nCurrent
- nOffset,
+ m_xData->pActUndoArray->maUndoActions.data() + nCurrent,
+ m_xData->pActUndoArray->maUndoActions.data() + nCurrent
- nOffset - 1);
+ // Move aAction to the top.
+ m_xData->pActUndoArray->maUndoActions[nCurrent - 1] =
std::move(aAction);
}
}
diff --git a/sw/qa/extras/tiledrendering/tiledrendering.cxx
b/sw/qa/extras/tiledrendering/tiledrendering.cxx
index 4f0c7e3c5c7b..46a4eab3ac98 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx
@@ -110,6 +110,7 @@ public:
void testUndoLimiting();
void testUndoReordering();
void testUndoReorderingRedo();
+ void testUndoReorderingMulti();
void testUndoShapeLimiting();
void testUndoDispatch();
void testUndoRepairDispatch();
@@ -193,6 +194,7 @@ public:
CPPUNIT_TEST(testUndoLimiting);
CPPUNIT_TEST(testUndoReordering);
CPPUNIT_TEST(testUndoReorderingRedo);
+ CPPUNIT_TEST(testUndoReorderingMulti);
CPPUNIT_TEST(testUndoShapeLimiting);
CPPUNIT_TEST(testUndoDispatch);
CPPUNIT_TEST(testUndoRepairDispatch);
@@ -1408,6 +1410,56 @@ void SwTiledRenderingTest::testUndoReorderingRedo()
SfxViewShell::Current()->setLibreOfficeKitViewCallback(nullptr);
}
+void SwTiledRenderingTest::testUndoReorderingMulti()
+{
+ // 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.
+ 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, twice.
+ pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'x', 0);
+ pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'x', 0);
+ Scheduler::ProcessEventsToIdle();
+ // Go to the start of the paragraph, to avoid grouping.
+ pWrtShell2->SttPara();
+ pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYINPUT, 'y', 0);
+ pXTextDocument->postKeyEvent(LOK_KEYEVENT_KEYUP, 'y', 0);
+ Scheduler::ProcessEventsToIdle();
+ CPPUNIT_ASSERT_EQUAL(OUString("a"), pTextNode1->GetText());
+ CPPUNIT_ASSERT_EQUAL(OUString("yx"), pTextNode2->GetText());
+
+ // When view 1 presses undo:
+ SfxLokHelper::setView(nView1);
+ dispatchCommand(mxComponent, ".uno:Undo", {});
+ Scheduler::ProcessEventsToIdle();
+
+ // Then make sure view 1's 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. out of order undo was not executed, the first paragrph was still
"a".
+ CPPUNIT_ASSERT(pTextNode1->GetText().isEmpty());
+ // The top 2 undo actions are not invoked, as they belong to view 2.
+ CPPUNIT_ASSERT_EQUAL(OUString("yx"), 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/inc/UndoManager.hxx
b/sw/source/core/inc/UndoManager.hxx
index b8b130875d90..2db896f36db9 100644
--- a/sw/source/core/inc/UndoManager.hxx
+++ b/sw/source/core/inc/UndoManager.hxx
@@ -98,9 +98,9 @@ public:
/**
* Checks if the topmost undo action owned by pView is independent from
the topmost action undo
- * action.
+ * action. Sets rOffset to the offset of that independent undo action on
success.
*/
- bool IsViewUndoActionIndependent(const SwView* pView) const;
+ bool IsViewUndoActionIndependent(const SwView* pView, sal_uInt16& rOffset)
const;
private:
virtual void EmptyActionsChanged() override;
diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx
index 1b8ae19b3a65..07998e9404ef 100644
--- a/sw/source/core/undo/docundo.cxx
+++ b/sw/source/core/undo/docundo.cxx
@@ -40,6 +40,7 @@
#include <sfx2/viewfrm.hxx>
#include <sfx2/bindings.hxx>
#include <osl/diagnose.h>
+#include <o3tl/temporary.hxx>
#include <UndoInsert.hxx>
@@ -358,7 +359,7 @@ UndoManager::EndUndo(SwUndoId eUndoId, SwRewriter
const*const pRewriter)
* Checks if the topmost undo action owned by pView is independent from the
topmost action undo
* action.
*/
-bool UndoManager::IsViewUndoActionIndependent(const SwView* pView) const
+bool UndoManager::IsViewUndoActionIndependent(const SwView* pView, sal_uInt16&
rOffset) const
{
if (GetUndoActionCount() <= 1)
{
@@ -378,10 +379,16 @@ bool UndoManager::IsViewUndoActionIndependent(const
SwView* pView) const
// 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)
+ size_t nOffset = 0;
+ for (size_t i = 0; i < GetUndoActionCount(); ++i)
{
- pViewAction = pAction;
+ const SfxUndoAction* pAction = GetUndoAction(i);
+ if (pAction->GetViewShellId() == nViewId)
+ {
+ pViewAction = pAction;
+ nOffset = i;
+ break;
+ }
}
if (!pViewAction)
@@ -421,7 +428,13 @@ bool UndoManager::IsViewUndoActionIndependent(const
SwView* pView) const
}
}
- return rViewInsert.IsIndependent(rTopInsert);
+ if (!rViewInsert.IsIndependent(rTopInsert))
+ {
+ return false;
+ }
+
+ rOffset = nOffset;
+ return true;
}
bool
@@ -442,7 +455,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();
// Unless we know that the other view's undo action is independent
from us.
- if (pAction->GetViewShellId() != nViewShellId &&
!IsViewUndoActionIndependent(pView))
+ if (pAction->GetViewShellId() != nViewShellId
+ && !IsViewUndoActionIndependent(pView,
o3tl::temporary(sal_uInt16())))
{
if (o_pId)
{
diff --git a/sw/source/uibase/shells/basesh.cxx
b/sw/source/uibase/shells/basesh.cxx
index f492123f9b47..40c12473759f 100644
--- a/sw/source/uibase/shells/basesh.cxx
+++ b/sw/source/uibase/shells/basesh.cxx
@@ -627,12 +627,13 @@ void SwBaseShell::ExecUndo(SfxRequest &rReq)
const SfxUndoAction* pAction = rManager.GetUndoAction();
SwView& rView = rWrtShell.GetView();
ViewShellId nViewShellId = rView.GetViewShellId();
+ sal_uInt16 nOffset = 0;
if (pAction->GetViewShellId() != nViewShellId
- && rManager.IsViewUndoActionIndependent(&rView))
+ && rManager.IsViewUndoActionIndependent(&rView,
nOffset))
{
// 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;
+ nUndoOffset += nOffset;
}
}
