From 32750c8a1121177ea48d3e7b237a9ddc834fde38 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 12 Feb 2020 15:02:18 +0100 Subject: 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. (cherry picked from commit 99847d6b3005c5444ed5a46ca578c0e40149d77c) Conflicts: sw/qa/extras/ooxmlexport/ooxmlexport7.cxx writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx Change-Id: I670e4dc2acbd2a0c6d47fe964cb5e3f2300e6848 --- sw/qa/extras/ooxmlexport/ooxmlexport10.cxx | 4 ++-- sw/qa/extras/ooxmlexport/ooxmlexport13.cxx | 4 ++-- sw/qa/extras/ooxmlexport/ooxmlexport4.cxx | 4 +--- sw/qa/extras/ooxmlexport/ooxmlexport7.cxx | 2 +- sw/qa/extras/ooxmlexport/ooxmlexport9.cxx | 2 +- writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx | 17 +++++++++++++++++ .../dmapper/data/inline-anchored-zorder.docx | Bin 0 -> 16684 bytes writerfilter/source/dmapper/GraphicImport.cxx | 11 +++++++++-- 8 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 writerfilter/qa/cppunittests/dmapper/data/inline-anchored-zorder.docx diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx index 92d7d63aa80d..9b086540f9dd 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx @@ -487,11 +487,11 @@ DECLARE_OOXMLEXPORT_TEST(testStrict, "strict.docx") getParagraphOfText(1, xHeaderText, "This is a header."); // Picture was missing. - uno::Reference xServiceInfo(getShape(1), uno::UNO_QUERY); + uno::Reference 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 e586a666dff4..d2765488b797 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport13.cxx @@ -556,10 +556,10 @@ DECLARE_OOXMLEXPORT_TEST(testTdf119201, "tdf119201.docx") auto xShape(getShape(1)); CPPUNIT_ASSERT_MESSAGE("First shape should be visible.", getProperty(xShape, "Visible")); CPPUNIT_ASSERT_MESSAGE("First shape should be printable.", getProperty(xShape, "Printable")); - xShape = getShape(2); + xShape = getShapeByName("Rectangle 1"); CPPUNIT_ASSERT_MESSAGE("Second shape should not be visible.", !getProperty(xShape, "Visible")); CPPUNIT_ASSERT_MESSAGE("Second shape should not be printable.", !getProperty(xShape, "Printable")); - xShape = getShape(3); + xShape = getShapeByName("Oval 2"); CPPUNIT_ASSERT_MESSAGE("Third shape should be visible.", getProperty(xShape, "Visible")); CPPUNIT_ASSERT_MESSAGE("Third shape should be printable.", getProperty(xShape, "Printable")); } diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx index a7a1336dd480..f9c5997a42b1 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx @@ -1084,9 +1084,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf102466, "tdf102466.docx") // check content of the first page { - uno::Reference xDrawPageSupplier(mxComponent, uno::UNO_QUERY); - uno::Reference xIndexAccess = xDrawPageSupplier->getDrawPage(); - uno::Reference xFrame(xIndexAccess->getByIndex(0), uno::UNO_QUERY); + uno::Reference xFrame(getShapeByName("Marco1"), uno::UNO_QUERY); // no border CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty(xFrame, "LineWidth")); diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx index b673c06a3d2a..860d6927e505 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport7.cxx @@ -586,7 +586,7 @@ DECLARE_OOXMLEXPORT_TEST(fdo76591, "fdo76591.docx") xmlDocPtr pXmlDoc = parseExport("word/document.xml"); if (!pXmlDoc) return; - 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_TEST(test76317_2K10, "test76317_2K10.docx") diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx index ece282f437aa..f9959d21886a 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx @@ -569,7 +569,7 @@ DECLARE_OOXMLEXPORT_TEST(testTdf103573, "tdf103573.docx") DECLARE_OOXMLEXPORT_TEST(testTdf106132, "tdf106132.docx") { - uno::Reference xShape(getShape(1), uno::UNO_QUERY); + uno::Reference xShape(getShapeByName("Frame1"), uno::UNO_QUERY); // This was 250, was ignored for an outer shape. CPPUNIT_ASSERT_EQUAL(static_cast(0), getProperty(xShape, "TextRightDistance")); } diff --git a/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx b/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx index f20694b828cc..d5140f134cb3 100644 --- a/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx +++ b/writerfilter/qa/cppunittests/dmapper/GraphicImport.cxx @@ -17,6 +17,7 @@ #include #include #include +#include #include @@ -107,6 +108,22 @@ CPPUNIT_TEST_FIXTURE(Test, testDrawShapeInlineEffect) // i.e. the layout result had less pages than expected (compared to Word). CPPUNIT_ASSERT_EQUAL(static_cast(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 xDrawPageSupplier(getComponent(), uno::UNO_QUERY); + uno::Reference xDrawPage = xDrawPageSupplier->getDrawPage(); + uno::Reference 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 Binary files /dev/null and b/writerfilter/qa/cppunittests/dmapper/data/inline-anchored-zorder.docx differ diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx index b69f4565fb13..df5f644e34ba 100644 --- a/writerfilter/source/dmapper/GraphicImport.cxx +++ b/writerfilter/source/dmapper/GraphicImport.cxx @@ -287,7 +287,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) { @@ -358,7 +363,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); } } @@ -898,6 +904,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())); -- cgit