ODFstudio/odfstudio
2
0
Fork 0
Code Issues Packages Activity

tdf#171068 tdf#159193 Fix crash on undo after paste page desc into new doc

Browse Source 
Followup to tdf#148703 to fix issue discovered in tdf#159193
and tracked in tdf#171068

(I have verified that this patch fixes the original issue of
 tdf#159193 around page orientation as well as the crash
 observed in tdf#171068)

My patch in 68e1db614c missed
two issues related to stashed frame formats:

1. Stashed frame formats used the UIName "CopyDesc" which
   broke the logic to find the associated page desc.
2. SwUndoCopyHeaderFooter didn't handle saving/restoring
   stashed frame formats.

This was missed by my testing because I never tested copying
a page desc that included stashed frame formats.

For (1), I believe that it is more correct for frame formats
to match the UIName of their containing page desc. The
SwPageDesc constructor does that, for example.

Test plan:

I added a regression test covering the stashed frame format
case:

CPPUNIT_TEST_NAME="SwCoreUndoTest testTdf171068CopyPageDescWithStashedHeaderAcrossDocuments" make CppunitTest_sw_core_undo

I have also manually tested against the example doc from this
comment:

https://bugs.documentfoundation.org/show_bug.cgi?id=159193#c0

Change-Id: I7671e531293731c31846ffe26aa851f0ccfa7abb
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/207604
Tested-by: Jenkins
Reviewed-by: Hossein <hossein@libreoffice.org>
This commit is contained in:
David Hashe
2026-07-22 14:40:57 +02:00
committed by Hossein
parent 0b054a016a
commit f89ca556d4
4 changed files with 144 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sw/inc/undobj.hxx
+1
View File
@@ -309,6 +309,7 @@ class SwUndoCopyHeaderFooter final : public SwUndo, private SwUndoSaveSection
bool m_bIsLeft;
bool m_bIsFirstMaster;
bool m_bIsFirstLeft;
bool m_bIsStashed;
public:
SwUndoCopyHeaderFooter(const SwDoc& rDoc, const SwNode& rSttNd, const UIName& rFmtName);
sw/qa/core/undo/undo.cxx
+72
View File
@@ -9,6 +9,9 @@
#include <swmodeltestbase.hxx>
#include <com/sun/star/style/BreakType.hpp>
#include <com/sun/star/text/ControlCharacter.hpp>
#include <comphelper/propertyvalue.hxx>
#include <comphelper/sequenceashashmap.hxx>
#include <unotxdoc.hxx>
@@ -18,6 +21,7 @@
#include <frameformats.hxx>
#include <fmtcntnt.hxx>
#include <cntfrm.hxx>
#include <fmthdft.hxx>
#include <fmtpdsc.hxx>
#include <pagefrm.hxx>
#include <rootfrm.hxx>
@@ -339,6 +343,74 @@ CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testPageDescCreate)
const_cast<const SwPageDesc*>(pNewPageDesc2));
}
CPPUNIT_TEST_FIXTURE(SwCoreUndoTest, testTdf171068CopyPageDescWithStashedHeaderAcrossDocuments)
{
createSwDoc();
SwDoc* pSrcDocument = getSwDoc();
uno::Reference<lang::XComponent> xSrcComponent = mxComponent;
mxComponent.clear();
createSwDoc();
uno::Reference<lang::XComponent> xDstComponent = mxComponent;
mxComponent.clear();
// Build the source document, which will be a two page document with one paragraph
// on each page, separated by a page break that sets the page style of the second
// page to a custom page Style "SourceStyle", which includes a stashed header;
// Create the page style and attach the stashed header
{
SwPageDesc* pSrcPageDesc = pSrcDocument->MakePageDesc(UIName(u"SourceStyle"_ustr));
pSrcPageDesc->ChgFirstShare(false);
CPPUNIT_ASSERT(!pSrcPageDesc->IsFirstShared());
SwFrameFormat& rFirstMaster = pSrcPageDesc->GetFirstMaster();
rFirstMaster.SetFormatAttr(SwFormatHeader(true));
pSrcPageDesc->StashFrameFormat(pSrcPageDesc->GetFirstMaster(), true, false, true);
pSrcDocument->ChgPageDesc(UIName(u"SourceStyle"_ustr), *pSrcPageDesc);
CPPUNIT_ASSERT(pSrcPageDesc->HasStashedFormat(true, false, true));
}
// Insert the paragraphs and the page break which sets the page style for page 2
{
uno::Reference<text::XTextDocument> xTextDoc(xSrcComponent, uno::UNO_QUERY);
uno::Reference<text::XText> xText = xTextDoc->getText();
uno::Reference<text::XTextCursor> xCursor = xText->createTextCursor();
xText->insertString(xCursor, u"First page"_ustr, false);
xText->insertControlCharacter(xCursor, css::text::ControlCharacter::PARAGRAPH_BREAK, false);
uno::Reference<beans::XPropertySet> xProps(xCursor, uno::UNO_QUERY);
xProps->setPropertyValue(u"BreakType"_ustr, uno::Any(style::BreakType_PAGE_BEFORE));
xProps->setPropertyValue(u"PageDescName"_ustr, uno::Any(u"SourceStyle"_ustr));
xText->insertString(xCursor, u"Second page"_ustr, false);
}
dispatchCommand(xSrcComponent, u".uno:SelectAll"_ustr, {});
dispatchCommand(xSrcComponent, u".uno:Copy"_ustr, {});
dispatchCommand(xDstComponent, u".uno:SelectAll"_ustr, {});
dispatchCommand(xDstComponent, u".uno:Paste"_ustr, {});
dispatchCommand(xDstComponent, u".uno:Undo"_ustr, {});
// Without the fix, undoing the copy of the page desc failed
// because the page desc could not be located on the dest
// document, because the frame format was not named the same
// as the page desc.
// Also, the stashed frame format was not correctly saved
// to the undo nodes, which led to corruption of the
// node offsets similar to tdf#148703.
dispatchCommand(xDstComponent, u".uno:Redo"_ustr, {});
dispatchCommand(xDstComponent, u".uno:Undo"_ustr, {});
xSrcComponent->dispose();
xDstComponent->dispose();
}
CPPUNIT_PLUGIN_IMPLEMENT();
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
sw/source/core/doc/docfmt.cxx
+5 -1
View File
@@ -1573,7 +1573,11 @@ void SwDoc::CopyPageDesc( const SwPageDesc& rSrcDesc, SwPageDesc& rDstDesc,
{
if (&pStashedFormatSrc->GetDoc() != this)
{
SwFrameFormat newFormat(GetAttrPool(), UIName(u"CopyDesc"_ustr), GetDfltFrameFormat());
OSL_ENSURE(pStashedFormatSrc->GetName() == rSrcDesc.GetName(),
"stashed SwFrameFormat UIName differs from containing "
"SwPageDesc UIName");
SwFrameFormat newFormat(GetAttrPool(), rSrcDesc.GetName(),
GetDfltFrameFormat());
SfxItemSet aAttrSet(pStashedFormatSrc->GetAttrSet());
aAttrSet.ClearItem(RES_HEADER);
sw/source/core/undo/untblk.cxx
+66 -9
View File
@@ -497,6 +497,7 @@ SwUndoCopyHeaderFooter::SwUndoCopyHeaderFooter(const SwDoc& rDoc, const SwNode&
, m_bIsLeft(false)
, m_bIsFirstMaster(false)
, m_bIsFirstLeft(false)
, m_bIsStashed(false)
{
}
@@ -555,6 +556,52 @@ void SwUndoCopyHeaderFooter::UndoImpl(::sw::UndoRedoContext& rContext)
m_bIsFirstLeft = lcl_checkMatchesFormat(pPgDesc->GetFirstLeft(), false, m_aOff);
}
bool bFound = m_bIsMaster || m_bIsLeft || m_bIsFirstMaster || m_bIsFirstLeft;
if (!bFound)
{
// This m_aOff is not used on any of the active frames on the PageDesc.
// Check whether it is used on one of its stashed frame formats.
// { bFirst, bLeft, bHeader }
std::vector<std::tuple<bool, bool, bool>> vHdftLocs = {
{true, true, true},
{true, true, false},
{true, false, true},
{true, false, false},
{false, true, true},
{false, true, false},
};
auto it = std::ranges::find_if(vHdftLocs, [&](const auto& aHdftLoc) {
const auto& [bFirst, bLeft, bHeader] = aHdftLoc;
if (auto pStashedFormat
= pPgDesc->GetStashedFrameFormat(bHeader, bLeft, bFirst))
{
return lcl_checkMatchesFormat(*pStashedFormat, bHeader, m_aOff);
}
return false;
});
if (it != vHdftLocs.end())
{
const auto& [bFirst, bLeft, bHeader] = *it;
m_bIsHeader = bHeader;
m_bIsMaster = !bFirst && !bLeft;
m_bIsLeft = !bFirst && bLeft;
m_bIsFirstMaster = bFirst && !bLeft;
m_bIsFirstLeft = bFirst && bLeft;
m_bIsStashed = true;
bFound = true;
}
}
OSL_ENSURE(bFound,
"Saving header/footer content that we can't associate with any SwFrameFormat on its "
"SwPageDesc");
SaveSection(SwNodeIndex(rDoc.GetNodes(), m_aOff));
}
@@ -570,15 +617,25 @@ void SwUndoCopyHeaderFooter::RedoImpl(::sw::UndoRedoContext& rContext)
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);
if (m_bIsStashed)
{
bool bLeft = m_bIsLeft || m_bIsFirstLeft;
bool bFirst = m_bIsFirstMaster || m_bIsFirstLeft;
const SwFrameFormat* pFrmFmt = pPgDesc->GetStashedFrameFormat(m_bIsHeader, bLeft, bFirst);
if (pFrmFmt)
pItem = lcl_itemForFormat(*pFrmFmt, m_bIsHeader);
}
else
{
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*>(