sw/qa/extras/layout/layout5.cxx | 52 +++++++++++++++++-------- sw/qa/extras/odfexport/data/z-index-header.odt |binary sw/qa/extras/odfexport/odfexport2.cxx | 13 ++++++ sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 43 ++++++++++++++------ sw/source/core/doc/textboxhelper.cxx | 8 +-- xmloff/qa/unit/draw.cxx | 2 xmloff/source/draw/shapeimport.cxx | 29 ++++++++++--- 7 files changed, 107 insertions(+), 40 deletions(-)
New commits: commit 66a1ad2163d5799a79b23554b22e08a07f693225 Author: Michael Stahl <[email protected]> AuthorDate: Fri Jan 16 18:46:51 2026 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Wed Jan 21 11:04:00 2026 +0100 xmloff: ODF import: fix z-index rotation in header/footer ShapeGroupContext::popGroupAndPostProcess() is called 2 times when importing an ODF package file, once at the end of styles.xml, then at the end of content.xml; in the 2nd case, all shapes of styles.xml end up in maUnsortedList. There are 2 problems with it: 1. in the XShapes3 branch, once maZOrderList is empty, any remaining elements in maUnsortedList are ignored and thus the aNewOrder contains duplicate 0s at the end which will cause sort() to throw 2. the XShapes3 branch removes elements from maUnsortedList, hence when the exception is caught and the fallback path runs, it only moves the elements that happened to remain in maUnsortedList Effectively, what happens is that the z-order of everything in headers/footers is rotated so that the last (topmost) N elements go to the start (background). 3. In case the objects in content.xml happen to be sorted correctly wrt each other, but there was an object with higher z-index in styles.xml, sorting is skipped. Turns out that testTdf159158_zOrder_headerBehind already tests this. (regression from commit a8b1699ca9c7e8c43eff79467451fd1fcb4fde9b) Change-Id: Ifdde6fc43af4c655b430d723b875dfd46e8533f2 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197475 Tested-by: Jenkins CollaboraOffice <[email protected]> Reviewed-by: Stephan Bergmann <[email protected]> diff --git a/sw/qa/extras/odfexport/data/z-index-header.odt b/sw/qa/extras/odfexport/data/z-index-header.odt new file mode 100644 index 000000000000..e5f9a22992d5 Binary files /dev/null and b/sw/qa/extras/odfexport/data/z-index-header.odt differ diff --git a/sw/qa/extras/odfexport/odfexport2.cxx b/sw/qa/extras/odfexport/odfexport2.cxx index b7da0cc96ed1..7e466cef359f 100644 --- a/sw/qa/extras/odfexport/odfexport2.cxx +++ b/sw/qa/extras/odfexport/odfexport2.cxx @@ -608,6 +608,19 @@ DECLARE_ODFEXPORT_TEST(tdf149420, "tdf149420.odt") CPPUNIT_ASSERT_EQUAL(sal_uInt16(567), getProperty<sal_uInt16>(getParagraph(4), u"ParaHyphenationZone"_ustr)); } +DECLARE_ODFEXPORT_TEST(zIndexHeader, "z-index-header.odt") +{ + uno::Reference<beans::XPropertySet> xOrder0(getShape(1), uno::UNO_QUERY_THROW); + uno::Reference<beans::XPropertySet> xOrder1(getShape(2), uno::UNO_QUERY_THROW); + uno::Reference<beans::XPropertySet> xOrder2(getShape(3), uno::UNO_QUERY_THROW); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(xOrder0, u"ZOrder"_ustr)); + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(xOrder1, u"ZOrder"_ustr)); + CPPUNIT_ASSERT_EQUAL(sal_Int32(2), getProperty<sal_Int32>(xOrder2, u"ZOrder"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"HeaderFrameIndex0"_ustr, uno::Reference<container::XNamed>(xOrder0, uno::UNO_QUERY_THROW)->getName()); + CPPUNIT_ASSERT_EQUAL(u"BodyFrameIndex1"_ustr, uno::Reference<container::XNamed>(xOrder1, uno::UNO_QUERY_THROW)->getName()); + CPPUNIT_ASSERT_EQUAL(u"HeaderFrameIndex2"_ustr, uno::Reference<container::XNamed>(xOrder2, uno::UNO_QUERY_THROW)->getName()); +} + DECLARE_ODFEXPORT_TEST(testArabicZeroNumbering, "arabic-zero-numbering.odt") { CPPUNIT_ASSERT_EQUAL(1, getPages()); diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx index 65f78c7779d4..0797db5773ea 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx @@ -1076,19 +1076,38 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf159158_zOrder_behindDocB) verify(/*bIsExport*/ true); } -DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_headerBehind, "tdf159158_zOrder_headerBehind.odt") +CPPUNIT_TEST_FIXTURE(Test, testTdf159158_zOrder_headerBehind) { - // given a blue star (not marked as behind text) anchored in the header - // and an overlapping yellow rectangle anchored in the body text. - // (note that in ODT format the star is on top, but for DOCX format it must be behind (hidden) - uno::Reference<beans::XPropertySet> zOrder0(getShape(1), uno::UNO_QUERY); - uno::Reference<beans::XPropertySet> zOrder1(getShape(2), uno::UNO_QUERY); - CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(zOrder0, u"ZOrder"_ustr)); // lower - CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(zOrder1, u"ZOrder"_ustr)); // higher - // I don't know why the star is the lowest order in ODT import (maybe header weirdness), - // but it certainly needs to be the lowest on docx round-trip (also for header weirdness) - CPPUNIT_ASSERT_EQUAL(u"StarInHeader"_ustr, getProperty<OUString>(zOrder0, u"Name"_ustr)); - CPPUNIT_ASSERT_EQUAL(u"RectangleInBody"_ustr, getProperty<OUString>(zOrder1,u"Name"_ustr)); + createSwDoc("tdf159158_zOrder_headerBehind.odt"); + + { + // given a blue star (not marked as behind text) anchored in the header + // and an overlapping yellow rectangle anchored in the body text. + // (note that in ODT format the star is on top, but for DOCX format it must be behind (hidden) + uno::Reference<beans::XPropertySet> zOrder0(getShape(1), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> zOrder1(getShape(2), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(zOrder0, u"ZOrder"_ustr)); // lower + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(zOrder1, u"ZOrder"_ustr)); // higher + CPPUNIT_ASSERT_EQUAL(u"RectangleInBody"_ustr, getProperty<OUString>(zOrder0, u"Name"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"StarInHeader"_ustr, getProperty<OUString>(zOrder1, u"Name"_ustr)); + } + + maTempFile.EnableKillingFile(false); + saveAndReload(mpFilter); + + { + // given a blue star (not marked as behind text) anchored in the header + // and an overlapping yellow rectangle anchored in the body text. + // (note that in ODT format the star is on top, but for DOCX format it must be behind (hidden) + uno::Reference<beans::XPropertySet> zOrder0(getShape(1), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> zOrder1(getShape(2), uno::UNO_QUERY); + CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(zOrder0, u"ZOrder"_ustr)); // lower + CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(zOrder1, u"ZOrder"_ustr)); // higher + CPPUNIT_ASSERT_EQUAL(u"StarInHeader"_ustr, getProperty<OUString>(zOrder0, u"Name"_ustr)); + CPPUNIT_ASSERT_EQUAL(u"RectangleInBody"_ustr, getProperty<OUString>(zOrder1, u"Name"_ustr)); + } + + maTempFile.EnableKillingFile(); } DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_headerBehind2, "tdf159158_zOrder_headerBehind2.docx") diff --git a/xmloff/source/draw/shapeimport.cxx b/xmloff/source/draw/shapeimport.cxx index fe251856a0e4..f8fe35258311 100644 --- a/xmloff/source/draw/shapeimport.cxx +++ b/xmloff/source/draw/shapeimport.cxx @@ -621,8 +621,12 @@ void ShapeGroupContext::popGroupAndPostProcess() [](const ZOrderHint& rLeft, const ZOrderHint& rRight) { return rLeft.nShould < rRight.nShould; } ); - if (bSorted) + if (maZOrderList.empty() || (bSorted + // check that no content.xml shape goes before last styles.xml shape + && maUnsortedList.size() <= o3tl::make_unsigned(maZOrderList.front().nShould))) + { return; // nothin' to do + } // sort z-ordered shapes by nShould field std::sort(maZOrderList.begin(), maZOrderList.end()); @@ -634,27 +638,37 @@ void ShapeGroupContext::popGroupAndPostProcess() auto pNewOrder = aNewOrder.getArray(); sal_Int32 nIndex = 0; + auto itU{maUnsortedList.begin()}; for (const ZOrderHint& rHint : maZOrderList) { // fill in the gaps from unordered list - for (std::vector<ZOrderHint>::iterator aIt = maUnsortedList.begin(); aIt != maUnsortedList.end() && nIndex < rHint.nShould; ) + for (; itU != maUnsortedList.end() && nIndex < rHint.nShould; ++itU) { - pNewOrder[nIndex++] = (*aIt).nIs; - aIt = maUnsortedList.erase(aIt); + pNewOrder[nIndex] = (*itU).nIs; + ++nIndex; } pNewOrder[nIndex] = rHint.nIs; nIndex++; } + for (; itU != maUnsortedList.end(); ++itU) + { + pNewOrder[nIndex] = (*itU).nIs; + ++nIndex; + } + assert(nIndex == aNewOrder.getLength()); try { xShapes3->sort(aNewOrder); + maUnsortedList.clear(); maZOrderList.clear(); return; } - catch (const css::lang::IllegalArgumentException& /*e*/) - {} + catch (const css::lang::IllegalArgumentException&) + { + TOOLS_WARN_EXCEPTION("xmloff.draw", "XShapes3::sort() throws: "); + } } // this is the current index, all shapes before that @@ -674,6 +688,7 @@ void ShapeGroupContext::popGroupAndPostProcess() nIndex++; } + maUnsortedList.clear(); // it's not necessary to move remaining elements maZOrderList.clear(); } @@ -724,7 +739,7 @@ void XMLShapeImportHelper::shapeWithZIndexAdded( css::uno::Reference< css::drawi aNewHint.nShould = nZIndex; aNewHint.pShape = xShape.get(); - if( nZIndex == -1 ) + if (nZIndex < 0) // not set or invalid value { // don't care, so add to unsorted list mpImpl->mpGroupContext->maUnsortedList.push_back(aNewHint); commit 327e92c78603e011e7ddf8b57749dfd9e690addf Author: Michael Stahl <[email protected]> AuthorDate: Mon Jan 19 16:16:15 2026 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Wed Jan 21 11:03:53 2026 +0100 sw: fix setting mbIsTextBox on shapes' text boxes SetTextBoxes(true) is never called because it checks the shape's SdrObject, while previously this code was in SwFrameFormat::SetOtherTextBoxFormat() where it would be called a second time for the text box SwFrameFormat which has the SwFlyDrawObj. SwTextBoxHelper::getCount() can't ignore the text boxes if the flag is never set; this ends up confusing ShapeGroupContext::popGroupAndPostProcess() (and causes tests like testTdf84695 to fail with the fixes in the next commit). Because shapes with text boxes are sorted again with this, testTdf146272 and testTextBoxLoss need adapting. (regression from commit 504d78acb866495fd954fcd6db22ea68f174a5ab) Change-Id: I80c375d900a10f1956cd112d60c3f25332c485ac Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197594 Reviewed-by: Michael Stahl <[email protected]> Tested-by: Jenkins CollaboraOffice <[email protected]> diff --git a/sw/qa/extras/layout/layout5.cxx b/sw/qa/extras/layout/layout5.cxx index 31fc9099f545..b0cde5f76107 100644 --- a/sw/qa/extras/layout/layout5.cxx +++ b/sw/qa/extras/layout/layout5.cxx @@ -61,21 +61,43 @@ CPPUNIT_TEST_FIXTURE(SwLayoutWriter5, testTdf146272) { createSwDoc("tdf146272.odt"); - uno::Reference<beans::XPropertySet> xPicture(getShape(2), uno::UNO_QUERY); - uno::Reference<beans::XPropertySet> xDrawing(getShape(1), uno::UNO_QUERY); - uno::Reference<beans::XPropertySet> xFrame(xDrawing->getPropertyValue(u"TextBoxContent"_ustr), - uno::UNO_QUERY); - - CPPUNIT_ASSERT(xPicture); - CPPUNIT_ASSERT(xDrawing); - CPPUNIT_ASSERT(xFrame); - - const sal_uInt64 nPitureZorder = xPicture->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); - const sal_uInt64 nDrawingZorder = xDrawing->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); - const sal_uInt64 nFrameZorder = xFrame->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); - - CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nDrawingZorder < nFrameZorder); - CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nFrameZorder < nPitureZorder); + uno::Reference<beans::XPropertySet> xPicture1(getShapeByName(u"graphics1"), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xDrawing1(getShapeByName(u"Shape2"), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xFrame1(xDrawing1->getPropertyValue(u"TextBoxContent"_ustr), + uno::UNO_QUERY); + + CPPUNIT_ASSERT(xPicture1); + CPPUNIT_ASSERT(xDrawing1); + CPPUNIT_ASSERT(xFrame1); + + const sal_uInt64 nPicture1Zorder + = xPicture1->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + const sal_uInt64 nDrawing1Zorder + = xDrawing1->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + const sal_uInt64 nFrame1Zorder = xFrame1->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nDrawing1Zorder < nFrame1Zorder); + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nFrame1Zorder < nPicture1Zorder); + + uno::Reference<beans::XPropertySet> xPicture2(getShapeByName(u"Image423"), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xDrawing2(getShapeByName(u"Shape3"), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xFrame2(xDrawing2->getPropertyValue(u"TextBoxContent"_ustr), + uno::UNO_QUERY); + + CPPUNIT_ASSERT(xPicture2); + CPPUNIT_ASSERT(xDrawing2); + CPPUNIT_ASSERT(xFrame2); + + const sal_uInt64 nPicture2Zorder + = xPicture2->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + const sal_uInt64 nDrawing2Zorder + = xDrawing2->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + const sal_uInt64 nFrame2Zorder = xFrame2->getPropertyValue(u"ZOrder"_ustr).get<sal_uInt64>(); + + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nDrawing2Zorder < nFrame2Zorder); + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nFrame2Zorder < nPicture2Zorder); + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nDrawing1Zorder < nDrawing2Zorder); + CPPUNIT_ASSERT_MESSAGE("Bad ZOrder!", nDrawing2Zorder < nPicture1Zorder); } CPPUNIT_TEST_FIXTURE(SwLayoutWriter5, testTdf138773) diff --git a/sw/source/core/doc/textboxhelper.cxx b/sw/source/core/doc/textboxhelper.cxx index 653bc2849624..70f89bcedb3b 100644 --- a/sw/source/core/doc/textboxhelper.cxx +++ b/sw/source/core/doc/textboxhelper.cxx @@ -1808,11 +1808,9 @@ void SwTextBoxNode::AddTextBox(SdrObject* pDrawObject, SwFrameFormat* pNewTextBo } } - auto pSwFlyDraw = dynamic_cast<SwFlyDrawObj*>(pDrawObject); - if (pSwFlyDraw) - { - pSwFlyDraw->SetTextBox(true); - } + SdrObject* const pFlyObject{ pNewTextBox->FindSdrObject() }; + assert(dynamic_cast<SwFlyDrawObj*>(pFlyObject)); + static_cast<SwFlyDrawObj*>(pFlyObject)->SetTextBox(true); m_pTextBoxes.push_back(aElem); } diff --git a/xmloff/qa/unit/draw.cxx b/xmloff/qa/unit/draw.cxx index e356e95d00a2..265b021430cf 100644 --- a/xmloff/qa/unit/draw.cxx +++ b/xmloff/qa/unit/draw.cxx @@ -98,7 +98,7 @@ CPPUNIT_TEST_FIXTURE(XmloffDrawTest, testTextBoxLoss) // Make sure that the shape is still a textbox. uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(mxComponent, uno::UNO_QUERY); uno::Reference<drawing::XDrawPage> xDrawPage = xDrawPageSupplier->getDrawPage(); - uno::Reference<beans::XPropertySet> xShape(xDrawPage->getByIndex(1), uno::UNO_QUERY); + uno::Reference<beans::XPropertySet> xShape(xDrawPage->getByIndex(0), uno::UNO_QUERY); bool bTextBox = false; xShape->getPropertyValue(u"TextBox"_ustr) >>= bTextBox;
