sw/qa/core/draw/data/sdt-textbox-header.docx |binary sw/qa/core/draw/draw.cxx | 11 +++++++ sw/source/core/draw/dcontact.cxx | 41 +++++++++++++++++++++++++++ sw/source/core/draw/dflyobj.cxx | 13 ++++++++ sw/source/core/inc/dflyobj.hxx | 1 5 files changed, 66 insertions(+)
New commits: commit 914d246e0edcbdfa5969b7f3eea7c67e1a5b7522 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Wed Sep 28 08:54:06 2022 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Wed Sep 28 09:46:30 2022 +0200 crashtesting: fix PDF export of fdo83057-2.docx Loading this document and laying it out resulted in an assertion failure since commit de90c192cb8f1f03a4028493d8bfe9a127a76b2a (sw content controls, plain text: enable DOCX filter with data binding, 2022-09-19). Writer has two relevant invariants: First, if a draw shape is anchored into a header, then the "master" shape (typically on the first page) and its "virtual" shapes (on other pages) have their indexes / ZOrders set so that the master shape has the highest index. The second is that in case a draw+fly format is paired into a "textbox", then the fly index is always the draw index + 1. The assert fails because in this case the virtual draw shape's index is not smaller than the master one. What seems to happen here is that first SwDrawContact::ConnectToLayout() iterates the frames of the text node in the header, and happens to first visit page 1 and then page 2. So the master SdrObjects go to the first page, the virtual ones go to the second page. Then later SwDrawContact::ConnectToLayout() is called again, which removes the master and the virtual draw SdrObject from the layout and the virtual draw SdrObject from the model. Then it visits the text frames of the header text node again, but this time it happens to find the second page, and only then the first page (for reasons unclear to me, this didn't happen before the above commit). The result is that both invariants are failing: the master draw+fly is no longer on the same page and the indexes are also out of order, so one of the pages don't show shape text for the shape from the header. Fix the problem by two tweaks: - Improve SwDrawContact::ConnectToLayout(), so that in case it just appends the draw object to the layout for the master, then it adjusts its ZOrder so the matching fly will be the draw one + 1. This was working in the virtual case already, since there we call AddVirtObj() already, which calls SwDrawVirtObj::AddToDrawingPage(), which already knows how to maintain this invariant. This fixes master draw SdrObjects to be directly before their fly counterparts, even if the page order changes. - Improve SwDrawVirtObj::AddToDrawingPage(), so that in case the virtual draw SdrObject finds its fly, but that fly has an incorrect index (due to the changed page visit order), then we first fix up the fly and only then set the index of the virtual draw SdrObject. The result is a doc model like this: <SdrObject ptr="0x52ecd30" symbol="13SwDrawVirtObj" name="" title="" description="" nOrdNum="0" aOutRect="-16000, -16000, 0, 0"/> <SwVirtFlyDrawObj ptr="0x4e4bfa0" fly-frame="5"> <SdrObject ptr="0x4e4bfa0" symbol="16SwVirtFlyDrawObj" name="" title="" description="" nOrdNum="1" aOutRect="0, 0, 0, 0"/> <SdrObject ptr="0x4ea4300" symbol="17SdrObjCustomShape" name="Rectangle 2" title="" description="" nOrdNum="2" aOutRect="10418, 2441, 1775, 485"> <SwVirtFlyDrawObj ptr="0x52ec630" fly-frame="14"> <SdrObject ptr="0x52ec630" symbol="16SwVirtFlyDrawObj" name="" title="" description="" nOrdNum="3" aOutRect="0, 0, 0, 0"/> I.e. each draw SdrObject is followed by its fly and the masters have the highest indexes. This all stems from the behavior that 1) re-connecting a draw contact to the layout removes the draw shapes but not the flys from the layout, 2) the order of frames in an SwIterator visiting is not something we can depend on and 3) textboxes are implemented using a pair of draw+fly formats. Change-Id: Ia03f0ec70cf67626660349accc63770fe2afef5d Reviewed-on: https://gerrit.libreoffice.org/c/core/+/140688 Reviewed-by: Miklos Vajna <vmik...@collabora.com> Tested-by: Jenkins diff --git a/sw/qa/core/draw/data/sdt-textbox-header.docx b/sw/qa/core/draw/data/sdt-textbox-header.docx new file mode 100644 index 000000000000..a8b2d9625fbe Binary files /dev/null and b/sw/qa/core/draw/data/sdt-textbox-header.docx differ diff --git a/sw/qa/core/draw/draw.cxx b/sw/qa/core/draw/draw.cxx index c2514808f8e4..feb347adc8d4 100644 --- a/sw/qa/core/draw/draw.cxx +++ b/sw/qa/core/draw/draw.cxx @@ -141,6 +141,17 @@ CPPUNIT_TEST_FIXTURE(SwCoreDrawTest, testTdf107727FrameBorder) CPPUNIT_ASSERT_EQUAL(Color(0x0000ff), Color(ColorTransparency, aBorder.Color)); } +CPPUNIT_TEST_FIXTURE(SwCoreDrawTest, testSdtTextboxHeader) +{ + // Given a 2 page document, same header on both pages, content control in the header and + // shape+fly pair (textbox) anchored in the same header: + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "sdt-textbox-header.docx"; + + // When loading that document, then make sure that layout doesn't fail with an assertion because + // the "master SdrObj should have the highest index" invariant doesn't hold: + mxComponent = loadFromDesktop(aURL, "com.sun.star.text.TextDocument", {}); +} + CPPUNIT_PLUGIN_IMPLEMENT(); /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/draw/dcontact.cxx b/sw/source/core/draw/dcontact.cxx index 369c46e4ea80..fe19106949b7 100644 --- a/sw/source/core/draw/dcontact.cxx +++ b/sw/source/core/draw/dcontact.cxx @@ -1954,6 +1954,36 @@ void SwDrawContact::ConnectToLayout( const SwFormatAnchor* pAnch ) { // append 'master' drawing object pAnchorFrameOfMaster = pFrame; + + const SwFrameFormat* pFlyFormat = nullptr; + if (!maAnchoredDrawObj.GetDrawObj()->IsGroupObject()) + { + pFlyFormat = SwTextBoxHelper::getOtherTextBoxFormat(GetFormat(), RES_DRAWFRMFMT); + } + + if (pFlyFormat) + { + // This is a master draw object and it has an associated fly format. + // See if a fly frame is already inserted to the layout: if so, this + // master draw object should be ordered directly before the fly one. + if (const SwSortedObjs* pObjs = pFrame->GetDrawObjs()) + { + for (const SwAnchoredObject* pAnchoredObj : *pObjs) + { + if (&pAnchoredObj->GetFrameFormat() == pFlyFormat) + { + SdrPage* pDrawPage = pAnchoredObj->GetDrawObj()->getSdrPageFromSdrObject(); + if (pDrawPage) + { + sal_uInt32 nOrdNum = pAnchoredObj->GetDrawObj()->GetOrdNum(); + pDrawPage->SetObjectOrdNum(maAnchoredDrawObj.GetDrawObj()->GetOrdNumDirect(), nOrdNum); + break; + } + } + } + } + } + pFrame->AppendDrawObj( maAnchoredDrawObj ); } else @@ -2328,6 +2358,17 @@ void SwDrawVirtObj::AddToDrawingPage(SwFrame const& rAnchorFrame) if (&pAnchoredObj->GetFrameFormat() == pFlyFormat) { assert(dynamic_cast<SwFlyFrame const*>(pAnchoredObj)); + + if (pAnchoredObj->GetDrawObj()->GetOrdNum() >= GetReferencedObj().GetOrdNum()) + { + // This virtual draw object has an associated fly one, but the fly's index + // is not below the masters, fix it up. + if (pDrawPg) + { + pDrawPg->SetObjectOrdNum(pAnchoredObj->GetDrawObj()->GetOrdNumDirect(), GetReferencedObj().GetOrdNum()); + } + } + nOrdNum = pAnchoredObj->GetDrawObj()->GetOrdNum(); // the master SdrObj should have the highest index assert(nOrdNum < GetReferencedObj().GetOrdNum()); diff --git a/sw/source/core/draw/dflyobj.cxx b/sw/source/core/draw/dflyobj.cxx index 467df8864a7c..9e2b96ce3118 100644 --- a/sw/source/core/draw/dflyobj.cxx +++ b/sw/source/core/draw/dflyobj.cxx @@ -1302,4 +1302,17 @@ bool SwVirtFlyDrawObj::IsTextBox() const return SwTextBoxHelper::isTextBox(GetFormat(), RES_FLYFRMFMT, this); } +void SwVirtFlyDrawObj::dumpAsXml(xmlTextWriterPtr pWriter) const +{ + (void)xmlTextWriterStartElement(pWriter, BAD_CAST("SwVirtFlyDrawObj")); + (void)xmlTextWriterWriteFormatAttribute(pWriter, BAD_CAST("ptr"), "%p", this); + (void)xmlTextWriterWriteAttribute( + pWriter, BAD_CAST("fly-frame"), + BAD_CAST(OString::number(m_pFlyFrame->GetFrameId()).getStr())); + + SdrVirtObj::dumpAsXml(pWriter); + + (void)xmlTextWriterEndElement(pWriter); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/sw/source/core/inc/dflyobj.hxx b/sw/source/core/inc/dflyobj.hxx index 3858f8901850..db853d2262b2 100644 --- a/sw/source/core/inc/dflyobj.hxx +++ b/sw/source/core/inc/dflyobj.hxx @@ -139,6 +139,7 @@ public: virtual bool HasLimitedRotation() const override; virtual bool IsTextBox() const override; + void dumpAsXml(xmlTextWriterPtr pWriter) const override; }; #endif