Fix two issues in ActiveX DOCX import / export code

* Inline anchored VML shape had wrong vertical position
** In MSO inline shapes are positioned to the top of the baseline
* During export all shape ids were the same (shape_0)
** VML shapes used to be exported only as fallback,
I guess that's why it did not cause any issue before.
** Override the shapeid generator with a new one, which
actually generates unique shapeids.

Change-Id: I752f39d092d0b61d91824141655dae662dbeafbc
Reviewed-on: https://gerrit.libreoffice.org/41319
Reviewed-by: Tamás Zolnai <tamas.zolnai@collabora.com>
Tested-by: Tamás Zolnai <tamas.zolnai@collabora.com>

8 files changed, 120 insertions, 14 deletions

diff --git a/include/filter/msfilter/escherex.hxx b/include/filter/msfilter/escherex.hxx
index 14e1d69d5aab..f1468ef7f5df 100644
--- a/include/filter/msfilter/escherex.hxx
+++ b/include/filter/msfilter/escherex.hxx
@@ -1052,7 +1052,7 @@ public:
 
     /** Creates and returns a new shape identifier, updates the internal shape
         counters and registers the identifier in the DGG cluster table. */
-    sal_uInt32   GenerateShapeId() { return mxGlobal->GenerateShapeId( mnCurrentDg, mbEscherSpgr ); }
+    virtual sal_uInt32   GenerateShapeId() { return mxGlobal->GenerateShapeId( mnCurrentDg, mbEscherSpgr ); }
 
     /** Returns the graphic provider from the global object that has been
         passed to the constructor.
diff --git a/include/oox/export/vmlexport.hxx b/include/oox/export/vmlexport.hxx
index c098ace13cb7..ee40b933deaa 100644
--- a/include/oox/export/vmlexport.hxx
+++ b/include/oox/export/vmlexport.hxx
@@ -111,6 +111,17 @@ class OOX_DLLPUBLIC VMLExport : public EscherEx
     /// Use '#' mark for type attribute (check Type Attribute of VML shape in OOXML documentation)
     bool m_bUseHashMarkForType;
 
+    /** There is a shapeid generation mechanism in EscherEx, but it does not seem to work
+    *   so override the existing behavior to get actually unique ids.
+    */
+    bool m_bOverrideShapeIdGeneration;
+
+    /// Prefix for overriden shape id generation (used if m_bOverrideShapeIdGeneration is true)
+    OString m_sShapeIDPrefix;
+
+    /// Counter for generating shape ids (used if m_bOverrideShapeIdGeneration is true)
+    sal_uInt64 m_nShapeIDCounter;
+
 public:
                         VMLExport( ::sax_fastparser::FSHelperPtr const & pSerializer, VMLTextExport* pTextExport = nullptr);
     virtual             ~VMLExport() override;
@@ -130,8 +141,9 @@ public:
     virtual void  AddSdrObjectVMLObject( const SdrObject& rObj) override;
     static bool IsWaterMarkShape(const OUString& rStr);
 
-    void    SetSkipwzName() { m_bSkipwzName = true; }
-    void    SetHashMarkForType() { m_bUseHashMarkForType = true; }
+    void    SetSkipwzName(bool bSkipwzName) { m_bSkipwzName = bSkipwzName; }
+    void    SetHashMarkForType(bool bUseHashMarkForType) { m_bUseHashMarkForType = bUseHashMarkForType; }
+    void    OverrideShapeIDGen(bool bOverrideShapeIdGeneration, const OString sShapeIDPrefix = OString());
 protected:
     /// Add an attribute to the generated <v:shape/> element.
     ///
@@ -142,6 +154,9 @@ protected:
     using EscherEx::StartShape;
     using EscherEx::EndShape;
 
+    /// Override shape ID generation when m_bOverrideShapeIdGeneration is set to true
+    virtual sal_uInt32   GenerateShapeId() override;
+
     /// Start the shape for which we just collected the information.
     ///
     /// Returns the element's tag number, -1 means we wrote nothing.
@@ -165,7 +180,7 @@ private:
 
 private:
     /// Create an OString representing the id from a numerical id.
-    static OString ShapeIdString( sal_uInt32 nId );
+    OString ShapeIdString( sal_uInt32 nId );
 
     /// Add flip X and\or flip Y
     void AddFlipXY( );
