DOCX import: fix ZOrder of inline vs anchored shapes

Shapes which are anchored but are not in the background should be always
on top of as-char anchored shapes in OOXML terms. Writer supports a
custom ZOrder even for as-char shapes, so make sure that they are
always behind anchored shapes.

To avoid unnecessary work, make sure that when there are multiple inline
shapes, we don't pointlessly reorder them (the old vs new style of the
sorting controls exactly this, what happens when two shapes have the
same ZOrder, and all inline shapes have a 0 ZOrder).

Adapt a few tests that used ZOrder indexes to access shapes, but the
intention was to just refer to a shape: fix the index and migrate to
shape names where possible.

Change-Id: I670e4dc2acbd2a0c6d47fe964cb5e3f2300e6848
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/88540
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

8 files changed, 33 insertions, 11 deletions

diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index dca06548b815..6d11c3c12590 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
@@ -482,11 +482,11 @@ DECLARE_OOXMLEXPORT_TEST(testStrict, "strict.docx")
     getParagraphOfText(1, xHeaderText, "This is a header.");
 
     // Picture was missing.
-    uno::Reference<lang::XServiceInfo> xServiceInfo(getShape(1), uno::UNO_QUERY);
+    uno::Reference<lang::XServiceInfo> xServiceInfo(getShapeByName("Picture 2"), uno::UNO_QUERY);
     CPPUNIT_ASSERT(xServiceInfo->supportsService("com.sun.star.text.TextGraphicObject"));
 
     // SmartArt was missing.
-    xServiceInfo.set(getShape(2), uno::UNO_QUERY);
+    xServiceInfo.set(getShape(1), uno::UNO_QUERY);
     CPPUNIT_ASSERT(xServiceInfo->supportsService("com.sun.star.drawing.GroupShape"));
 
     // Chart was missing.
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
index 27c66c141357..c3a8c4948397 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx
@@ -484,10 +484,10 @@ DECLARE_OOXMLEXPORT_TEST(testTdf119201, "tdf119201.docx")
     auto xShape(getShape(1));
     CPPUNIT_ASSERT_MESSAGE("First shape should be visible.", getProperty<bool>(xShape, "Visible"));
     CPPUNIT_ASSERT_MESSAGE("First shape should be printable.", getProperty<bool>(xShape, "Printable"));
-    xShape = getShape(2);
+    xShape = getShapeByName("Rectangle 1");
     CPPUNIT_ASSERT_MESSAGE("Second shape should not be visible.", !getProperty<bool>(xShape, "Visible"));
     CPPUNIT_ASSERT_MESSAGE("Second shape should not be printable.", !getProperty<bool>(xShape, "Printable"));
-    xShape = getShape(3);
+    xShape = getShapeByName("Oval 2");
     CPPUNIT_ASSERT_MESSAGE("Third shape should be visible.", getProperty<bool>(xShape, "Visible"));
     CPPUNIT_ASSERT_MESSAGE("Third shape should be printable.", getProperty<bool>(xShape, "Printable"));
 }
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
index 05466e36b70f..998d7cd5b2d8 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
@@ -998,9 +998,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf102466, "tdf102466.docx")
 
     // check content of the first page
     {
-        uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(mxComponent, uno::UNO_QUERY);
-        uno::Reference<container::XIndexAccess> xIndexAccess = xDrawPageSupplier->getDrawPage();
-        uno::Reference<beans::XPropertySet> xFrame(xIndexAccess->getByIndex(0), uno::UNO_QUERY);
+        uno::Reference<beans::XPropertySet> xFrame(getShapeByName("Marco1"), uno::UNO_QUERY);
 
         // no border
         CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(xFrame, "LineWidth"));
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx
index 344b493dae4b..e057f4e1c856 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx
@@ -564,7 +564,7 @@ DECLARE_OOXMLEXPORT_TEST(test76317, "test76317.docx")
 DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(fdo76591, "fdo76591.docx")
 {
     xmlDocPtr pXmlDoc = parseExport("word/document.xml");
-    assertXPath(pXmlDoc, "/w:document[1]/w:body[1]/w:p[1]/w:r[3]/mc:AlternateContent[1]/mc:Choice[1]/w:drawing[1]/wp:anchor[1]", "relativeHeight", "3");
+    assertXPath(pXmlDoc, "/w:document[1]/w:body[1]/w:p[1]/w:r[3]/mc:AlternateContent[1]/mc:Choice[1]/w:drawing[1]/wp:anchor[1]", "relativeHeight", "4");
 }
 
 DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(test76317_2K10, "test76317_2K10.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
index df96f077389a..b7f6852a6a23 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
@@ -630,7 +630,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf103573, "tdf103573.docx")
 
 DECLARE_OOXMLEXPORT_TEST(testTdf106132, "tdf106132.docx")
 {
-    uno::Reference<beans::XPropertySet> xShape(getShape(1), uno::UNO_QUERY);
+    uno::Reference<beans::XPropertySet> xShape(getShapeByName("Frame1"), uno::UNO_QUERY);
     // This was 250, <wps:bodyPr ... rIns="0" ...> was ignored for an outer shape.
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(xShape, "TextRightDistance"));
 }
