DOCX import: fix relativeFrom=page, align=right handling again

sw/qa/extras/ooxmlexport/data/wpg-nested.docx has a right sidebar on the
page, which is a nested drawingML group shape. This was fixed to have a
reasonable position in commit 51a61bd4aca15c860d301b687d582a39193089e2
(DOCX import: fix relativeFrom=page, align=right handling, 2013-12-06)
but the testcase only asserted the document model, so it wasn't caught
that later commit af313fc149f80adb0f1680ca20e19745ccb7fede (tdf#105143
DOCX import: enable DoNotCaptureDrawObjsOnPage layout compat option,
2017-01-06) broke it.

Fix it again by reverting the original fix and focusing only the part
that is still a problem: the margins around the shape prevent it to be
exactly at the right edge (instead there is some spacing). The margins
are not relevant in the "no wrap: in front of text" and "no wrap: behind
text" cases, the Word UI even disables the controls in those wrap cases.

Improve the relevant testcase to assert the layout instead, so next time
this breaks, we'll more likely notice it.

For each modified existing testcase, I compared the old Writer, new
Writer and Word rendering result and came to the conclusion that it's
the test that should be changed, not the code.

Change-Id: Ida29929671148db77fdd9ce8dbfb5214bb4728dd
Reviewed-on: https://gerrit.libreoffice.org/55192
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Jenkins <ci@libreoffice.org>

4 files changed, 37 insertions, 20 deletions

diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
index 82469f0b2355..ad30e3d67629 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport10.cxx
@@ -155,8 +155,8 @@ DECLARE_OOXMLEXPORT_TEST(testWpsOnly, "wps-only.docx")
     // Check position, it was 0. This is a shape, so use getPosition(), not a property.
     CPPUNIT_ASSERT_EQUAL(oox::drawingml::convertEmuToHmm(671830), xShape->getPosition().X);
 
-    // Left margin was 0, instead of 114300 EMU's.
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(318), getProperty<sal_Int32>(xShape, "LeftMargin"));
+    // Left margin should be 0, as margins are not relevant for <wp:wrapNone/>.
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(xShape, "LeftMargin"));
     // Wrap type was PARALLEL.
     CPPUNIT_ASSERT_EQUAL(text::WrapTextMode_THROUGH, getProperty<text::WrapTextMode>(xShape, "Surround"));
     // Confirm that the deprecated (incorrectly spelled) _THROUGHT also matches
@@ -181,8 +181,20 @@ DECLARE_OOXMLEXPORT_TEST(testWpgNested, "wpg-nested.docx")
     uno::Reference<drawing::XShapeDescriptor> xShapeDescriptor(xGroup->getByIndex(0), uno::UNO_QUERY);
     // This was a com.sun.star.drawing.CustomShape, due to lack of handling of groupshapes inside groupshapes.
     CPPUNIT_ASSERT_EQUAL(OUString("com.sun.star.drawing.GroupShape"), xShapeDescriptor->getShapeType());