diff --git a/oox/source/export/vmlexport.cxx b/oox/source/export/vmlexport.cxx
index a401c3c44465..f45edde6cc86 100644
--- a/oox/source/export/vmlexport.cxx
+++ b/oox/source/export/vmlexport.cxx
@@ -68,6 +68,8 @@ VMLExport::VMLExport( ::sax_fastparser::FSHelperPtr const & pSerializer, VMLText
     , m_aShapeTypeWritten( ESCHER_ShpInst_COUNT )
     , m_bSkipwzName( false )
     , m_bUseHashMarkForType( false )
+    , m_bOverrideShapeIdGeneration( false )
+    , m_nShapeIDCounter( 0 )
 {
     mnGroupLevel = 1;
 }
@@ -208,6 +210,18 @@ bool VMLExport::IsWaterMarkShape(const OUString& rStr)
      return rStr.match("PowerPlusWaterMarkObject") || rStr.match("WordPictureWatermark");
 }
 
+void VMLExport::OverrideShapeIDGen(bool bOverrideShapeIdGen, const OString sShapeIDPrefix)
+{
+    m_bOverrideShapeIdGeneration = bOverrideShapeIdGen;
+    if(bOverrideShapeIdGen)
+    {
+        assert(!sShapeIDPrefix.isEmpty());
+        m_sShapeIDPrefix = sShapeIDPrefix;
+    }
+    else
+        m_sShapeIDPrefix.clear();
+}
+
 static void impl_AddArrowHead( sax_fastparser::FastAttributeList *pAttrList, sal_Int32 nElement, sal_uInt32 nValue )
 {
     if ( !pAttrList )
@@ -884,7 +898,10 @@ void VMLExport::Commit( EscherPropertyContainer& rProps, const tools::Rectangle&
 
 OString VMLExport::ShapeIdString( sal_uInt32 nId )
 {
-    return "shape_" + OString::number( nId );
+    if(m_bOverrideShapeIdGeneration)
+        return m_sShapeIDPrefix + OString::number( nId );
+    else
+        return "shape_" + OString::number( nId );
 }
 
 void VMLExport::AddFlipXY( )
@@ -1025,6 +1042,14 @@ OUString lcl_getAnchorIdFromGrabBag(const SdrObject* pSdrObject)
     return aResult;
 }
 
+sal_uInt32 VMLExport::GenerateShapeId()
+{
+    if(!m_bOverrideShapeIdGeneration)
+        return EscherEx::GenerateShapeId();
+    else
+        return m_nShapeIDCounter++;
+}
+
 sal_Int32 VMLExport::StartShape()
 {
     if ( m_nShapeType == ESCHER_ShpInst_Nil )
diff --git a/oox/source/vml/vmlshape.cxx b/oox/source/vml/vmlshape.cxx
index b51a9020393f..e1d0cf6d9a41 100644
--- a/oox/source/vml/vmlshape.cxx
+++ b/oox/source/vml/vmlshape.cxx
@@ -616,6 +616,8 @@ void lcl_SetAnchorType(PropertySet& rPropSet, const ShapeTypeModel& rTypeModel,
     else // static (is the default) means anchored inline
     {
         rPropSet.setProperty(PROP_AnchorType, text::TextContentAnchorType_AS_CHARACTER);
+        // Use top orientation, this one seems similar to what MSO uses as inline
+        rPropSet.setAnyProperty(PROP_VertOrient, makeAny(text::VertOrientation::TOP));
     }
     lcl_setSurround( rPropSet, rTypeModel, rGraphicHelper );
 }
diff --git a/sw/qa/extras/ooxmlexport/data/activex_control_align.odt b/sw/qa/extras/ooxmlexport/data/activex_control_align.odt
new file mode 100755
index 000000000000..b9944c7e5abe
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/activex_control_align.odt
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
index 43de0cc7ec55..f19fb6b65d76 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
@@ -23,6 +23,7 @@
 #include <com/sun/star/style/PageStyleLayout.hpp>
 #include <com/sun/star/text/HoriOrientation.hpp>
 #include <com/sun/star/text/RelOrientation.hpp>
+#include <com/sun/star/text/VertOrientation.hpp>
 #include <com/sun/star/view/XViewSettingsSupplier.hpp>
 #include <com/sun/star/style/LineSpacing.hpp>
 #include <com/sun/star/style/LineSpacingMode.hpp>
@@ -855,6 +856,67 @@ DECLARE_OOXMLEXPORT_TEST( testActiveXCheckbox, "activex_checkbox.docx" )
     CPPUNIT_ASSERT_EQUAL(text::TextContentAnchorType_AT_CHARACTER,getProperty<text::TextContentAnchorType>(xPropertySet2,"AnchorType"));
 }
 
+DECLARE_OOXMLEXPORT_TEST(testActiveXControlAlign, "activex_control_align.odt")
+{
+    // First check box aligned as a floating object
+    uno::Reference<drawing::XControlShape> xControlShape(getShape(1), uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xControlShape.is());
+
+    // Check whether we have the right control
+    uno::Reference<beans::XPropertySet> xPropertySet(xControlShape->getControl(), uno::UNO_QUERY);
+    uno::Reference<lang::XServiceInfo> xServiceInfo(xPropertySet, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(true, bool(xServiceInfo->supportsService( "com.sun.star.form.component.CheckBox")));
+    CPPUNIT_ASSERT_EQUAL(OUString("Floating Check Box"), getProperty<OUString>(xPropertySet, "Label"));
+
+    // Check anchor type
+    uno::Reference<beans::XPropertySet> xPropertySet2(xControlShape, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(text::TextContentAnchorType_AT_CHARACTER,getProperty<text::TextContentAnchorType>(xPropertySet2,"AnchorType"));
+
+    // Also check positin and size
+    uno::Reference<drawing::XShape> xShape(xControlShape, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xShape.is());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4470), xShape->getSize().Width);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1427), xShape->getSize().Height);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(5126), xShape->getPosition().X);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(2341), xShape->getPosition().Y);
+
+    // Second check box aligned inline / as character
+    xControlShape.set(getShape(2), uno::UNO_QUERY);
+
+    // Check whether we have the right control
+    xPropertySet.set(xControlShape->getControl(), uno::UNO_QUERY);
+    xServiceInfo.set(xPropertySet, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(true, bool(xServiceInfo->supportsService("com.sun.star.form.component.CheckBox")));
+    CPPUNIT_ASSERT_EQUAL(OUString("Inline Check Box"), getProperty<OUString>(xPropertySet, "Label"));
+
+    // Check anchor type
+    xPropertySet2.set(xControlShape, uno::UNO_QUERY);
+    CPPUNIT_ASSERT_EQUAL(text::TextContentAnchorType_AS_CHARACTER,getProperty<text::TextContentAnchorType>(xPropertySet2,"AnchorType"));
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(text::VertOrientation::TOP),getProperty<sal_Int32>(xPropertySet2,"VertOrient"));
+
+    // Also check positin and size
+    xShape.set(xControlShape, uno::UNO_QUERY);
+    CPPUNIT_ASSERT(xShape.is());
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(4410), xShape->getSize().Width);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(1083), xShape->getSize().Height);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), xShape->getPosition().X);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(-1085), xShape->getPosition().Y);
+
+    // Also check the specific OOXML elements
+    xmlDocPtr pXmlDoc = parseExport();
+    CPPUNIT_ASSERT(pXmlDoc);
+    // For inline controls use w:object as parent element and pictureFrame shapetype
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r/w:object", 1);
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r/w:object/v:shapetype", "spt", "75");
+    // For floating controls use w:pict as parent element and hostControl shapetype
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r[1]/w:pict", 1);
+    assertXPath(pXmlDoc, "/w:document/w:body/w:p/w:r[1]/w:pict/v:shapetype", "spt", "201");
+
+     // Have different shape ids
+    CPPUNIT_ASSERT(getXPath(pXmlDoc, "/w:document/w:body/w:p/w:r/w:object/v:shape", "id") !=
+        getXPath(pXmlDoc, "/w:document/w:body/w:p/w:r[1]/w:pict/v:shape", "id"));
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
index e44d41f26d03..894c573a929f 100644
--- a/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
+++ b/sw/qa/extras/ooxmlimport/ooxmlimport.cxx
@@ -86,7 +86,6 @@ public:
     }
 };
 
