tdf#159158 tdf#120760 DOCX shape import: fix Z-order with behindDoc #2
This fixes documents where both a z-index and a relativeHeight are in
the hell-layer. z-indexes indicate behindDoc with a negative number,
so prior to this patch relativeHeights were always above z-indexes,
but in fact the reverse ought to be true.
The reason why this works so well is because relativeHeight
can be at maximum 1DFF FFFF, and we are subtracting 7FFF FFFF
which makes it a very low number that z-index is very
unlikely to ever reach.
I don't see any reason at all why this logic would be limited
to headers and footers. I assume that was done just to limit
the potential regression hate, but in that case a comment
should have been made stating that.
documentation: previous patchsets contain a list of unit tests
that have behindDoc without being in the HeaderFooter.
None were interesting...
The unit tests I modified were generally not surprising,
since I am changing the zOrder and getShapes() is based on zOrder,
so either refer to the shape by name when the order is not important
(and might change depending on ODT import, no z-index defined etc),
or else change the number to match the moved-to location
if it changed because of initial import.
The interesting one was the compat15 document which is
specified as behindDoc but that is supposed to be ignored...
perhaps if() could have just used m_bOpaque, but that is a logic
step a bit too far to take.
This can (and should) affect RTF as well, since
{\sn fBehindDocument}{\sv 1}
is documented as a shape consideration in the RTF spec.
make CppunitTest_sw_ooxmlexport18 \
CPPUNIT_TEST_NAME=testTdf159158_zOrder_behindDocA
make CppunitTest_sw_ooxmlexport18 \
CPPUNIT_TEST_NAME=testTdf159158_zOrder_behindDocB
Change-Id: Ic9eaecee28fd412f9aecce61b1e3a607f26d424a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162030
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
diff --git a/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docx b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docx
new file mode 100644
index 0000000..b4e0b78
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docx
Binary files differ
diff --git a/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docx b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docx
new file mode 100644
index 0000000..7dec311
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docx
Binary files differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index f519c9d..8306ec2 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
@@ -108,7 +108,7 @@ CPPUNIT_TEST_FIXTURE(Test, testFdo69548)
DECLARE_OOXMLEXPORT_TEST(testWpsOnly, "wps-only.docx")
{
// Document has wp:anchor, not wp:inline, so handle it accordingly.
uno::Reference<drawing::XShape> xShape = getShape(1);
uno::Reference<drawing::XShape> xShape = getShapeByName(u"Isosceles Triangle 1");
text::TextContentAnchorType eValue = getProperty<text::TextContentAnchorType>(xShape, "AnchorType");
// Word only as as-char and at-char, so at-char is our only choice.
CPPUNIT_ASSERT_EQUAL(text::TextContentAnchorType_AT_CHARACTER, eValue);
@@ -124,7 +124,7 @@ DECLARE_OOXMLEXPORT_TEST(testWpsOnly, "wps-only.docx")
// This should be in front of text.
CPPUNIT_ASSERT_EQUAL(true, getProperty<bool>(xShape, "Opaque"));
// And this should be behind the document.
CPPUNIT_ASSERT_EQUAL(false, getProperty<bool>(getShape(2), "Opaque"));
CPPUNIT_ASSERT_EQUAL(false, getProperty<bool>(getShapeByName(u"Isosceles Triangle 2"), "Opaque"));
}
CPPUNIT_TEST_FIXTURE(Test, testFloattableNestedDOCXExport)
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index f7df210..1a5e068 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -718,7 +718,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf116976, "tdf116976.docx")
{
// This was 0, relative size of shape after bitmap was ignored.
CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(40),
getProperty<sal_Int16>(getShape(1), "RelativeWidth"));
getProperty<sal_Int16>(getShapeByName(u"Text Box 2"), "RelativeWidth"));
}
DECLARE_OOXMLEXPORT_TEST(testTdf116985, "tdf116985.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
index 96e7e6b..99bab0b 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport15.cxx
@@ -110,7 +110,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf137850_compat14ZOrder, "tdf137850_compat14ZOrder
{
// The file contains 2 shapes which have a different value of behindDoc.
// Test that the textbox is hidden behind the arrow (for Word <= 2010/compatibilityMode==14)
uno::Reference<text::XText> xShape(getShape(2), uno::UNO_QUERY);
uno::Reference<text::XText> xShape(getShape(1), uno::UNO_QUERY);
CPPUNIT_ASSERT_EQUAL(OUString("2015"), xShape->getString());
CPPUNIT_ASSERT_EQUAL_MESSAGE("Textbox is in the background", false, getProperty<bool>(xShape, "Opaque"));
}
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
index d83498b..e7040b6 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport18.cxx
@@ -1001,6 +1001,36 @@ DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_zIndexWins, "tdf159158_zOrder_zInd
// CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Blue"), getProperty<OUString>(zOrder2,"Name"));
}
DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_behindDocA, "tdf159158_zOrder_behindDocA.docx")
{
// given a yellow star with lowest relativeHeight 2 but behindDoc
// followed by an overlapping blue star with negative z-index -1644167168.
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, "ZOrder")); // lower
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(zOrder1, "ZOrder")); // higher
// yellow is at the lowest hell-level possible for relativeHeight, so expected to be under blue
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Yellow"), getProperty<OUString>(zOrder0, "Name"));
if (!isExported()) // the name is lost on export
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Blue"), getProperty<OUString>(zOrder1,"Name"));
}
DECLARE_OOXMLEXPORT_TEST(testTdf159158_zOrder_behindDocB, "tdf159158_zOrder_behindDocB.docx")
{
// given a yellow star with a high relativeHeight 503314431 (1DFF F7FF) but behindDoc
// followed by an overlapping blue star with negative z-index -1644167168.
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, "ZOrder")); // lower
CPPUNIT_ASSERT_EQUAL(sal_Int32(1), getProperty<sal_Int32>(zOrder1, "ZOrder")); // higher
// yellow is at the highest hell-level possible for relativeHeight,
// so you will be forgiven for thinking yellow should be on the top.
// Any z-index level ends up being above any relativeHeight, so blue should still be on top
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Yellow"), getProperty<OUString>(zOrder0, "Name"));
if (!isExported()) // the name is lost on export
CPPUNIT_ASSERT_EQUAL(OUString("5-Point Star Blue"), getProperty<OUString>(zOrder1,"Name"));
}
DECLARE_OOXMLEXPORT_TEST(testTdf155903, "tdf155903.odt")
{
// Without the accompanying fix in place, this test would have crashed,
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
index 686371b..6e4cd0a 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport8.cxx
@@ -59,7 +59,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf48569)
CPPUNIT_ASSERT_EQUAL(2, getShapes());
CPPUNIT_ASSERT_EQUAL(1, getPages());
// File crashing while saving in LO
text::TextContentAnchorType eValue = getProperty<text::TextContentAnchorType>(getShape(1), "AnchorType");
text::TextContentAnchorType eValue = getProperty<text::TextContentAnchorType>(getShapeByName(u"Marco1"), "AnchorType");
CPPUNIT_ASSERT_EQUAL(text::TextContentAnchorType_AS_CHARACTER, eValue);
}
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 93ef346..b5f6d33 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -386,6 +386,7 @@ public:
void applyZOrder(uno::Reference<beans::XPropertySet> const & xGraphicObjectProperties) const
{
sal_Int32 nZOrder = m_zOrder;
bool bBehindText = m_bBehindDoc && !m_bOpaque;
if (m_rGraphicImportType == GraphicImportType::IMPORT_AS_DETECTED_INLINE
&& !m_rDomainMapper.IsInShape())
{
@@ -394,7 +395,7 @@ public:
if (nZOrder >= 0)
{
// tdf#120760 Send objects with behinddoc=true to the back.
if (m_bBehindDoc && m_rDomainMapper.IsInHeaderFooter())
if (bBehindText)
nZOrder -= SAL_MAX_INT32;
// TODO: it is possible that RTF has been wrong all along as well. Always true here?