-    // This was text::RelOrientation::PAGE_FRAME, effectively placing the group shape on the left side of the page instead of the right one.
-    CPPUNIT_ASSERT_EQUAL(text::RelOrientation::PAGE_RIGHT, getProperty<sal_Int16>(xGroup, "HoriOrientRelation"));
+
+    // This failed, the right edge of the shape was outside the page
+    // boundaries.
+    xmlDocPtr pXmlDoc = parseLayoutDump();
+    sal_Int32 nPageLeft = getXPath(pXmlDoc, "/root/page[2]/infos/bounds", "left").toInt32();
+    sal_Int32 nPageWidth = getXPath(pXmlDoc, "/root/page[2]/infos/bounds", "width").toInt32();
+    sal_Int32 nShapeLeft
+        = getXPath(pXmlDoc, "/root/page[2]/body/txt/anchored/SwAnchoredDrawObject/bounds", "left")
+              .toInt32();
+    sal_Int32 nShapeWidth
+        = getXPath(pXmlDoc, "/root/page[2]/body/txt/anchored/SwAnchoredDrawObject/bounds", "width")
+              .toInt32();
+    // Make sure the shape is within the page bounds.
+    CPPUNIT_ASSERT_GREATEREQUAL(nShapeLeft + nShapeWidth, nPageLeft + nPageWidth);
 }
 
 DECLARE_OOXMLEXPORT_TEST(textboxWpgOnly, "textbox-wpg-only.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
index eb42cee254a2..6d099e57f8bb 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport11.cxx
@@ -193,12 +193,19 @@ DECLARE_OOXMLEXPORT_TEST(testSignatureLineShape, "signature-line-all-props-set.d
 
 DECLARE_OOXMLEXPORT_TEST(testTdf113183, "tdf113183.docx")
 {
-    // This was 2096, the horizontal positioning of the star shape affected the
-    // positioning of the triangle one, so the triangle was outside the page
-    // frame.
-    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0),
-                         getProperty<sal_Int32>(getShapeByName("triangle"),
-                                                "HoriOrientPosition"));
+    // The horizontal positioning of the star shape affected the positioning of
+    // the triangle one, so the triangle was outside the page frame.
+    xmlDocPtr pXmlDoc = parseLayoutDump();
+    sal_Int32 nPageLeft = getXPath(pXmlDoc, "/root/page[1]/infos/bounds", "left").toInt32();
+    sal_Int32 nPageWidth = getXPath(pXmlDoc, "/root/page[1]/infos/bounds", "width").toInt32();
+    sal_Int32 nShapeLeft
+        = getXPath(pXmlDoc, "/root/page/body/txt/anchored/SwAnchoredDrawObject[2]/bounds", "left")
+              .toInt32();
+    sal_Int32 nShapeWidth
+        = getXPath(pXmlDoc, "/root/page/body/txt/anchored/SwAnchoredDrawObject[2]/bounds", "width")
+              .toInt32();
+    // Make sure the second triangle shape is within the page bounds (with ~1px tolerance).
+    CPPUNIT_ASSERT_GREATEREQUAL(nShapeLeft + nShapeWidth, nPageLeft + nPageWidth + 21);
 }
 
 DECLARE_OOXMLEXPORT_TEST(testGraphicObjectFliph, "graphic-object-fliph.docx")
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
index 23602ac34bb1..36340d4cf1b6 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport4.cxx
@@ -76,7 +76,7 @@ DECLARE_OOXMLEXPORT_TEST(testRelorientation, "relorientation.docx")
 {
     uno::Reference<drawing::XShape> xShape = getShape(1);
     // This was text::RelOrientation::FRAME, when handling relativeFrom=page, align=right
-    CPPUNIT_ASSERT_EQUAL(text::RelOrientation::PAGE_RIGHT, getProperty<sal_Int16>(xShape, "HoriOrientRelation"));
+    CPPUNIT_ASSERT_EQUAL(text::RelOrientation::PAGE_FRAME, getProperty<sal_Int16>(xShape, "HoriOrientRelation"));
 
     uno::Reference<drawing::XShapes> xGroup(xShape, uno::UNO_QUERY);
     // This resulted in lang::IndexOutOfBoundsException, as nested groupshapes weren't handled.
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 45621b9c1686..57390765f134 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -1036,15 +1036,6 @@ void GraphicImport::lcl_sprm(Sprm& rSprm)
                     m_pImpl->nHoriRelation = pHandler->relation();
                     m_pImpl->nHoriOrient = pHandler->orientation();
                     m_pImpl->nLeftPosition = pHandler->position();
-                    if (m_pImpl->nHoriRelation == text::RelOrientation::PAGE_FRAME && m_pImpl->nHoriOrient == text::HoriOrientation::RIGHT)
-                    {
-                        // If the shape is relative from page and aligned to
-                        // right, then set the relation to right and clear the
-                        // orientation, that provides the same visual result as
-                        // Word.
-                        m_pImpl->nHoriRelation = text::RelOrientation::PAGE_RIGHT;
-                        m_pImpl->nHoriOrient = text::HoriOrientation::NONE;
-                    }
                 }
             }
         }
@@ -1090,6 +1081,13 @@ void GraphicImport::lcl_sprm(Sprm& rSprm)
         case NS_ooxml::LN_EG_WrapType_wrapNone: // 90944; - doesn't contain attributes
             //depending on the behindDoc attribute text wraps through behind or in front of the object
             m_pImpl->nWrap = text::WrapTextMode_THROUGH;
+
+            // Wrap though means the margins defined earlier should not be
+            // respected.
+            m_pImpl->nLeftMargin = 0;
+            m_pImpl->nTopMargin = 0;
+            m_pImpl->nRightMargin = 0;
+            m_pImpl->nBottomMargin = 0;
         break;
         case NS_ooxml::LN_EG_WrapType_wrapTopAndBottom: // 90948;
             m_pImpl->nWrap = text::WrapTextMode_NONE;