-#if !defined _WIN32
 class FailTest : public Test
 {
 public:
@@ -118,7 +117,7 @@ public:
         finish();
     }
 };
-#endif
+
 
 DECLARE_OOXMLIMPORT_TEST(testImageHyperlink, "image-hyperlink.docx")
 {
@@ -126,8 +125,6 @@ DECLARE_OOXMLIMPORT_TEST(testImageHyperlink, "image-hyperlink.docx")
     CPPUNIT_ASSERT_EQUAL(OUString("http://www.libreoffice.org/"), URL);
 }
 
-#if !defined(_WIN32)
-
 DECLARE_SW_IMPORT_TEST(testMathMalformedXml, "math-malformed_xml.docx", nullptr, FailTest)
 {
     CPPUNIT_ASSERT(!mxComponent.is());
@@ -391,6 +388,7 @@ DECLARE_OOXMLIMPORT_TEST(testN775899, "n775899.docx")
 DECLARE_OOXMLIMPORT_TEST(testN777345, "n777345.docx")
 {
 #if !defined(MACOSX)
+#if !defined(_WIN32)
     // The problem was that v:imagedata inside v:rect was ignored.
     uno::Reference<document::XEmbeddedObjectSupplier2> xSupplier(getShape(1), uno::UNO_QUERY);
     uno::Reference<graphic::XGraphic> xGraphic = xSupplier->getReplacementGraphic();
@@ -399,6 +397,7 @@ DECLARE_OOXMLIMPORT_TEST(testN777345, "n777345.docx")
     // the checksum of a white/transparent placeholder rectangle.
     CPPUNIT_ASSERT_EQUAL(BitmapChecksum(SAL_CONST_UINT64(18203404956065762943)), aGraphic.GetChecksum());
 #endif
+#endif
 }
 
 DECLARE_OOXMLIMPORT_TEST(testN778140, "n778140.docx")
@@ -553,7 +552,7 @@ DECLARE_OOXMLIMPORT_TEST(testGroupshapeChildRotation, "groupshape-child-rotation
     uno::Reference<drawing::XShapes> xGroupShape(getShape(1), uno::UNO_QUERY);
     uno::Reference<drawing::XShape> xShape(xGroupShape->getByIndex(0), uno::UNO_QUERY);
     CPPUNIT_ASSERT_EQUAL(sal_Int32(0), xShape->getPosition().X);
-    CPPUNIT_ASSERT_EQUAL(sal_Int32(0), xShape->getPosition().Y);
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(-5741), xShape->getPosition().Y);
 
 #if ! TEST_FONTS_MISSING
     xShape.set(xGroupShape->getByIndex(4), uno::UNO_QUERY);
@@ -1062,8 +1061,6 @@ DECLARE_OOXMLIMPORT_TEST(testTdf49073, "tdf49073.docx")
     CPPUNIT_ASSERT_EQUAL(sal_Int16(text::RubyAdjust_RIGHT)  ,getProperty<sal_Int16>(getParagraph(6)->getStart(),"RubyAdjust"));
 }
 
