tdf#112118: DOCX: properly import/export border distance

https://wiki.openoffice.org/wiki/Writer/MSInteroperability/PageBorder
discusses implementation differences between ODF model and MS formats
wrt dealing with page margins and distances to borders.

This patch corrects import from DOCX, so that the border distance and
width doesn't add to the margin size imported from file anymore. It
takes care to preserve size from page edge to text (the most important
size that affects document layout). When borders go outside of range
valid for ODF, the margin is set to keep text area intact, and the
border is placed as close to intended position as possible.

Export code now also properly handles border width. Also, an improved
heuristic implemented to better export cases unsupported by Word, so
that the result would look closer to ODF original. We still write
correct sizes to OOXML, so that when reopened by LO, the borders will
be in correct places; but as Word cannot handle sizes more than 31 pt,
it will show borders shifted.

This prevents from adding border widths and distances to page margins
at each opening of DOCX, saving back the changed value, increasing
the margins each time.

Change-Id: Ia978ab119dd661949d6c321aea91397f28d205b0
Reviewed-on: https://gerrit.libreoffice.org/51267Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/51399Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
Tested-by: Miklos Vajna <vmiklos@collabora.co.uk>

sw/qa/extras/ooxmlexport/ooxmlexport9.cxx

@@ -11,6 +11,7 @@
 
 #include <com/sun/star/beans/XPropertySet.hpp>
 #include <com/sun/star/drawing/EnhancedCustomShapeParameterPair.hpp>
+#include <com/sun/star/table/BorderLine.hpp>
 #include <com/sun/star/text/XDependentTextField.hpp>
 #include <com/sun/star/text/XPageCursor.hpp>
 #include <com/sun/star/text/XTextColumns.hpp>
@@ -647,6 +648,42 @@ DECLARE_OOXMLEXPORT_TEST(testGraphicObjectFliph, "graphic-object-fliph.docx")
     CPPUNIT_ASSERT(getProperty<bool>(getShape(1), "HoriMirroredOnOddPages"));
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf112118, "tdf112118.docx")
+{
+    auto xStyles = getStyles("PageStyles");
+    const std::initializer_list<OUStringLiteral> sides = {
+        OUStringLiteral("Top"),
+        OUStringLiteral("Left"),
+        OUStringLiteral("Bottom"),
+        OUStringLiteral("Right")
+    };
+    auto testProc = [&](const OUString& sStyleName, sal_Int32 nMargin, sal_Int32 nBorderDistance,
+                        sal_Int16 nBorderWidth)
+    {
+        uno::Reference<beans::XPropertySet> xStyle(xStyles->getByName(sStyleName), uno::UNO_QUERY_THROW);
+        for (const auto& side : sides)
+        {
+            table::BorderLine aBorder = getProperty<table::BorderLine>(xStyle, side + "Border");
+            CPPUNIT_ASSERT_EQUAL(sal_Int16(nBorderWidth), aBorder.OuterLineWidth);
+            CPPUNIT_ASSERT_EQUAL(sal_Int16(0), aBorder.InnerLineWidth);
+            CPPUNIT_ASSERT_EQUAL(sal_Int16(0), aBorder.LineDistance);
+
+            sal_Int32 nMarginActual = getProperty<sal_Int32>(xStyle, side + "Margin");
+            CPPUNIT_ASSERT_EQUAL(nMargin, nMarginActual);
+
+            sal_Int32 nBorderDistanceActual = getProperty<sal_Int32>(xStyle, side + "BorderDistance");
+            CPPUNIT_ASSERT_EQUAL(nBorderDistance, nBorderDistanceActual);
+        }
+    };
+
+    // For both styles used in document, the total distance from page edge to text must be 2.54 cm.
+    // The first style uses "from edge" border distance; the second uses "from text" border distance
+    // Border distances in both cases are 24 pt = 847 mm100; line widths are 6 pt = 212 mm100.
+    // 1482 + 847 + 212 = 2541
+    testProc("Standard", 847, 1482, 212);
+    testProc("Converted1", 1482, 847, 212);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */

sw/source/filter/ww8/docxattributeoutput.hxx

@@ -67,6 +67,8 @@ enum DocxColBreakStatus
     COLBRK_WRITE
 };
 
