include/svl/undo.hxx | 6 ++
svl/source/undo/undo.cxx | 10 +++
sw/inc/IDocumentUndoRedo.hxx | 3 +
sw/inc/editsh.hxx | 2
sw/qa/extras/tiledrendering/tiledrendering.cxx | 47 ++++++++++++++++
sw/source/core/edit/edundo.cxx | 5 -
sw/source/core/inc/UndoCore.hxx | 5 +
sw/source/core/inc/UndoInsert.hxx | 2
sw/source/core/inc/UndoManager.hxx | 9 ++-
sw/source/core/undo/docundo.cxx | 71 +++++++++++++++++++++++--
sw/source/core/undo/unins.cxx | 5 +
sw/source/uibase/inc/wrtsh.hxx | 2
sw/source/uibase/shells/basesh.cxx | 21 +++++++
sw/source/uibase/wrtsh/wrtundo.cxx | 4 -
14 files changed, 178 insertions(+), 14 deletions(-)
New commits:
commit c72e500ccaf0ce2261c5233b80fba9342778f810
Author: Miklos Vajna <vmik...@collabora.com>
AuthorDate: Wed Nov 10 08:50:08 2021 +0100
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Thu Nov 11 09:11:10 2021 +0100
sw: allow undo of typing in 2 views independent from each other
Undoing out of order is dangerous by default, so limit this to a very
specific case as a start, that allows growing in follow-up commits.
For now, allow out of order undo if:
1) redo stack is empty
2) we're in LOK mode (different views represent different users)
3) we undo a single action (count is 1)
4) the top undo action doesn't belong to the current view
5) the top and the previous undo actions are independent
Which only requires that SwUndoInsert::UndoImpl() is independent for two
different paragraphs, which seems to be the case.
Independent undo actions opt in for this, currently the only such
allowed undo action is SwUndoInsert ("typing"), which adds characters to
a single text node. Even those are only considered independent if they
operate on different text nodes.
On the positive side, this allows out of order undo in the frequent case
where two users collaborate on a long document and they just type some
new content into the document at different paragraphs.
(cherry picked from commit 8e8e72f08b01a284cf1a90b888d48acfb6a33d2e)
Conflicts:
sw/source/core/inc/UndoManager.hxx
sw/source/uibase/shells/basesh.cxx
Change-Id: Ibb4551e8f7046b4947491b8bf751eaa0cbb2d060
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/124955
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com>
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
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 2ce8fa84bf17..5889d59a064b 100644
--- a/svl/source/undo/undo.cxx
+++ b/svl/source/undo/undo.cxx
@@ -688,6 +688,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 ff7393292eb5..f3aecf92759c 100644
--- a/sw/inc/editsh.hxx
+++ b/sw/inc/editsh.hxx
@@ -608,7 +608,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 20a8141584fb..d596f7ea7fb7 100644
--- a/sw/qa/extras/tiledrendering/tiledrendering.cxx
+++ b/sw/qa/extras/tiledrendering/tiledrendering.cxx
@@ -107,6 +107,7 @@ public:
void testTextEditViewInvalidations();
void testUndoInvalidations();
void testUndoLimiting();
+ void testUndoReordering();
void testUndoShapeLimiting();
void testUndoDispatch();
void testUndoRepairDispatch();
@@ -187,6 +188,7 @@ public:
CPPUNIT_TEST(testTextEditViewInvalidations);
CPPUNIT_TEST(testUndoInvalidations);
CPPUNIT_TEST(testUndoLimiting);
+ CPPUNIT_TEST(testUndoReordering);
CPPUNIT_TEST(testUndoShapeLimiting);
CPPUNIT_TEST(testUndoDispatch);
CPPUNIT_TEST(testUndoRepairDispatch);
@@ -1299,6 +1301,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<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.
+ 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 a55f45651fba..4a5c3c73bd88 100644
--- a/sw/source/core/edit/edundo.cxx
+++ b/sw/source/core/edit/edundo.cxx
@@ -94,7 +94,7 @@ void SwEditShell::HandleUndoRedoContext(::sw::UndoRedoContext
& rContext)
}
}
-void SwEditShell::Undo(sal_uInt16 const nCount)
+void SwEditShell::Undo(sal_uInt16 const nCount, sal_uInt16 nOffset)
{
CurrShell aCurr( this );
@@ -128,8 +128,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 8fb5e8dcf3f7..af6025c121b0 100644
--- a/sw/source/core/inc/UndoCore.hxx
+++ b/sw/source/core/inc/UndoCore.hxx
@@ -106,11 +106,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
diff --git a/sw/source/core/inc/UndoInsert.hxx
b/sw/source/core/inc/UndoInsert.hxx
index 801997d65e39..d5476b432a27 100644
--- a/sw/source/core/inc/UndoInsert.hxx
+++ b/sw/source/core/inc/UndoInsert.hxx
@@ -86,6 +86,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 4113d54d8f52..93126322f809 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<SfxUndoAction> 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;
+
protected:
virtual void EmptyActionsChanged() override;
@@ -119,7 +126,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 6e350836fc20..8c46f659cf06 100644
--- a/sw/source/core/undo/docundo.cxx
+++ b/sw/source/core/undo/docundo.cxx
@@ -40,6 +40,8 @@
#include <sfx2/viewfrm.hxx>
#include <sfx2/bindings.hxx>
+#include <UndoInsert.hxx>
+
using namespace ::com::sun::star;
// the undo array should never grow beyond this limit:
@@ -351,6 +353,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<const SwUndo*>(pTopAction);
+ if (!pTopSwAction || pTopSwAction->GetId() != SwUndoId::TYPING)
+ {
+ return false;
+ }
+
+ auto pViewSwAction = dynamic_cast<const SwUndo*>(pViewAction);
+ if (!pViewSwAction || pViewSwAction->GetId() != SwUndoId::TYPING)
+ {
+ return false;
+ }
+
+ const auto& rTopInsert = *static_cast<const SwUndoInsert*>(pTopSwAction);
+ const auto& rViewInsert = *static_cast<const SwUndoInsert*>(pViewSwAction);
+
+ return rViewInsert.IsIndependent(rTopInsert);
+}
+
bool
UndoManager::GetLastUndoInfo(
OUString *const o_pStr, SwUndoId *const o_pId, const SwView* pView)
const
@@ -368,7 +425,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)
{
@@ -571,7 +629,7 @@ private:
}
-bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo)
+bool UndoManager::impl_DoUndoRedo(UndoOrRedoType undoOrRedo, size_t
nUndoOffset)
{
SwDoc & rDoc(GetUndoNodes().GetDoc());
@@ -600,6 +658,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)
@@ -629,7 +688,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())
{
@@ -637,7 +698,7 @@ bool UndoManager::Undo()
}
else
{
- return impl_DoUndoRedo(UndoOrRedoType::Undo);
+ return impl_DoUndoRedo(UndoOrRedoType::Undo, nUndoOffset);
}
}
@@ -649,7 +710,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 dcd3c4c689f0..0dd0d24d77cd 100644
--- a/sw/source/core/undo/unins.cxx
+++ b/sw/source/core/undo/unins.cxx
@@ -475,6 +475,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 a98cb26bfdfc..3374c10e55dc 100644
--- a/sw/source/uibase/inc/wrtsh.hxx
+++ b/sw/source/uibase/inc/wrtsh.hxx
@@ -371,7 +371,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 57e9394fc03c..fc7c5daf8ce4 100644
--- a/sw/source/uibase/shells/basesh.cxx
+++ b/sw/source/uibase/shells/basesh.cxx
@@ -96,6 +96,7 @@
#include <shellres.hxx>
#include <UndoTable.hxx>
+#include <UndoManager.hxx>
FlyMode SwBaseShell::eFrameMode = FLY_DRAG_END;
@@ -554,7 +555,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 1e92006a232b..b409e4472513 100644
--- a/sw/source/uibase/wrtsh/wrtundo.cxx
+++ b/sw/source/uibase/wrtsh/wrtundo.cxx
@@ -29,7 +29,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();
@@ -41,7 +41,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#
