sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docx |binary
sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docx |binary
sw/qa/extras/ooxmlexport/ooxmlexport10.cxx | 4 -
sw/qa/extras/ooxmlexport/ooxmlexport11.cxx | 2
sw/qa/extras/ooxmlexport/ooxmlexport15.cxx | 2
sw/qa/extras/ooxmlexport/ooxmlexport18.cxx | 30
++++++++++
sw/qa/extras/ooxmlexport/ooxmlexport8.cxx | 2
writerfilter/source/dmapper/GraphicImport.cxx | 3 -
8 files changed, 37 insertions(+), 6 deletions(-)
New commits:
commit 12ea98fe637eda5f038b6447a5f0c5ea7104f448
Author: Justin Luth <justin.l...@collabora.com>
AuthorDate: Tue Jan 16 21:08:08 2024 -0500
Commit: Miklos Vajna <vmik...@collabora.com>
CommitDate: Mon Jan 22 14:05:07 2024 +0100
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 <jl...@mail.com>
Reviewed-by: Miklos Vajna <vmik...@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 000000000000..b4e0b78c4675
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docx 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 000000000000..7dec311097ee
Binary files /dev/null and
b/sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docx differ
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index f519c9ddfe87..8306ec2c5465 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 f7df210641af..1a5e0683d757 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 96e7e6bae029..99bab0bf58bd 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 d83498b4427c..e7040b6d8d0b 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 686371bec25c..6e4cd0ae48fc 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 93ef346dcc99..b5f6d3385d58 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?