+struct BorderDistances;
+
 /**
  * A structure that holds information about the options selected
  * when outputting a border to DOCX.
@@ -79,15 +81,13 @@ enum DocxColBreakStatus
  */
 struct OutputBorderOptions
 {
-    sal_Int32           tag;
-    bool                bUseStartEnd;
-    bool                bWriteTag;
-    bool                bWriteInsideHV;
-    bool                bWriteDistance;
-    SvxShadowLocation   aShadowLocation;
-    bool                bCheckDistanceSize;
-
-    OutputBorderOptions() : tag(0), bUseStartEnd(false), bWriteTag(true), bWriteInsideHV(false), bWriteDistance(false), aShadowLocation(SVX_SHADOW_NONE), bCheckDistanceSize(false) {}
+    sal_Int32           tag = 0;
+    bool                bUseStartEnd = false;
+    bool                bWriteTag = true;
+    bool                bWriteInsideHV = false;
+    bool                bWriteDistance = false;
+    SvxShadowLocation   aShadowLocation = SVX_SHADOW_NONE;
+    std::shared_ptr<BorderDistances> pDistances;
 };
 
 /**

writerfilter/source/dmapper/DomainMapper.cxx

@@ -2102,7 +2102,6 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext )
 
             // Set the borders to the context and apply them to the styles
             pHandler->SetBorders( pSectionContext );
-            pSectionContext->SetBorderParams( pHandler->GetDisplayOffset( ) );
         }
     }
     break;

writerfilter/source/dmapper/PageBordersHandler.cxx

@@ -37,8 +37,8 @@ PgBorder::~PgBorder( )
 
 PageBordersHandler::PageBordersHandler( ) :
 LoggedProperties("PageBordersHandler"),
-m_nDisplay( 0 ),
-m_nOffset( 0 )
+m_eBorderApply(SectionPropertyMap::BorderApply::ToAllInSection),
+m_eOffsetFrom(SectionPropertyMap::BorderOffsetFrom::Text)
 {
 }
 
@@ -57,13 +57,13 @@ void PageBordersHandler::lcl_attribute( Id eName, Value& rVal )
             {
                 default:
                 case NS_ooxml::LN_Value_doc_ST_PageBorderDisplay_allPages:
-                    m_nDisplay = 0;
+                    m_eBorderApply = SectionPropertyMap::BorderApply::ToAllInSection;
                     break;
                 case NS_ooxml::LN_Value_doc_ST_PageBorderDisplay_firstPage:
-                    m_nDisplay = 1;
+                    m_eBorderApply = SectionPropertyMap::BorderApply::ToFirstPageInSection;
                     break;
                 case NS_ooxml::LN_Value_doc_ST_PageBorderDisplay_notFirstPage:
-                    m_nDisplay = 2;
+                    m_eBorderApply = SectionPropertyMap::BorderApply::ToAllButFirstInSection;
                     break;
             }
         }
@@ -74,10 +74,10 @@ void PageBordersHandler::lcl_attribute( Id eName, Value& rVal )
             {
                 default:
                 case NS_ooxml::LN_Value_doc_ST_PageBorderOffset_page:
-                    m_nOffset = 1;
+                    m_eOffsetFrom = SectionPropertyMap::BorderOffsetFrom::Edge;
                     break;
                 case NS_ooxml::LN_Value_doc_ST_PageBorderOffset_text:
-                    m_nOffset = 0;
+                    m_eOffsetFrom = SectionPropertyMap::BorderOffsetFrom::Text;
                     break;
             }
         }
@@ -137,6 +137,8 @@ void PageBordersHandler::SetBorders( SectionPropertyMap* pSectContext )
     {
         pSectContext->SetBorder( rBorder.m_ePos, rBorder.m_nDistance, rBorder.m_rLine, rBorder.m_bShadow );
     }
+    pSectContext->SetBorderApply(m_eBorderApply);
+    pSectContext->SetBorderOffsetFrom(m_eOffsetFrom);
 }
 
 } }

writerfilter/source/dmapper/PageBordersHandler.hxx

@@ -50,8 +50,8 @@ class PageBordersHandler : public LoggedProperties
 private:
 
     // See implementation of SectionPropertyMap::ApplyBorderToPageStyles
-    sal_Int32 m_nDisplay;
-    sal_Int32 m_nOffset;
+    SectionPropertyMap::BorderApply m_eBorderApply;
+    SectionPropertyMap::BorderOffsetFrom m_eOffsetFrom;
     std::vector<PgBorder> m_aBorders;
 
     // Properties
@@ -62,10 +62,6 @@ public:
     PageBordersHandler( );
     virtual ~PageBordersHandler( ) override;
 
-    inline sal_Int32 GetDisplayOffset( )
-    {
-        return ( m_nOffset << 5 ) + m_nDisplay;
-    };
     void SetBorders( SectionPropertyMap* pSectContext );
 };
 

writerfilter/source/dmapper/PropertyMap.cxx

@@ -395,7 +395,8 @@ void PropertyMap::printProperties()
 
 SectionPropertyMap::SectionPropertyMap(bool bIsFirstSection) :
     m_bIsFirstSection( bIsFirstSection )
-    ,m_nBorderParams( 0 )
+    ,m_eBorderApply( BorderApply::ToAllInSection )
+    ,m_eBorderOffsetFrom( BorderOffsetFrom::Text )
     ,m_bTitlePage( false )
     ,m_nColumnCount( 0 )
     ,m_nColumnDistance( 1249 )
@@ -568,7 +569,7 @@ void SectionPropertyMap::SetBorder( BorderPosition ePos, sal_Int32 nLineDistance
 void SectionPropertyMap::ApplyBorderToPageStyles(
             const uno::Reference< container::XNameContainer >& xPageStyles,
             const uno::Reference < lang::XMultiServiceFactory >& xTextFactory,
-        sal_Int32 nValue )
+            BorderApply eBorderApply, BorderOffsetFrom eOffsetFrom)
 {
             /*
             page border applies to:
@@ -585,26 +586,25 @@ void SectionPropertyMap::ApplyBorderToPageStyles(
             */
     uno::Reference< beans::XPropertySet >  xFirst;
     uno::Reference< beans::XPropertySet >  xSecond;
