sw/qa/extras/odfexport/data/z-index-header.odt |binary sw/qa/extras/odfexport/odfexport2.cxx | 13 +++++++ sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 43 ++++++++++++++++++------- xmloff/source/draw/shapeimport.cxx | 29 ++++++++++++---- 4 files changed, 66 insertions(+), 19 deletions(-)
New commits: commit 3c131592416c96ab08257429a9272f550215ca1c Author: Michael Stahl <[email protected]> AuthorDate: Fri Jan 16 18:46:51 2026 +0100 Commit: Stephan Bergmann <[email protected]> CommitDate: Wed Jan 21 21:07: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]> (cherry picked from commit 66a1ad2163d5799a79b23554b22e08a07f693225) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/197728 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 7bd37ed31582..0a830c4afaa1 100644 --- a/sw/qa/extras/odfexport/odfexport2.cxx +++ b/sw/qa/extras/odfexport/odfexport2.cxx @@ -637,6 +637,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 bce66556cbef..f5e0b536e6cf 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx @@ -1121,19 +1121,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(TestFilter::DOCX); + + { + // 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 fcd12a711194..5a8f135dddf5 100644 --- a/xmloff/source/draw/shapeimport.cxx +++ b/xmloff/source/draw/shapeimport.cxx @@ -620,8 +620,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()); @@ -633,27 +637,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 @@ -673,6 +687,7 @@ void ShapeGroupContext::popGroupAndPostProcess() nIndex++; } + maUnsortedList.clear(); // it's not necessary to move remaining elements maZOrderList.clear(); } @@ -723,7 +738,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);
