sw/inc/doc.hxx | 4 - sw/qa/core/undo/undo.cxx | 109 +++++++++++++++++++++++++++++++++ sw/source/core/inc/SwUndoPageDesc.hxx | 3 sw/source/core/inc/UndoAttribute.hxx | 1 sw/source/core/undo/SwUndoPageDesc.cxx | 108 ++++++++++++++++++++++++++++++++ sw/source/core/undo/unattr.cxx | 28 ++++++++ 6 files changed, 251 insertions(+), 2 deletions(-)
New commits: commit 6de1b56c576cafae0d9ee027f037e45533983a5b Author: David Hashe <[email protected]> AuthorDate: Thu Nov 20 18:11:33 2025 -0500 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Nov 25 19:45:03 2025 +0100 tdf#148703 Re-apply page desc to nodes when undoing Creating and deleting page descs was not interacting well with undo/redo. Redoing the creation or undoing the deletion of a page desc did not restore it to the nodes that previously had it, nor did it restore the page desc as the follow to any other page descs that it previously followed. This patch does four related things: 1) make SwUndoPageDescCreate do a linear scan of the undo nodes and record which nodes this page desc will apply to, and then make those nodes use the new page desc on redo 2) make SwUndoAttr look up page descs by UIName, so that it finds and is able to apply the recreated page desc (this class already did something similar for char styles). 3) make SwUndoPageDescDelete do a linear scan (linear in number of pages) of the document nodes and record which nodes this page desc applies do, and then make those nodes use the new page desc on undo 4) make SwUndoPageDescDelete record which other page descs in the document have this page desc as a follow, and then make those page desc use the new page desc as the follow on undo Together, these changes make page descs play nice with undo/redo. This is necessary to complete tdf#148703 because a part of that bug involves undoing and redoing the creation of a page desc. Testing: CPPUNIT_TEST_NAME="SwCoreUndoTest testPageDescCreate" make CppunitTest_sw_core_undo CPPUNIT_TEST_NAME="SwCoreUndoTest testPageDescDelete" make CppunitTest_sw_core_undo I have also manually tested against the example docs from this comment: https://bugs.documentfoundation.org/show_bug.cgi?id=148703#c0 Change-Id: Ic80454bb364e45999bb8e92462eca473c5d1a328 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/194295 Reviewed-by: Mike Kaganski <[email protected]> Code-Style: Mike Kaganski <[email protected]> Tested-by: Jenkins diff --git a/sw/inc/doc.hxx b/sw/inc/doc.hxx index 3ee03fbe31f1..aaaccc335651 100644 --- a/sw/inc/doc.hxx +++ b/sw/inc/doc.hxx @@ -922,8 +922,8 @@ public: // For Reader SW_DLLPUBLIC void ChgPageDesc( const UIName & rName, const SwPageDesc& ); SW_DLLPUBLIC void ChgPageDesc( size_t i, const SwPageDesc& ); - void DelPageDesc( const UIName & rName, bool bBroadcast = false); - void DelPageDesc( size_t i, bool bBroadcast = false ); + SW_DLLPUBLIC void DelPageDesc( const UIName & rName, bool bBroadcast = false ); + SW_DLLPUBLIC void DelPageDesc( size_t i, bool bBroadcast = false ); void PreDelPageDesc(SwPageDesc const * pDel); SW_DLLPUBLIC SwPageDesc* MakePageDesc(const UIName &rName, const SwPageDesc* pCpy = nullptr, bool bRegardLanguage = true); diff --git a/sw/qa/core/undo/undo.cxx b/sw/qa/core/undo/undo.cxx index ad8eeed2a27d..4ac1f0fe251a 100644 --- a/sw/qa/core/undo/undo.cxx +++ b/sw/qa/core/undo/undo.cxx @@ -17,6 +17,10 @@ #include <swdtflvr.hxx> #include <frameformats.hxx> #include <fmtcntnt.hxx> +#include <cntfrm.hxx> +#include <fmtpdsc.hxx> +#include <pagefrm.hxx> +#include <rootfrm.hxx> /// Covers sw/source/core/undo/ fixes. class SwCoreUndoTest : public SwModelTestBase @@ -230,6 +234,111 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTdf148703CopyFooterAcrossDocuments) xText->insertString(xCursor, u"test"_ustr, /*bAbsorb=*/false); } +static const SwPageDesc* lcl_getLastPagePageDesc(SwDoc& rDoc) +{ + SwRootFrame* pRootFrame = rDoc.GetAllLayouts()[0]; + const SwPageFrame* pPageFrameIter = pRootFrame->GetLastPage(); + const SwContentFrame* pContentFrame = pPageFrameIter->FindFirstBodyContent(); + const SwFormatPageDesc& rFormatPageDesc = pContentFrame->GetPageDescItem(); + const sw::BroadcastingModify* pMod = rFormatPageDesc.GetDefinedIn(); + + if (auto pContentNode = dynamic_cast<const SwContentNode*>(pMod)) + return pContentNode->GetAttr(RES_PAGEDESC).GetPageDesc(); + + return nullptr; +} + +CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testPageDescDelete) +{ + createSwDoc(); + + SwDoc* pDoc = getSwDoc(); + + // Create two custom page descs such that a page using customPageDesc1 would be followed + // by a page containing customPageDesc2, and then set the second page of the document + // to use customPageDesc2. (the first page just uses the default page desc) + + SwPageDesc* pOldPageDesc2 = pDoc->MakePageDesc(UIName("customPageDesc2"), nullptr, true); + SwPageDesc* pPageDesc1 = pDoc->MakePageDesc(UIName("customPageDesc1"), nullptr, true); + pPageDesc1->SetFollow(pOldPageDesc2); + + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + + UIName aName2("customPageDesc2"); + pWrtShell->InsertPageBreak(&aName2); + + CPPUNIT_ASSERT_EQUAL(lcl_getLastPagePageDesc(*pDoc), + const_cast<const SwPageDesc*>(pOldPageDesc2)); + + pDoc->DelPageDesc(UIName("customPageDesc2")); + + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + + SwPageDesc* pNewPageDesc2 = pDoc->FindPageDesc(UIName("customPageDesc2")); + + // After deleting customPageDesc2 and undoing that delete, customPageDesc1 should still + // have customPageDesc2 as its follow. + + CPPUNIT_ASSERT(pNewPageDesc2); + CPPUNIT_ASSERT_EQUAL(pNewPageDesc2, pPageDesc1->GetFollow()); + + // Deleting customPageDesc2 removed it from the second page of the document. Undoing that + // delete should have re-added it to the second page. + + CPPUNIT_ASSERT_EQUAL(lcl_getLastPagePageDesc(*pDoc), + const_cast<const SwPageDesc*>(pNewPageDesc2)); + + dispatchCommand(mxComponent, u".uno:Redo"_ustr, {}); +} + +CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testPageDescCreate) +{ + createSwDoc(); + + SwDoc* pDoc = getSwDoc(); + + // Create two custom page descs such that a page using customPageDesc1 would be followed + // by a page containing customPageDesc2, and then set the second page of the document + // to use customPageDesc2. (the first page just uses the default page desc) + + SwPageDesc* pOldPageDesc2 = pDoc->MakePageDesc(UIName("customPageDesc2"), nullptr, true); + SwPageDesc* pOldPageDesc1 = pDoc->MakePageDesc(UIName("customPageDesc1"), nullptr, true); + pOldPageDesc1->SetFollow(pOldPageDesc2); + + SwDocShell* pDocShell = getSwDocShell(); + SwWrtShell* pWrtShell = pDocShell->GetWrtShell(); + + UIName aName2("customPageDesc2"); + pWrtShell->InsertPageBreak(&aName2); + + CPPUNIT_ASSERT_EQUAL(lcl_getLastPagePageDesc(*pDoc), + const_cast<const SwPageDesc*>(pOldPageDesc2)); + + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); // undo insert page break + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); // undo create customPageDesc1 + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); // undo create customPageDesc2 + dispatchCommand(mxComponent, u".uno:Redo"_ustr, {}); + dispatchCommand(mxComponent, u".uno:Redo"_ustr, {}); + dispatchCommand(mxComponent, u".uno:Redo"_ustr, {}); + + SwPageDesc* pNewPageDesc1 = pDoc->FindPageDesc(UIName("customPageDesc1")); + SwPageDesc* pNewPageDesc2 = pDoc->FindPageDesc(UIName("customPageDesc2")); + + // After undoing past the creation of both custom page descs, and then redoing to the present, + // both page descs should exist and customPageDesc2 should still follow customPageDesc1. + + CPPUNIT_ASSERT(pNewPageDesc1); + CPPUNIT_ASSERT(pNewPageDesc2); + CPPUNIT_ASSERT_EQUAL(pNewPageDesc2, pNewPageDesc1->GetFollow()); + + // Undoing the creation of customPageDesc2 removed it from the second page of the document. Redoing + // should have re-added it to the second page. + + CPPUNIT_ASSERT_EQUAL(lcl_getLastPagePageDesc(*pDoc), + const_cast<const SwPageDesc*>(pNewPageDesc2)); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/inc/SwUndoPageDesc.hxx b/sw/source/core/inc/SwUndoPageDesc.hxx index 5d63a7d12d5c..7c14642647f2 100644 --- a/sw/source/core/inc/SwUndoPageDesc.hxx +++ b/sw/source/core/inc/SwUndoPageDesc.hxx @@ -52,6 +52,7 @@ class SwUndoPageDescCreate final : public SwUndo const SwPageDesc * m_pDesc; SwPageDescExt m_aNew; SwDoc& m_rDoc; + std::vector<SwNodeOffset> m_aAppliedNodes; void DoImpl(); @@ -70,6 +71,8 @@ class SwUndoPageDescDelete final : public SwUndo { SwPageDescExt m_aOld; SwDoc& m_rDoc; + std::vector<SwNodeOffset> m_aAppliedNodes; + std::vector<UIName> m_aPageDescsThisFollowed; void DoImpl(); diff --git a/sw/source/core/inc/UndoAttribute.hxx b/sw/source/core/inc/UndoAttribute.hxx index bb0c1d082176..8aab4092c027 100644 --- a/sw/source/core/inc/UndoAttribute.hxx +++ b/sw/source/core/inc/UndoAttribute.hxx @@ -43,6 +43,7 @@ class SwUndoAttr final : public SwUndo, private SwUndRng SwNodeOffset m_nNodeIndex; // Offset: for Redlining const SetAttrMode m_nInsertFlags; // insert flags UIName m_aChrFormatName; + UIName m_aPageDescName; void RemoveIdx( SwDoc& rDoc ); void redoAttribute(SwPaM& rPam, const sw::UndoRedoContext& rContext); diff --git a/sw/source/core/undo/SwUndoPageDesc.cxx b/sw/source/core/undo/SwUndoPageDesc.cxx index 3df5d986c691..6eea25cbafbb 100644 --- a/sw/source/core/undo/SwUndoPageDesc.cxx +++ b/sw/source/core/undo/SwUndoPageDesc.cxx @@ -24,11 +24,16 @@ #include <pagedesc.hxx> #include <SwUndoPageDesc.hxx> #include <SwRewriter.hxx> +#include <UndoManager.hxx> #include <undobj.hxx> #include <strings.hrc> #include <fmtcntnt.hxx> #include <fmthdft.hxx> #include <osl/diagnose.h> +#include <cntfrm.hxx> +#include <fmtpdsc.hxx> +#include <pagefrm.hxx> +#include <rootfrm.hxx> SwUndoPageDesc::SwUndoPageDesc(const SwPageDesc & _aOld, const SwPageDesc & _aNew, @@ -268,6 +273,29 @@ void SwUndoPageDescCreate::UndoImpl(::sw::UndoRedoContext &) { if (m_pDesc) { + SwNodes& rUndoNds = m_rDoc.GetUndoManager().GetUndoNodes(); + + // Record which nodes in the undo nodes used this page description. + rUndoNds.ForEach([](SwNode* pNode, void* pArgs) -> bool { + SwUndoPageDescCreate* pThis = static_cast<SwUndoPageDescCreate*>(pArgs); + + const SwPageDesc *pPgDesc = nullptr; + + if (pNode->IsContentNode()) + pPgDesc = pNode->GetContentNode()->GetAttr(RES_PAGEDESC).GetPageDesc(); + else if (pNode->IsTableNode()) + pPgDesc = pNode->GetTableNode()->GetTable(). + GetFrameFormat()->GetPageDesc().GetPageDesc(); + else if (pNode->IsSectionNode()) + pPgDesc = pNode->GetSectionNode()->GetSection(). + GetFormat()->GetPageDesc().GetPageDesc(); + + if (pPgDesc == pThis->m_pDesc) + pThis->m_aAppliedNodes.push_back(pNode->GetIndex()); + + return true; + }, this); + m_aNew = *m_pDesc; m_pDesc = nullptr; } @@ -281,6 +309,21 @@ void SwUndoPageDescCreate::DoImpl() SwPageDesc* pNewPageDesc = m_rDoc.MakePageDesc(m_aNew.GetName(), &aPageDesc, false); if (m_aNew.GetName() == m_aNew.GetFollowName()) pNewPageDesc->SetFollow(pNewPageDesc); + + SwNodes& rUndoNds = m_rDoc.GetUndoManager().GetUndoNodes(); + + for (const SwNodeOffset& aOffset : m_aAppliedNodes) + { + SwNode* pNd = rUndoNds[aOffset]; + + SwFormatPageDesc aNew(pNewPageDesc); + if (pNd->IsContentNode()) + pNd->GetContentNode()->SetAttr(aNew); + else if (pNd->IsTableNode()) + pNd->GetTableNode()->GetTable().GetFrameFormat()->SetFormatAttr(aNew); + else if (pNd->IsSectionNode()) + pNd->GetSectionNode()->GetSection().GetFormat()->SetFormatAttr(aNew); + } } void SwUndoPageDescCreate::RedoImpl(::sw::UndoRedoContext &) @@ -310,6 +353,50 @@ SwUndoPageDescDelete::SwUndoPageDescDelete(const SwPageDesc & _aOld, SwDoc& rDoc) : SwUndo(SwUndoId::DELETE_PAGEDESC, rDoc), m_aOld(_aOld, rDoc), m_rDoc(rDoc) { + // Record which page descriptions in this document used this page desc as their follow + for (size_t i = 0; i < m_rDoc.GetPageDescCnt(); ++i) + { + if (const auto rPageDesc = m_rDoc.GetPageDesc(i); rPageDesc.GetFollow() == &_aOld) + { + m_aPageDescsThisFollowed.emplace_back(rPageDesc.GetName()); + } + } + + // Record which nodes in the document used this page description. + for (SwRootFrame* pRootFrame : rDoc.GetAllLayouts()) + { + const SwPageFrame* pPageFrameIter = pRootFrame->GetLastPage(); + while (pPageFrameIter) + { + const SwContentFrame* pContentFrame = pPageFrameIter->FindFirstBodyContent(); + if (pContentFrame) + { + const SwFormatPageDesc& rFormatPageDesc = pContentFrame->GetPageDescItem(); + if (const sw::BroadcastingModify* pMod = rFormatPageDesc.GetDefinedIn()) + { + const SwPageDesc *pPgDesc = nullptr; + if (auto pNd = dynamic_cast<const SwNode*>( pMod); pNd != nullptr) + { + pPgDesc = pNd->GetContentNode()->GetAttr( RES_PAGEDESC ).GetPageDesc(); + + if (pNd->IsContentNode()) + pPgDesc = pNd->GetContentNode()->GetAttr( RES_PAGEDESC ).GetPageDesc(); + else if (pNd->IsTableNode()) + pPgDesc = pNd->GetTableNode()->GetTable(). + GetFrameFormat()->GetPageDesc().GetPageDesc(); + else if (pNd->IsSectionNode()) + pPgDesc = pNd->GetSectionNode()->GetSection(). + GetFormat()->GetPageDesc().GetPageDesc(); + + if (pPgDesc == &_aOld) + m_aAppliedNodes.push_back(pNd->GetIndex()); + } + + } + } + pPageFrameIter = static_cast<const SwPageFrame*>(pPageFrameIter->GetPrev()); + } + } } SwUndoPageDescDelete::~SwUndoPageDescDelete() @@ -322,6 +409,27 @@ void SwUndoPageDescDelete::UndoImpl(::sw::UndoRedoContext &) SwPageDesc* pNewPageDesc = m_rDoc.MakePageDesc(m_aOld.GetName(), &aPageDesc, false); if (m_aOld.GetName() == m_aOld.GetFollowName()) pNewPageDesc->SetFollow(pNewPageDesc); + + for (const UIName& aName : m_aPageDescsThisFollowed) + { + if (SwPageDesc* pPageDesc = m_rDoc.FindPageDesc(aName); pPageDesc != nullptr) + { + pPageDesc->SetFollow(pNewPageDesc); + } + } + + for (const SwNodeOffset& aOffset : m_aAppliedNodes) + { + SwNode* pNd = m_rDoc.GetNodes()[aOffset]; + + SwFormatPageDesc aNew(pNewPageDesc); + if (pNd->IsContentNode()) + pNd->GetContentNode()->SetAttr(aNew); + else if (pNd->IsTableNode()) + pNd->GetTableNode()->GetTable().GetFrameFormat()->SetFormatAttr(aNew); + else if (pNd->IsSectionNode()) + pNd->GetSectionNode()->GetSection().GetFormat()->SetFormatAttr(aNew); + } } void SwUndoPageDescDelete::DoImpl() diff --git a/sw/source/core/undo/unattr.cxx b/sw/source/core/undo/unattr.cxx index d96f8ad4649d..1b7b8956c585 100644 --- a/sw/source/core/undo/unattr.cxx +++ b/sw/source/core/undo/unattr.cxx @@ -58,6 +58,7 @@ #include <calbck.hxx> #include <frameformats.hxx> #include <editsh.hxx> +#include <fmtpdsc.hxx> #if ENABLE_YRS #include <docufld.hxx> @@ -767,6 +768,14 @@ SwUndoAttr::SwUndoAttr( const SwPaM& rRange, const SfxPoolItem& rAttr, if (aValue >>= sTmp) m_aChrFormatName = UIName(sTmp); } + + // Save page desc as a style name, not as a reference + const SfxPoolItem* pPgDescItem = m_AttrSet.GetItem(RES_PAGEDESC); + if (pPgDescItem) + { + if (const SwPageDesc *pPgDesc = pPgDescItem->StaticWhichCast(RES_PAGEDESC).GetPageDesc(); pPgDesc != nullptr) + m_aPageDescName = pPgDesc->GetName(); + } } SwUndoAttr::SwUndoAttr( const SwPaM& rRange, SfxItemSet aSet, @@ -787,6 +796,14 @@ SwUndoAttr::SwUndoAttr( const SwPaM& rRange, SfxItemSet aSet, if (aValue >>= sTmp) m_aChrFormatName = UIName(sTmp); } + + // Save page desc as a style name, not as a reference + const SfxPoolItem* pPgDescItem = m_AttrSet.GetItem(RES_PAGEDESC); + if (pPgDescItem) + { + if (const SwPageDesc *pPgDesc = pPgDescItem->StaticWhichCast(RES_PAGEDESC).GetPageDesc(); pPgDesc != nullptr) + m_aPageDescName = pPgDesc->GetName(); + } } SwUndoAttr::~SwUndoAttr() @@ -906,6 +923,17 @@ void SwUndoAttr::redoAttribute(SwPaM& rPam, const sw::UndoRedoContext & rContext } } + // Restore pointer to page desc from name + if (!m_aPageDescName.isEmpty()) + { + SwPageDesc* pPageDesc = rDoc.FindPageDesc(m_aPageDescName); + if (pPageDesc) + { + SwFormatPageDesc aFormat(pPageDesc); + m_AttrSet.Put(aFormat); + } + } + if ( m_pRedlineData && IDocumentRedlineAccess::IsRedlineOn( GetRedlineFlags() ) ) { RedlineFlags eOld = rDoc.getIDocumentRedlineAccess().GetRedlineFlags();
