diff options
author | Michael Stahl <mstahl@redhat.com> | 2017-05-08 16:56:34 +0200 |
---|---|---|
committer | Michael Stahl <mstahl@redhat.com> | 2017-05-08 17:21:16 +0200 |
commit | 13d7778fc10638a145af7fb077688e032bd16f81 (patch) | |
tree | 386949c45b447a8ebe956bab50c2060bb7e0e2da | |
parent | f9f0b6ac12c5d6c73d9aa415a9de5e37105085d8 (diff) |
tdf#70960 sw::UndoManager: fix exponential Repeat proliferation
In case of a multi-selection, the usual pattern is that the EditShell
contains a loop around GetCursor()->GetRingContainer() that creates N
"groups" of undo-actions, all of which are inside a SfxListUndoAction.
If that is then repeated again with a multi-selection, the result will
be N*N actions, etc. This is usually not an issue because most
repeatable actions are idempotent - except for the resource usage.
Prevent the 2nd step of the proliferation by introducing a flag in
SwUndo to do nothing in SwUndo::Repeat() that is set for all but the
1st cursor of the multi-selection in sw::UndoManager::Repeat().
(presumably regression from CWS undoapi)
Change-Id: I095258f6d57af6bcaa8a39747632422e8311e694
-rw-r--r-- | sw/inc/undobj.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/inc/UndoManager.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/undo/docundo.cxx | 13 | ||||
-rw-r--r-- | sw/source/core/undo/undobj.cxx | 12 |
4 files changed, 26 insertions, 3 deletions
diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx index 91185bbcded0..bfd7cf804051 100644 --- a/sw/inc/undobj.hxx +++ b/sw/inc/undobj.hxx @@ -53,6 +53,7 @@ class SwUndo SwUndoId const m_nId; RedlineFlags nOrigRedlineFlags; ViewShellId m_nViewShellId; + bool m_isRepeatIgnored; /// for multi-selection, only repeat 1st selection protected: bool bCacheComment; @@ -124,6 +125,7 @@ public: static bool FillSaveDataForFormat( const SwPaM& , SwRedlineSaveDatas& ); static void SetSaveData( SwDoc& rDoc, SwRedlineSaveDatas& rSData ); static bool HasHiddenRedlines( const SwRedlineSaveDatas& rSData ); + void IgnoreRepeat() { m_isRepeatIgnored = true; } }; enum class DelContentType : sal_uInt16 diff --git a/sw/source/core/inc/UndoManager.hxx b/sw/source/core/inc/UndoManager.hxx index ede9604059ad..6064757c4bc0 100644 --- a/sw/source/core/inc/UndoManager.hxx +++ b/sw/source/core/inc/UndoManager.hxx @@ -107,6 +107,8 @@ private: /// If true, then repair mode is enabled. bool m_bRepair; bool m_bLockUndoNoModifiedPosition : 1; + /// set the IgnoreRepeat flag on every added action + bool m_isAddWithIgnoreRepeat; /// position in Undo-Array at which Doc was saved (and is not modified) UndoStackMark m_UndoSaveMark; SwDocShell* m_pDocShell; diff --git a/sw/source/core/undo/docundo.cxx b/sw/source/core/undo/docundo.cxx index 833044fb3f49..42417c94af38 100644 --- a/sw/source/core/undo/docundo.cxx +++ b/sw/source/core/undo/docundo.cxx @@ -61,6 +61,7 @@ UndoManager::UndoManager(std::shared_ptr<SwNodes> const & xUndoNodes, , m_bDrawUndo(true) , m_bRepair(false) , m_bLockUndoNoModifiedPosition(false) + , m_isAddWithIgnoreRepeat(false) , m_UndoSaveMark(MARK_INVALID) , m_pDocShell(nullptr) , m_pView(nullptr) @@ -520,6 +521,10 @@ void UndoManager::AddUndoAction(SfxUndoAction *pAction, bool bTryMerge) { pUndo->SetRedlineFlags( m_rRedlineAccess.GetRedlineFlags() ); } + if (m_isAddWithIgnoreRepeat) + { + pUndo->IgnoreRepeat(); + } } SdrUndoManager::AddUndoAction(pAction, bTryMerge); // if the undo nodes array is too large, delete some actions @@ -685,10 +690,18 @@ bool UndoManager::Repeat(::sw::RepeatContext & rContext, for(SwPaM& rPaM : rContext.GetRepeatPaM().GetRingContainer()) { // iterate over ring rContext.m_pCurrentPaM = &rPaM; + if (DoesUndo() && & rPaM != pTmp) + { + m_isAddWithIgnoreRepeat = true; + } for (sal_uInt16 nRptCnt = nRepeatCount; nRptCnt > 0; --nRptCnt) { pRepeatAction->Repeat(rContext); } + if (DoesUndo() && & rPaM != pTmp) + { + m_isAddWithIgnoreRepeat = false; + } rContext.m_bDeleteRepeated = false; // reset for next PaM } rContext.m_pCurrentPaM = pTmp; diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx index 96c8969dee6d..2ad6df159422 100644 --- a/sw/source/core/undo/undobj.cxx +++ b/sw/source/core/undo/undobj.cxx @@ -158,9 +158,10 @@ void SwUndo::RemoveIdxRel( sal_uLong nIdx, const SwPosition& rPos ) } SwUndo::SwUndo(SwUndoId const nId, const SwDoc* pDoc) - : m_nId(nId), nOrigRedlineFlags(RedlineFlags::NONE), - m_nViewShellId(CreateViewShellId(pDoc)), - bCacheComment(true), pComment(nullptr) + : m_nId(nId), nOrigRedlineFlags(RedlineFlags::NONE) + , m_nViewShellId(CreateViewShellId(pDoc)) + , m_isRepeatIgnored(false) + , bCacheComment(true), pComment(nullptr) { } @@ -240,6 +241,10 @@ void SwUndo::RedoWithContext(SfxUndoContext & rContext) void SwUndo::Repeat(SfxRepeatTarget & rContext) { + if (m_isRepeatIgnored) + { + return; // ignore Repeat for multi-selections + } ::sw::RepeatContext *const pRepeatContext( dynamic_cast< ::sw::RepeatContext * >(& rContext)); assert(pRepeatContext); @@ -250,6 +255,7 @@ bool SwUndo::CanRepeat(SfxRepeatTarget & rContext) const { assert(dynamic_cast< ::sw::RepeatContext * >(& rContext)); (void)rContext; + // a MultiSelection action that doesn't do anything must still return true return (SwUndoId::REPEAT_START <= GetId()) && (GetId() < SwUndoId::REPEAT_END); } |