include/svl/undo.hxx | 5 +++++
sd/source/core/undo/undomanager.cxx | 2 ++
svl/source/undo/undo.cxx | 28 ++++++++++++++++++++++++++++
sw/source/core/doc/docdesc.cxx | 7 ++++++-
sw/source/uibase/shells/basesh.cxx | 6 ------
5 files changed, 41 insertions(+), 7 deletions(-)
New commits:
commit 0a9da9e43c53406c867fe95cdd5c7192ce08bcb6
Author: Justin Luth <jl...@mail.com>
AuthorDate: Fri Jul 19 19:32:39 2024 -0400
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Fri Aug 16 09:03:26 2024 +0200
tdf#161741 tdf#161705 undo: delay ClearRedo until Undo finishes
I know NOTHING about the intricacies of Undo/Redo.
However, this is my feeble attempt to add some sanity to it.
It should not be the responsibility of the caller
to know when they are allowed to call ClearRedo.
This patch reverts commit 0cd000bb83719982c1fd2265ea040c82af5bf98e
author Daniel Arato (NISZ) on Mon May 10 15:41:28 2021 +0200
tdf#141613 sw: avoid possible crash when undoing header creation
which was not a sufficient hack.
I hope this patch lays a better framework
to handle future similar issues.
vvv NAIVE OPTIMIST ALERT vvv
mbDoing was added with
commit 9cb53122e9e098bc8a6bf53c14b18415e369dd6d
Author: Frank Schoenheit on Fri Oct 22 15:00:39 2010 +0200
undoapi: more I/SfxUndoManager changes
in preparation of new Undo API features
and has been untouched since then AFAICS,
and there was never any mechanism to change mbDoing.
In other words, it has been sitting there doing NOTHING.
So, I am taking it over and using it how I imagine it was intended,
and how it is documented.
Change-Id: I7aa355cc6458ac8ba34ddb9ee73fc850dcbca702
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170793
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmik...@collabora.com>
diff --git a/include/svl/undo.hxx b/include/svl/undo.hxx
index 0e8b9ddb1f73..707a6d8b025b 100644
--- a/include/svl/undo.hxx
+++ b/include/svl/undo.hxx
@@ -25,6 +25,7 @@
#include <o3tl/strong_int.hxx>
#include <memory>
+#include <optional>
#include <vector>
typedef o3tl::strong_int<sal_Int32, struct ViewShellIdTag> ViewShellId;
@@ -220,6 +221,10 @@ public:
Will assert and bail out when called while within a list action
(<member>IsInListAction</member>).
*/
virtual void ClearRedo();
+
+ const std::optional<bool>& GetNeedsClearRedo() const;
+ void SetNeedsClearRedo(const std::optional<bool>& oSet);
+
/** leaves any possible open list action
(<member>IsInListAction</member>), and clears both the Undo and the
Redo stack.
diff --git a/sd/source/core/undo/undomanager.cxx
b/sd/source/core/undo/undomanager.cxx
index 672fe00e1fd0..462a2eb7b151 100644
--- a/sd/source/core/undo/undomanager.cxx
+++ b/sd/source/core/undo/undomanager.cxx
@@ -33,6 +33,7 @@ void UndoManager::EnterListAction(const OUString &rComment,
const OUString& rRep
ClearLinkedRedoActions();
SdrUndoManager::EnterListAction( rComment, rRepeatComment, nId,
nViewShellId );
}
+ else assert(false);
}
void UndoManager::AddUndoAction( std::unique_ptr<SfxUndoAction> pAction, bool
bTryMerg /* = sal_False */ )
@@ -42,6 +43,7 @@ void UndoManager::AddUndoAction(
std::unique_ptr<SfxUndoAction> pAction, bool bT
ClearLinkedRedoActions();
SdrUndoManager::AddUndoAction( std::move(pAction), bTryMerg );
}
+ else assert(false);
}
void UndoManager::SetLinkedUndoManager (SfxUndoManager* pLinkedUndoManager)
diff --git a/svl/source/undo/undo.cxx b/svl/source/undo/undo.cxx
index 488a0538244f..51849ebc0744 100644
--- a/svl/source/undo/undo.cxx
+++ b/svl/source/undo/undo.cxx
@@ -185,6 +185,7 @@ struct SfxUndoManager_Data
bool mbDoing;
bool mbClearUntilTopLevel;
bool mbEmptyActions;
+ std::optional<bool> moNeedsClearRedo;
UndoListeners aListeners;
@@ -474,6 +475,11 @@ void SfxUndoManager::ClearAllLevels()
void SfxUndoManager::ImplClearRedo_NoLock( bool const i_currentLevel )
{
+ if (IsDoing())
+ {
+ SetNeedsClearRedo(i_currentLevel);
+ return;
+ }
UndoManagerGuard aGuard( *m_xData );
ImplClearRedo( aGuard, i_currentLevel );
}
@@ -486,6 +492,15 @@ void SfxUndoManager::ClearRedo()
ImplClearRedo_NoLock( CurrentLevel );
}
+const std::optional<bool>& SfxUndoManager::GetNeedsClearRedo() const
+{
+ return m_xData->moNeedsClearRedo;
+}
+
+void SfxUndoManager::SetNeedsClearRedo(const std::optional<bool>& oSet)
+{
+ m_xData->moNeedsClearRedo = oSet;
+}
void SfxUndoManager::Reset()
{
@@ -685,6 +700,8 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext*
i_contextOrNull )
assert( !IsDoing() && "SfxUndoManager::Undo: *nested* Undo/Redo actions?
How this?" );
::comphelper::FlagGuard aDoingGuard( m_xData->mbDoing );
+ m_xData->mbDoing = true;
+
LockGuard aLockGuard( *this );
if ( ImplIsInListAction_Lock() )
@@ -746,6 +763,13 @@ bool SfxUndoManager::ImplUndo( SfxUndoContext*
i_contextOrNull )
throw;
}
+ m_xData->mbDoing = false;
+ if (GetNeedsClearRedo().has_value())
+ {
+ ImplClearRedo_NoLock(*GetNeedsClearRedo());
+ SetNeedsClearRedo(std::optional<bool>());
+ }
+
aGuard.scheduleNotification( &SfxUndoListener::actionUndone,
sActionComment );
return true;
@@ -810,6 +834,8 @@ bool SfxUndoManager::ImplRedo( SfxUndoContext*
i_contextOrNull )
assert( !IsDoing() && "SfxUndoManager::Redo: *nested* Undo/Redo actions?
How this?" );
::comphelper::FlagGuard aDoingGuard( m_xData->mbDoing );
+ m_xData->mbDoing = true;
+
LockGuard aLockGuard( *this );
if ( ImplIsInListAction_Lock() )
@@ -856,6 +882,8 @@ bool SfxUndoManager::ImplRedo( SfxUndoContext*
i_contextOrNull )
throw;
}
+ m_xData->mbDoing = false;
+ assert(!GetNeedsClearRedo().has_value() && "Assuming I don't need to
handle it here. What about if thrown?");
ImplCheckEmptyActions();
aGuard.scheduleNotification( &SfxUndoListener::actionRedone,
sActionComment );
diff --git a/sw/source/core/doc/docdesc.cxx b/sw/source/core/doc/docdesc.cxx
index a2a3cc977f4a..92b1de4e9cf4 100644
--- a/sw/source/core/doc/docdesc.cxx
+++ b/sw/source/core/doc/docdesc.cxx
@@ -434,7 +434,7 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc &rChged
)
{
SwUndoId nBeingUndone(SwUndoId::EMPTY);
GetIDocumentUndoRedo().GetFirstRedoInfo(nullptr, &nBeingUndone);
- if (SwUndoId::HEADER_FOOTER == nBeingUndone)
+ if (SwUndoId::HEADER_FOOTER == nBeingUndone ||
SwUndoId::INSERT_PAGE_NUMBER == nBeingUndone)
{
// The last format change is currently being undone. Remove
header/footer and corresponding nodes.
SwFormatHeader aDescMasterHeaderFormat =
rDesc.GetMaster().GetFormatAttr(RES_HEADER);
@@ -509,6 +509,11 @@ void SwDoc::ChgPageDesc( size_t i, const SwPageDesc
&rChged )
lDelHFFormat(&aDescLeftFooterFormat,
aDescLeftFooterFormat.GetFooterFormat());
else if (aDescFirstLeftFooterFormat.GetFooterFormat() &&
aDescFirstLeftFooterFormat != aChgedFirstLeftFooterFormat)
lDelHFFormat(&aDescFirstLeftFooterFormat,
aDescFirstLeftFooterFormat.GetFooterFormat());
+
+ // tdf#141613 FIXME: Disable redoing header/footer changes for now.
+ // The proper solution would be to write a SwUndoHeaderFooter class
+ // to represent the addition of a header or footer to the current
page.
+ GetIDocumentUndoRedo().ClearRedo();
}
}
::sw::UndoGuard const undoGuard(GetIDocumentUndoRedo());
diff --git a/sw/source/uibase/shells/basesh.cxx
b/sw/source/uibase/shells/basesh.cxx
index 51e2a2ea7064..1a7fc5b6c197 100644
--- a/sw/source/uibase/shells/basesh.cxx
+++ b/sw/source/uibase/shells/basesh.cxx
@@ -680,12 +680,6 @@ void SwBaseShell::ExecUndo(SfxRequest &rReq)
for (SwViewShell& rShell : rWrtShell.GetRingContainer())
rShell.UnlockPaint();
-
- // tdf#141613 FIXME: Disable redoing header/footer changes for
now.
- // The proper solution would be to write a SwUndoHeaderFooter
class
- // to represent the addition of a header or footer to the
current page.
- if (nUndoId == SwUndoId::HEADER_FOOTER)
- rUndoRedo.ClearRedo();
}
break;