diff --git a/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx b/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx
index 73f863b02100..f825ea1d7510 100644
--- a/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx
+++ b/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx
@@ -16,6 +16,7 @@
 #include <com/sun/star/table/BorderLine2.hpp>
 #include <com/sun/star/text/XTextTable.hpp>
 #include <com/sun/star/text/XTextTablesSupplier.hpp>
+#include <com/sun/star/container/XNamed.hpp>
 
 #include <comphelper/processfactory.hxx>
 
@@ -85,6 +86,22 @@ CPPUNIT_TEST_FIXTURE(Test, testDrawShapeInlineEffect)
     // i.e. the layout result had less pages than expected (compared to Word).
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(273), nBottomMargin);
 }
+
+CPPUNIT_TEST_FIXTURE(Test, testInlineAnchoredZOrder)
+{
+    // Load a document which has two shapes: an inline one and an anchored one. The inline has no
+    // explicit ZOrder, the anchored one has, and it's set to a value so it's visible.
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "inline-anchored-zorder.docx";
+    getComponent() = loadFromDesktop(aURL);
+    uno::Reference<drawing::XDrawPageSupplier> xDrawPageSupplier(getComponent(), uno::UNO_QUERY);
+    uno::Reference<drawing::XDrawPage> xDrawPage = xDrawPageSupplier->getDrawPage();
+    uno::Reference<container::XNamed> xOval(xDrawPage->getByIndex(1), uno::UNO_QUERY);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: Oval 2
+    // - Actual  :
+    // i.e. the rectangle (with no name) was on top of the oval one, not the other way around.
+    CPPUNIT_ASSERT_EQUAL(OUString("Oval 2"), xOval->getName());
+}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/writerfilter/qa/cppunittests/dmapper/data/inline-anchored-zorder.docx b/writerfilter/qa/cppunittests/dmapper/data/inline-anchored-zorder.docx
new file mode 100644
index 000000000000..93932c4703b7
--- /dev/null
+++ b/writerfilter/qa/cppunittests/dmapper/data/inline-anchored-zorder.docx
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index e3a88757125a..17691cb1c8b5 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -291,7 +291,12 @@ public:
         ,m_rPositionOffsets(rPositionOffsets)
         ,m_rAligns(rAligns)
         ,m_rPositivePercentages(rPositivePercentages)
-    {}
+    {
+        if (eGraphicImportType == GraphicImportType::IMPORT_AS_DETECTED_INLINE)
+        {
+            zOrder = 0;
+        }
+    }
 
     void setXSize(sal_Int32 _nXSize)
     {
@@ -362,7 +367,8 @@ public:
         if (zOrder >= 0)
         {
             GraphicZOrderHelper* pZOrderHelper = rDomainMapper.graphicZOrderHelper();
-            xGraphicObjectProperties->setPropertyValue(getPropertyName(PROP_Z_ORDER), uno::makeAny(pZOrderHelper->findZOrder(zOrder)));
+            bool bOldStyle = eGraphicImportType == GraphicImportType::IMPORT_AS_DETECTED_INLINE;
+            xGraphicObjectProperties->setPropertyValue(getPropertyName(PROP_Z_ORDER), uno::makeAny(pZOrderHelper->findZOrder(zOrder, bOldStyle)));
             pZOrderHelper->addItem(xGraphicObjectProperties, zOrder);
         }
     }
@@ -902,6 +908,7 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
                     {
                         uno::Reference< beans::XPropertySet > xShapeProps(m_xShape, uno::UNO_QUERY_THROW);
                         m_pImpl->applyMargins(xShapeProps);
+                        m_pImpl->applyZOrder(xShapeProps);
                         comphelper::SequenceAsHashMap aInteropGrabBag(xShapeProps->getPropertyValue("InteropGrabBag"));
                         aInteropGrabBag.update(m_pImpl->getInteropGrabBag());
                         xShapeProps->setPropertyValue("InteropGrabBag", uno::makeAny(aInteropGrabBag.getAsConstPropertyValueList()));