-    sal_Int32 nOffsetFrom = (nValue & 0x00E0) >> 5;
     //todo: negative spacing (from ww8par6.cxx)
-    switch( nValue & 0x07)
+    switch( eBorderApply )
     {
-        case 0: /*all styles*/
+        case BorderApply::ToAllInSection: // all styles
             if ( !m_sFollowPageStyleName.isEmpty() )
                 xFirst = GetPageStyle( xPageStyles, xTextFactory, false );
             if ( !m_sFirstPageStyleName.isEmpty() )
                 xSecond = GetPageStyle( xPageStyles, xTextFactory, true );
         break;
-        case 1: /*first page*/
+        case BorderApply::ToFirstPageInSection: /*first page*/
             if ( !m_sFirstPageStyleName.isEmpty() )
                 xFirst = GetPageStyle( xPageStyles, xTextFactory, true );
         break;
-        case 2: /*left and right*/
+        case BorderApply::ToAllButFirstInSection: /*left and right*/
             if ( !m_sFollowPageStyleName.isEmpty() )
-                xFirst  = GetPageStyle( xPageStyles, xTextFactory, false );
+                xFirst = GetPageStyle( xPageStyles, xTextFactory, false );
         break;
-        case 3: //whole document?
-            //todo: how to apply a border to the whole document - find all sections or access all page styles?
+        case BorderApply::ToWholeDocument: //whole document?
+            // todo: how to apply a border to the whole document - find all sections or access all page styles?
         default:
             return;
     }