-#endif
-
 DECLARE_OOXMLIMPORT_TEST(testTdf85232, "tdf85232.docx")
 {
     uno::Reference<drawing::XShapes> xShapes(getShapeByName("Group 219"), uno::UNO_QUERY);
diff --git a/sw/source/filter/ww8/docxattributeoutput.cxx b/sw/source/filter/ww8/docxattributeoutput.cxx
index 4bc32ff3157b..5d9bf2b040be 100644
--- a/sw/source/filter/ww8/docxattributeoutput.cxx
+++ b/sw/source/filter/ww8/docxattributeoutput.cxx
@@ -4819,8 +4819,9 @@ void DocxAttributeOutput::WriteActiveXControl(const SdrObject* pObject, const Sw
     std::pair<OString,OString> sRelIdAndName = m_rExport.WriteActiveXObject(xShape, xControlModel);
 
     // VML shape definition
-    m_rExport.VMLExporter().SetSkipwzName();
-    m_rExport.VMLExporter().SetHashMarkForType();
+    m_rExport.VMLExporter().SetSkipwzName(true);
+    m_rExport.VMLExporter().SetHashMarkForType(true);
+    m_rExport.VMLExporter().OverrideShapeIDGen(true, "control_shape_");
     OString sShapeId;
     if(bAnchoredInline)
     {
@@ -4835,6 +4836,10 @@ void DocxAttributeOutput::WriteActiveXControl(const SdrObject* pObject, const Sw
             rHoriOri.GetRelationOrient(),
             rVertOri.GetRelationOrient(), true);
     }
+    // Restore default values
+    m_rExport.VMLExporter().SetSkipwzName(false);
+    m_rExport.VMLExporter().SetHashMarkForType(false);
+    m_rExport.VMLExporter().OverrideShapeIDGen(false);
 
     // control
     m_pSerializer->singleElementNS(XML_w, XML_control,