tdf#159158 writerfilter: duplicate relativeHeights? last one wins
[When MS Word 2019 round-trips a file with a duplicate relativeHeight
it replaces them with unique relativeHeights in the export.]
My THEORY is that bOldStyle should always be true for GraphicImport,
so a follow-up commit will basically revert all of this,
but at least if my theory is wrong, I'll still have fixed this problem.
Notice that there are two things that feed into zOrder,
the other being z-Index. I assume the very explicit code that says
that (except for bOldStyle) first one wins must be the standard for z-Index.
make CppunitTest_sw_ooxmlexport18 \
CPPUNIT_TEST_NAME=testTdf159158_zOrder_duplicate
Change-Id: I842d016a7922427ce1290a7134d4e19fed52880f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161989
Tested-by: Jenkins
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_duplicate_compat15.docx b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_duplicate_compat15.docx
new file mode 100644
index 0000000..b39694f
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_duplicate_compat15.docx
Binary files differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index 0f3816a..e92994f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
@@ -868,6 +868,24 @@ DECLARE_OOXMLEXPORT_TEST(testTdf155736, "tdf155736_PageNumbers_footer.docx")
CPPUNIT_ASSERT_EQUAL(OUString("Page * of *"), parseDump("/root/page[2]/footer/txt/text()"_ostr));
}
DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_duplicate, "tdf159158_zOrder_duplicate_compat15.docx")
{
// given a yellow star with relativeHeight 2, followed by an overlapping blue star also with 2
uno::Reference<drawing::XShape> image1 = getShape(1), image2 = getShape(2);
sal_Int32 zOrder1, zOrder2;
OUString descr1, descr2;
uno::Reference<beans::XPropertySet> imageProperties1(image1, uno::UNO_QUERY);
imageProperties1->getPropertyValue( "ZOrder" ) >>= zOrder1;
imageProperties1->getPropertyValue( "Name" ) >>= descr1;
uno::Reference<beans::XPropertySet> imageProperties2(image2, uno::UNO_QUERY);
imageProperties2->getPropertyValue( "ZOrder" ) >>= zOrder2;
imageProperties2->getPropertyValue( "Name" ) >>= descr2;
CPPUNIT_ASSERT_EQUAL( sal_Int32( 0 ), zOrder1 ); // lower
CPPUNIT_ASSERT_EQUAL( sal_Int32( 1 ), zOrder2 ); // higher
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Yellow"), descr1);
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Blue"), descr2); // last one defined wins
}
DECLARE_OOXMLEXPORT_TEST(testTdf155903, "tdf155903.odt")
{
// Without the accompanying fix in place, this test would have crashed,
diff --git a/writerfilter/inc/dmapper/GraphicZOrderHelper.hxx b/writerfilter/inc/dmapper/GraphicZOrderHelper.hxx
index c6a308d..2ad71c4 100644
--- a/writerfilter/inc/dmapper/GraphicZOrderHelper.hxx
+++ b/writerfilter/inc/dmapper/GraphicZOrderHelper.hxx
@@ -19,6 +19,7 @@ public:
void addItem(css::uno::Reference<css::beans::XPropertySet> const& props,
sal_Int32 relativeHeight);
sal_Int32 findZOrder(sal_Int32 relativeHeight, bool bOldStyle = false);
bool hasZOrder(sal_Int32 relativeHeight) { return m_items.count(relativeHeight) != 0; }
private:
using Items = std::map<sal_Int32, css::uno::Reference<css::beans::XPropertySet>>;
diff --git a/writerfilter/source/dmapper/GraphicHelpers.cxx b/writerfilter/source/dmapper/GraphicHelpers.cxx
index 67739d4..d01ac5b 100644
--- a/writerfilter/source/dmapper/GraphicHelpers.cxx
+++ b/writerfilter/source/dmapper/GraphicHelpers.cxx
@@ -278,6 +278,10 @@ void GraphicZOrderHelper::addItem(uno::Reference<beans::XPropertySet> const& pro
// The relativeHeight value in .docx is an arbitrary number, where only the relative ordering matters.
// But in Writer, the z-order is index in 0..(numitems-1) range, so whenever a new item needs to be
// added in the proper z-order, it is necessary to find the proper index.
// The key to this function is that later on, when setPropertyValue("ZOrder", <returned sal_Int32>),
// SW also automatically increments ALL zOrders >= the one returned for this fly.
// Thus, getProperty PROP_Z_ORDER for relativeHeight "x" can return different values for itemZOrder.
sal_Int32 GraphicZOrderHelper::findZOrder( sal_Int32 relativeHeight, bool bOldStyle )
{
// std::map is iterated sorted by key
@@ -292,7 +296,7 @@ sal_Int32 GraphicZOrderHelper::findZOrder( sal_Int32 relativeHeight, bool bOldSt
if( it == m_items.end()) // we're topmost
{
if( m_items.empty())
return 0;
return 0; // the lowest
--it;
itemZOrderOffset = 1; // after the topmost
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 4f0fd03..6fbc172 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -735,7 +735,17 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
m_pImpl->m_bUseSimplePos = nIntValue > 0;
break;
case NS_ooxml::LN_CT_Anchor_relativeHeight:
{
m_pImpl->m_zOrder = nIntValue;
// Last one defined must win - opposite to what the existing code (for z-Index?) does.
GraphicZOrderHelper* pZOrderHelper = m_pImpl->m_rDomainMapper.graphicZOrderHelper();
if (pZOrderHelper->hasZOrder(m_pImpl->m_zOrder)
&& m_pImpl->m_rGraphicImportType != GraphicImportType::IMPORT_AS_DETECTED_INLINE)
{
++m_pImpl->m_zOrder;
}
}
break;
case NS_ooxml::LN_CT_Anchor_behindDoc:
if (nIntValue > 0)