@@ -648,10 +648,10 @@ void SectionPropertyMap::ApplyBorderToPageStyles(
                 nLineWidth = m_oBorderLines[nBorder]->LineWidth;
             if(xFirst.is())
                 SetBorderDistance( xFirst, aMarginIds[nBorder], aBorderDistanceIds[nBorder],
-                  m_nBorderDistances[nBorder], nOffsetFrom, nLineWidth );
-            if(xSecond.is())
+                    m_nBorderDistances[nBorder], eOffsetFrom, nLineWidth );
+            if ( xSecond.is() )
                 SetBorderDistance( xSecond, aMarginIds[nBorder], aBorderDistanceIds[nBorder],
-                      m_nBorderDistances[nBorder], nOffsetFrom, nLineWidth );
+                    m_nBorderDistances[nBorder], eOffsetFrom, nLineWidth );
         }
     }
 
@@ -679,25 +679,46 @@ table::ShadowFormat PropertyMap::getShadowFromBorder(const table::BorderLine2& r
 }
 
 void SectionPropertyMap::SetBorderDistance( uno::Reference< beans::XPropertySet > const& xStyle,
-        PropertyIds eMarginId, PropertyIds eDistId, sal_Int32 nDistance, sal_Int32 nOffsetFrom, sal_uInt32 nLineWidth )
+        PropertyIds eMarginId, PropertyIds eDistId, sal_Int32 nDistance, BorderOffsetFrom eOffsetFrom, sal_uInt32 nLineWidth )
 {
-    sal_Int32 nDist = nDistance;
-    if( nOffsetFrom == 1 ) // From page
-    {
-        const OUString sMarginName = getPropertyName( eMarginId );
-        uno::Any aMargin = xStyle->getPropertyValue( sMarginName );
-        sal_Int32 nMargin = 0;
-        aMargin >>= nMargin;
+    // See https://wiki.openoffice.org/wiki/Writer/MSInteroperability/PageBorder
 
-        // Change the margins with the border distance
-        xStyle->setPropertyValue( sMarginName, uno::makeAny( nDistance ) );
+    if (!xStyle.is())
+        return;
+    const OUString sMarginName = getPropertyName( eMarginId );
+    const OUString sBorderDistanceName = getPropertyName( eDistId );
+    uno::Any aMargin = xStyle->getPropertyValue( sMarginName );
+    sal_Int32 nMargin = 0;
+    aMargin >>= nMargin;
+    sal_Int32 nNewMargin = nMargin;
+    sal_Int32 nNewDist = nDistance;
 
-        // Set the distance to ( Margin - distance - nLineWidth )
-        nDist = nMargin - nDistance - nLineWidth;
+    switch (eOffsetFrom)
+    {
+    case BorderOffsetFrom::Text:
+        nNewMargin -= nDistance + nLineWidth;
+        break;
+    case BorderOffsetFrom::Edge:
+        nNewMargin = nDistance;
+        nNewDist = nMargin - nDistance - nLineWidth;
+        break;
     }
-    const OUString sBorderDistanceName = getPropertyName( eDistId );
-    if (xStyle.is())
-        xStyle->setPropertyValue( sBorderDistanceName, uno::makeAny( nDist ));
+    // Ensure corrent distance from page edge to text in cases not supported by us:
+    // when border is outside entire page area (eOffsetFrom == Text && nDistance > nMargin),
+    // and when border is inside page body area (eOffsetFrom == Edge && nDistance > nMargin)
+    if (nNewMargin < 0)
+    {
+        nNewMargin = 0;
+        nNewDist = std::max<sal_Int32>(nMargin - nLineWidth, 0);
+    }
+    else if (nNewDist < 0)
+    {
+        nNewMargin = std::max<sal_Int32>(nMargin - nLineWidth, 0);
+        nNewDist = 0;
+    }
+    // Change the margins with the border distance
+    xStyle->setPropertyValue( sMarginName, uno::makeAny( nNewMargin ) );
+    xStyle->setPropertyValue( sBorderDistanceName, uno::makeAny( nNewDist ) );
 }
 
 void SectionPropertyMap::DontBalanceTextColumns()
@@ -1408,7 +1429,7 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl )
                     getPropertyName( PROP_TEXT_COLUMNS ), uno::makeAny( xColumns ));
         }
 
