summaryrefslogtreecommitdiff
path: root/sw
diff options
context:
space:
mode:
authorJustin Luth <justin.luth@collabora.com>2024-01-16 21:08:08 -0500
committerMiklos Vajna <vmiklos@collabora.com>2024-01-22 14:05:07 +0100
commit12ea98fe637eda5f038b6447a5f0c5ea7104f448 (patch)
tree390d0fcfb2ca5f0f2d63cf4b16194a229bddc36c /sw
parent71fed56e4a09fbf141fdb8f064df6764e6e6d262 (diff)
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>
Diffstat (limited to 'sw')
-rw-r--r--sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocA.docxbin0 -> 15303 bytes
-rw-r--r--sw/qa/extras/ooxmlexport/data/tdf159158_zOrder_behindDocB.docxbin0 -> 15309 bytes
-rw-r--r--sw/qa/extras/ooxmlexport/ooxmlexport10.cxx4
-rw-r--r--sw/qa/extras/ooxmlexport/ooxmlexport11.cxx2
-rw-r--r--sw/qa/extras/ooxmlexport/ooxmlexport15.cxx2
-rw-r--r--sw/qa/extras/ooxmlexport/ooxmlexport18.cxx30
-rw-r--r--sw/qa/extras/ooxmlexport/ooxmlexport8.cxx2
7 files changed, 35 insertions, 5 deletions
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
--- /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 000000000000..7dec311097ee
--- /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 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);
}