sw/inc/strings.hrc | 1 sw/inc/swundo.hxx | 1 sw/inc/undobj.hxx | 22 ++++++ sw/qa/core/undo/data/tdf148703-dest.odt |binary sw/qa/core/undo/data/tdf148703-src.odt |binary sw/qa/core/undo/undo.cxx | 33 ++++++++++ sw/source/core/doc/docfmt.cxx | 6 + sw/source/core/undo/undobj.cxx | 3 sw/source/core/undo/untblk.cxx | 103 ++++++++++++++++++++++++++++++++ 9 files changed, 169 insertions(+)
New commits: commit 68e1db614c8618f0155d157cb1dc4c6f259b8573 Author: David Hashe <[email protected]> AuthorDate: Tue May 20 10:53:08 2025 -0400 Commit: Mike Kaganski <[email protected]> CommitDate: Tue Nov 25 19:44:43 2025 +0100 tdf#148703 Fix Writer crash on undo after paste When copy/pasting a selection from a source document with a custom page style and a header/footer over a selection in a destination document that does not have that custom page style, the undo information was not recording the insertion of the header/footer nodes into the destination document. Then, when undoing the copy/paste, the header/footer nodes remained in the destination document and messed up the absolute indexing of the insertion point of the overwritten destination selection, so re-inserting the destination selection during the undo causes a crash. This change modifies SwDoc::CopyPageDescHeaderFooterImpl to record the insertion of the header/footer nodes so that they are correctly undone and don't mess up the indexing. This requires a new undo class SwUndoCopyHeaderFooter. There is still a bit of work needed after this change; the custom page style will be recreated correctly, but it still will not be applied to the pages that should have it in the destination document after the copy is undone and then redone. See the ticket for more details. Test plan: I added a regression test based on a pair of minimal docs: CPPUNIT_TEST_NAME="SwCoreUndoTest testTdf148703CopyFooterAcrossDocuments" 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: I8a07cd0bff45c50487d61953b1f66cd7c05905dd Reviewed-on: https://gerrit.libreoffice.org/c/core/+/185940 Code-Style: Mike Kaganski <[email protected]> Tested-by: Jenkins Reviewed-by: Mike Kaganski <[email protected]> diff --git a/sw/inc/strings.hrc b/sw/inc/strings.hrc index 135b7e1b9759..65aae7f8d4f2 100644 --- a/sw/inc/strings.hrc +++ b/sw/inc/strings.hrc @@ -495,6 +495,7 @@ #define STR_ACCEPT_REDLINE NC_("STR_ACCEPT_REDLINE", "Accept change: $1") #define STR_REJECT_REDLINE NC_("STR_REJECT_REDLINE", "Reject change: $1") #define STR_REINSTATE_REDLINE NC_("STR_REINSTATE_REDLINE", "Reject but track change: $1") +#define STR_UNDO_COPY_HEADER_FOOTER NC_("STR_UNDO_COPY_HEADER_FOOTER", "Undo copy header or footer content") #define STR_SPLIT_TABLE NC_("STR_SPLIT_TABLE", "Split Table") #define STR_DONTEXPAND NC_("STR_DONTEXPAND", "Stop attribute") #define STR_AUTOCORRECT NC_("STR_AUTOCORRECT", "AutoCorrect") diff --git a/sw/inc/swundo.hxx b/sw/inc/swundo.hxx index 11255e7966cd..e19622a4f783 100644 --- a/sw/inc/swundo.hxx +++ b/sw/inc/swundo.hxx @@ -184,6 +184,7 @@ enum class SwUndoId MAKE_ENDNOTES_FOOTNOTES = 152, CONVERT_FIELD_TO_TEXT = 153, REINSTATE_REDLINE = 154, + COPY_HEADER_FOOTER = 155, }; OUString GetUndoComment(SwUndoId eId); diff --git a/sw/inc/undobj.hxx b/sw/inc/undobj.hxx index 885db7221593..93de6b198a87 100644 --- a/sw/inc/undobj.hxx +++ b/sw/inc/undobj.hxx @@ -295,6 +295,28 @@ public: SwUndoCpyDoc( const SwPaM& ); }; +/// Undo for copying a header or footer from one document into another. +/// This needs special handling because undoing the copy needs to completely +/// remove it from the doc nodes; in other cases the header/footer content +/// is just left in the doc nodes. +class SwUndoCopyHeaderFooter final : public SwUndo, private SwUndoSaveSection +{ + SwNodeOffset m_aOff; + UIName m_aFmtName; + + bool m_bIsHeader; + bool m_bIsMaster; + bool m_bIsLeft; + bool m_bIsFirstMaster; + bool m_bIsFirstLeft; + +public: + SwUndoCopyHeaderFooter(SwDoc& rDoc, SwNode& rSttNd, const UIName& rFmtName); + + virtual void UndoImpl(::sw::UndoRedoContext & rContext) override; + virtual void RedoImpl(::sw::UndoRedoContext & rContext) override; +}; + class SwUndoFlyBase : public SwUndo, private SwUndoSaveSection { protected: diff --git a/sw/qa/core/undo/data/tdf148703-dest.odt b/sw/qa/core/undo/data/tdf148703-dest.odt new file mode 100644 index 000000000000..29b32ea74437 Binary files /dev/null and b/sw/qa/core/undo/data/tdf148703-dest.odt differ diff --git a/sw/qa/core/undo/data/tdf148703-src.odt b/sw/qa/core/undo/data/tdf148703-src.odt new file mode 100644 index 000000000000..1e52678f9609 Binary files /dev/null and b/sw/qa/core/undo/data/tdf148703-src.odt differ diff --git a/sw/qa/core/undo/undo.cxx b/sw/qa/core/undo/undo.cxx index 094432d3f344..ad8eeed2a27d 100644 --- a/sw/qa/core/undo/undo.cxx +++ b/sw/qa/core/undo/undo.cxx @@ -197,6 +197,39 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testPageDescThatFollowsItself) CPPUNIT_ASSERT_EQUAL(pNewPageDesc, pNewPageDesc->GetFollow()); } +CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTdf148703CopyFooterAcrossDocuments) +{ + // A document containing some text to overwrite and no custom page styles. + createSwDoc("tdf148703-dest.odt"); + + // A document containing a custom page style, two paragraphs, and a footer. + uno::Reference<css::lang::XComponent> xSrcComponent + = loadFromDesktop(createFileURL(u"tdf148703-src.odt"), OUString(), {}); + + dispatchCommand(xSrcComponent, u".uno:SelectAll"_ustr, {}); + dispatchCommand(xSrcComponent, u".uno:Copy"_ustr, {}); + + dispatchCommand(mxComponent, u".uno:SelectAll"_ustr, {}); + dispatchCommand(mxComponent, u".uno:Paste"_ustr, {}); + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + + // Without the fix, the Undo command crashes with: + // SwUndoDelete::UndoImpl(sw::UndoRedoContext&): Assertion `pStartNode' failed. + // That happens because the footer nodes remained in the SwNodes + // after undo and messed up the index where the SwUndoDelete re-inserts + // the deleted content. + + dispatchCommand(mxComponent, u".uno:Redo"_ustr, {}); + dispatchCommand(mxComponent, u".uno:Undo"_ustr, {}); + + // Clear the redo stack by doing any other operation, to test the whether + // the destructor works. + uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY); + uno::Reference<text::XText> xText = xTextDocument->getText(); + uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor(); + xText->insertString(xCursor, u"test"_ustr, /*bAbsorb=*/false); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/doc/docfmt.cxx b/sw/source/core/doc/docfmt.cxx index a7ab0692d0fc..e1943f90d4d9 100644 --- a/sw/source/core/doc/docfmt.cxx +++ b/sw/source/core/doc/docfmt.cxx @@ -1406,6 +1406,12 @@ void SwDoc::CopyPageDescHeaderFooterImpl( bool bCpyHeader, rSrcNds.Copy_( aRg, *pSttNd->EndOfSectionNode() ); rSrcFormat.GetDoc().GetDocumentContentOperationsManager().CopyFlyInFlyImpl(aRg, nullptr, *pSttNd); // TODO: investigate calling CopyWithFlyInFly? + + if( GetIDocumentUndoRedo().DoesUndo() ) + { + GetIDocumentUndoRedo().AppendUndo( std::make_unique<SwUndoCopyHeaderFooter>(*this, *pSttNd, rDestFormat.GetName()) ); + } + SwPaM const source(aRg.aStart, aRg.aEnd); SwPosition dest(*pSttNd); sw::CopyBookmarks(source, dest); diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx index 7cbf440c4b10..7b83049455fe 100644 --- a/sw/source/core/undo/undobj.cxx +++ b/sw/source/core/undo/undobj.cxx @@ -697,6 +697,9 @@ OUString GetUndoComment(SwUndoId eId) case SwUndoId::REINSTATE_REDLINE: pId = STR_REINSTATE_REDLINE; break; + case SwUndoId::COPY_HEADER_FOOTER: + pId = STR_UNDO_COPY_HEADER_FOOTER; + break; } assert(pId); diff --git a/sw/source/core/undo/untblk.cxx b/sw/source/core/undo/untblk.cxx index d2900e225c12..b941720a40a3 100644 --- a/sw/source/core/undo/untblk.cxx +++ b/sw/source/core/undo/untblk.cxx @@ -34,6 +34,9 @@ #include <rolbck.hxx> #include <redline.hxx> #include <frameformats.hxx> +#include <fmtcntnt.hxx> +#include <fmthdft.hxx> + namespace sw { @@ -487,4 +490,104 @@ SwUndoCpyDoc::SwUndoCpyDoc( const SwPaM& rPam ) { } +SwUndoCopyHeaderFooter::SwUndoCopyHeaderFooter(SwDoc& rDoc, SwNode& rSttNd, const UIName& rFmtName) + : SwUndo(SwUndoId::COPY_HEADER_FOOTER, rDoc) + , m_aOff(rSttNd.GetIndex()) + , m_aFmtName(rFmtName) +{ +} + +static const SfxPoolItem* lcl_itemForFormat(SwFrameFormat& rFmt, bool bIsHeader) +{ + sal_uInt16 nAttr = bIsHeader ? sal_uInt16(RES_HEADER) : sal_uInt16(RES_FOOTER); + const SfxPoolItem* pItem; + if (SfxItemState::SET != rFmt.GetAttrSet().GetItemState(nAttr, false, &pItem)) + return nullptr; + + return pItem; +} + +static bool lcl_checkMatchesFormat(SwFrameFormat& rFmt, bool bIsHeader, SwNodeOffset aOff) +{ + const SfxPoolItem* pItem = lcl_itemForFormat(rFmt, bIsHeader); + if (!pItem) + return false; + + const SwFrameFormat* pHdFtFormat = bIsHeader + ? (pItem->StaticWhichCast(RES_HEADER).GetHeaderFormat()) + : (pItem->StaticWhichCast(RES_FOOTER).GetFooterFormat()); + if (!pHdFtFormat) + return false; + + const SwFormatContent& rContent = pHdFtFormat->GetAttrSet().GetContent(false); + + const SwNodeIndex *pFmtIdx = rContent.GetContentIdx(); + if (!pFmtIdx) + return false; + + return pFmtIdx->GetIndex() == aOff; +} + +void SwUndoCopyHeaderFooter::UndoImpl(::sw::UndoRedoContext& rContext) +{ + SwDoc& rDoc = rContext.GetDoc(); + + SwPageDesc* pPgDesc = rDoc.FindPageDesc(m_aFmtName); + + // We have to initialize these here because the PageDesc doesn't have the content of its + // frames set at the time that this class is constructed. + + m_bIsMaster = lcl_checkMatchesFormat(pPgDesc->GetMaster(), true, m_aOff); + m_bIsLeft = lcl_checkMatchesFormat(pPgDesc->GetLeft(), true, m_aOff); + m_bIsFirstMaster = lcl_checkMatchesFormat(pPgDesc->GetFirstMaster(), true, m_aOff); + m_bIsFirstLeft = lcl_checkMatchesFormat(pPgDesc->GetFirstLeft(), true, m_aOff); + + m_bIsHeader = m_bIsMaster || m_bIsLeft || m_bIsFirstMaster || m_bIsFirstLeft; + + if (!m_bIsHeader) + { + m_bIsMaster = lcl_checkMatchesFormat(pPgDesc->GetMaster(), false, m_aOff); + m_bIsLeft = lcl_checkMatchesFormat(pPgDesc->GetLeft(), false, m_aOff); + m_bIsFirstMaster = lcl_checkMatchesFormat(pPgDesc->GetFirstMaster(), false, m_aOff); + m_bIsFirstLeft = lcl_checkMatchesFormat(pPgDesc->GetFirstLeft(), false, m_aOff); + } + + SaveSection(SwNodeIndex(rDoc.GetNodes(), m_aOff)); +} + +void SwUndoCopyHeaderFooter::RedoImpl(::sw::UndoRedoContext& rContext) +{ + SwDoc& rDoc = rContext.GetDoc(); + + if (m_bIsHeader) + RestoreSection(rDoc, nullptr, SwHeaderStartNode); + else + RestoreSection(rDoc, nullptr, SwFooterStartNode); + + SwPageDesc* pPgDesc = rDoc.FindPageDesc(m_aFmtName); + + const SfxPoolItem* pItem = nullptr; + if (m_bIsMaster) + pItem = lcl_itemForFormat(pPgDesc->GetMaster(), m_bIsHeader); + else if (m_bIsLeft) + pItem = lcl_itemForFormat(pPgDesc->GetLeft(), m_bIsHeader); + else if (m_bIsFirstMaster) + pItem = lcl_itemForFormat(pPgDesc->GetFirstMaster(), m_bIsHeader); + else if (m_bIsFirstLeft) + pItem = lcl_itemForFormat(pPgDesc->GetFirstLeft(), m_bIsHeader); + + assert(pItem); + + SwFrameFormat* pHdFtFormat = const_cast<SwFrameFormat*>( + m_bIsHeader + ? (pItem->StaticWhichCast(RES_HEADER).GetHeaderFormat()) + : (pItem->StaticWhichCast(RES_FOOTER).GetFooterFormat()) + ); + + assert(pHdFtFormat); + + if (pHdFtFormat->GetAttrSet().GetItemIfSet(RES_CNTNT, false)) + pHdFtFormat->SetFormatAttr(SwFormatContent(static_cast<SwStartNode*>(rDoc.GetNodes()[m_aOff]))); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