-        ApplyBorderToPageStyles( rDM_Impl.GetPageStyles( ), rDM_Impl.GetTextFactory( ), m_nBorderParams );
+        ApplyBorderToPageStyles( rDM_Impl.GetPageStyles(), rDM_Impl.GetTextFactory(), m_eBorderApply, m_eBorderOffsetFrom );
 
         try
         {

writerfilter/source/dmapper/PropertyMap.hxx

@@ -172,6 +172,20 @@ typedef std::shared_ptr<PropertyMap>  PropertyMapPtr;
 
 class SectionPropertyMap : public PropertyMap
 {
+public:
+    enum class BorderApply
+    {
+        ToAllInSection = 0,
+        ToFirstPageInSection = 1,
+        ToAllButFirstInSection = 2,
+        ToWholeDocument = 3,
+    };
+    enum class BorderOffsetFrom
+    {
+        Text = 0,
+        Edge = 1,
+    };
+private:
     //--> debug
     sal_Int32 nSectionNumber;
     //<-- debug
@@ -188,7 +202,8 @@ class SectionPropertyMap : public PropertyMap
 
     boost::optional<css::table::BorderLine2> m_oBorderLines[4];
     sal_Int32                               m_nBorderDistances[4];
-    sal_Int32                               m_nBorderParams;
+    BorderApply                             m_eBorderApply;
+    BorderOffsetFrom                        m_eBorderOffsetFrom;
     bool                                    m_bBorderShadows[4];
 
     bool                                    m_bTitlePage;
@@ -261,7 +276,7 @@ class SectionPropertyMap : public PropertyMap
                            PropertyIds eMarginId,
                            PropertyIds eDistId,
                            sal_Int32 nDistance,
-                           sal_Int32 nOffsetFrom,
+                           BorderOffsetFrom eOffsetFrom,
                            sal_uInt32 nLineWidth);
     /// Determines if conversion of a given floating table is wanted or not.
     bool FloatingTableConversion(DomainMapper_Impl& rDM_Impl, FloatingTableInfo& rInfo);
@@ -297,7 +312,8 @@ public:
                 css::uno::RuntimeException, std::exception);
 
     void SetBorder(BorderPosition ePos, sal_Int32 nLineDistance, const css::table::BorderLine2& rBorderLine, bool bShadow);
-    void SetBorderParams( sal_Int32 nSet ) { m_nBorderParams = nSet; }
+    void SetBorderApply( BorderApply nSet ) { m_eBorderApply = nSet; }
+    void SetBorderOffsetFrom( BorderOffsetFrom nSet ) { m_eBorderOffsetFrom = nSet; }
 
     void SetColumnCount( sal_Int16 nCount ) { m_nColumnCount = nCount; }
     sal_Int16 ColumnCount() const { return m_nColumnCount; }
@@ -338,7 +354,7 @@ public:
     //determine which style gets the borders
     void ApplyBorderToPageStyles(const css::uno::Reference<css::container::XNameContainer>& xStyles,
                                  const css::uno::Reference<css::lang::XMultiServiceFactory>& xTextFactory,
-                                 sal_Int32 nValue);
+                                 BorderApply eBorderApply, BorderOffsetFrom eOffsetFrom);
 
     void CloseSectionGroup( DomainMapper_Impl& rDM_Impl );
     /// Handling of margins, header and footer for any kind of sections breaks.