[Libreoffice-commits] core.git: Branch 'libreoffice-6-1' - sw/qa writerfilter/source

[Libreoffice Gerrit user]

 sw/qa/extras/ooxmlimport/data/tdf120551.docx  |binary
 sw/qa/extras/ooxmlimport/ooxmlimport2.cxx     |    8 +++++++
 writerfilter/source/dmapper/GraphicImport.cxx |   27 ++++++++++++++++++++++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

New commits:
commit ef5875be7d0aefcb95f878fee49d79a65cd063ea
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Mon Nov 19 21:41:07 2018 +0100
Commit:     Caolán McNamara <caolanm at redhat.com>
CommitDate: Wed Nov 28 16:49:03 2018 +0100

    tdf#120551 DOCX import: fix position of group shape with absolute position
    
    Regression from commit af313fc149f80adb0f1680ca20e19745ccb7fede
    (tdf#105143 DOCX import: enable DoNotCaptureDrawObjsOnPage layout compat
    option, 2017-01-06), the group shape which covers most of the single
    page in the document had a negative left position, while it should have
    a small positive one (checking the drawingML markup).
    
    This was a pre-existing problem, but now it's visible since we no longer
    force objects to be on the page in Word compat mode.
    
    Seeing the ODT import never positions group shapes (it's just a
    container for child shapes), probably the DOCX import shouldn't do that
    either. Start moving into this direction, first only for
    absolute-positioned toplevel group shapes only, which already fixes the
    bug.
    
    (cherry picked from commit d220e476df38e86e094066a690985c624316d37f)
    
    Change-Id: I152ba06a81a2bd09195a4c724da4b8878b0457bb
    Reviewed-on: https://gerrit.libreoffice.org/63710
    Tested-by: Jenkins
    Reviewed-by: Caolán McNamara <caolanm at redhat.com>
    Tested-by: Caolán McNamara <caolanm at redhat.com>

diff --git a/sw/qa/extras/ooxmlimport/data/tdf120551.docx b/sw/qa/extras/ooxmlimport/data/tdf120551.docx
new file mode 100644
index 000000000000..0fc0057c4e03
Binary files /dev/null and b/sw/qa/extras/ooxmlimport/data/tdf120551.docx differ
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
index 6b54a7991fb3..b7e75e9c28fe 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport2.cxx
@@ -93,6 +93,14 @@ DECLARE_OOXMLIMPORT_TEST(testGroupShapeFontName, "groupshape-fontname.docx")
         getProperty<OUString>(getRun(getParagraphOfText(1, xText), 1), "CharFontNameAsian"));
 }
 
+DECLARE_OOXMLIMPORT_TEST(test120551, "tdf120551.docx")
+{
+    auto nHoriOrientPosition = getProperty<sal_Int32>(getShape(1), "HoriOrientPosition");
+    // Without the accompanying fix in place, this test would have failed with
+    // 'Expected: 436, Actual  : -2542'.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(436), nHoriOrientPosition);
+}
+
 DECLARE_OOXMLIMPORT_TEST(testTdf111550, "tdf111550.docx")
 {
     // The test document has following ill-formed structure:
diff --git a/writerfilter/source/dmapper/GraphicImport.cxx b/writerfilter/source/dmapper/GraphicImport.cxx
index 45621b9c1686..c9597a25d6b3 100644
--- a/writerfilter/source/dmapper/GraphicImport.cxx
+++ b/writerfilter/source/dmapper/GraphicImport.cxx
@@ -65,11 +65,27 @@
 #include "WrapPolygonHandler.hxx"
 #include "util.hxx"
 
+using namespace css;
+
+namespace
+{
+bool isTopGroupObj(const uno::Reference<drawing::XShape>& xShape)
+{
+    SdrObject* pObject = GetSdrObjectFromXShape(xShape);
+    if (!pObject)
+        return false;
+
+    if (pObject->GetUpGroup())
+        return false;
+
+    return pObject->IsGroupObject();
+}
+}
+
 namespace writerfilter {
 
 namespace dmapper
 {
-using namespace css;
 
 class XInputStreamHelper : public cppu::WeakImplHelper<io::XInputStream>
 {
@@ -818,8 +834,15 @@ void GraphicImport::lcl_attribute(Id nName, Value& rValue)
                                 xShapeProps->setPropertyValue("RotateAngle", uno::makeAny(sal_Int32(0)));
 
                             // Position of the groupshape should be set after children have been added.
+                            // Long-term we should get rid of positioning group
+                            // shapes, though. Do it for top-level ones with
+                            // absolute page position as a start.
                             // fdo#80555: also set position for graphic shapes here
-                            m_xShape->setPosition(awt::Point(m_pImpl->nLeftPosition, m_pImpl->nTopPosition));
+                            if (!isTopGroupObj(m_xShape)
+                                || m_pImpl->nHoriRelation != text::RelOrientation::PAGE_FRAME
+                                || m_pImpl->nVertRelation != text::RelOrientation::PAGE_FRAME)
+                                m_xShape->setPosition(
+                                    awt::Point(m_pImpl->nLeftPosition, m_pImpl->nTopPosition));
 
                             if (nRotation)
                                 xShapeProps->setPropertyValue("RotateAngle", uno::makeAny(nRotation));