tdf#104354 writerfilter: rewrite Autospacing

which also resolves a regression from tdf#113258.

The previous code segments were being spread out all over
the place. It all consolidates nicely in finishParagraph,
and the code is much easier to read using the new
GetAnyProperty function.

Plus there were regressions creeping in.
The mere presense of the Autospacing property normally, but
not necessarily means that it is on. Verify that it is enabled,
and update grabbag if autoSpacing changes.

Additionally, support was added for a zero top margin for
the first paragraph of the document.

This will be too hard to backport since it depends on
150c12fc0fba2c2f4b08b4298678ee49676ebae0 from tdf#72560
and the many code fixes related to GetPropertyFromStyleSheets.

Change-Id: Iaf1600fffea54e9800e215e89cad40006d5bcdda
Reviewed-on: https://gerrit.libreoffice.org/59705
Reviewed-by: László Németh <nemeth@numbertext.org>
Tested-by: László Németh <nemeth@numbertext.org>

6 files changed, 82 insertions, 112 deletions

diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index 4b07d253eb21..a85590baf5e7 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -646,54 +646,41 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
         // See SwWW8ImplReader::GetParagraphAutoSpace() on why these are 100 and 280
         case NS_ooxml::LN_CT_Spacing_beforeAutospacing:
         {
-            sal_Int32 default_spacing = 100;
-            if (!m_pImpl->GetSettingsTable()->GetDoNotUseHTMLParagraphAutoSpacing())
+            sal_Int32 default_spacing = -1;
+            if (nIntValue)
             {
-                // 49 is just the old value that should be removed, once the
-                // root cause in SwTabFrm::MakeAll() is fixed.
-                if (m_pImpl->GetSettingsTable()->GetView() == NS_ooxml::LN_Value_doc_ST_View_web)
-                    default_spacing = 49;
-                else
+                m_pImpl->SetParaAutoBefore(true);
+
+                default_spacing = 100;
+                if (!m_pImpl->GetSettingsTable()->GetDoNotUseHTMLParagraphAutoSpacing())
                 {
-                    // tdf#104354 first paragraphs of table cells and shapes get zero top margin
-                    if ((m_pImpl->GetIsFirstParagraphInSection() && m_pImpl->m_nTableDepth > 0) ||
-                         m_pImpl->GetIsFirstParagraphInShape())
-                        default_spacing = 0;
+                    // 49 is just the old value that should be removed, once the
+                    // root cause in SwTabFrm::MakeAll() is fixed.
+                    if (m_pImpl->GetSettingsTable()->GetView() == NS_ooxml::LN_Value_doc_ST_View_web)
+                        default_spacing = 49;
                     else
                         default_spacing = 280;
                 }
-            }
-            if  (nIntValue) // If auto spacing is set, then only store set value in InteropGrabBag
-            {
-                m_pImpl->SetParaAutoBefore(true);
-                m_pImpl->GetTopContext()->Insert( PROP_PARA_TOP_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ) );
-            }
-            else
-            {
-                default_spacing = -1;
+                // required at export (here mainly for StyleSheets) to determine if the setting has changed from grab_bag
+                m_pImpl->GetTopContext()->Insert(PROP_PARA_TOP_MARGIN, uno::makeAny(ConversionHelper::convertTwipToMM100(default_spacing)));
             }
             m_pImpl->GetTopContext()->Insert( PROP_PARA_TOP_MARGIN_BEFORE_AUTO_SPACING, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ),true, PARA_GRAB_BAG );
         }
         break;
         case NS_ooxml::LN_CT_Spacing_afterAutospacing:
         {
-            sal_Int32 default_spacing = 100;
-
-            if (!m_pImpl->GetSettingsTable()->GetDoNotUseHTMLParagraphAutoSpacing())
+            sal_Int32 default_spacing = -1;
+            if  (nIntValue)
             {
-                if (m_pImpl->GetSettingsTable()->GetView() == NS_ooxml::LN_Value_doc_ST_View_web)
-                    default_spacing = 49;
-                else
-                    default_spacing = 280;
-            }
-            if  (nIntValue) // If auto spacing is set, then only store set value in InteropGrabBag
-            {
-                m_pImpl->SetParaAutoAfter(true);
-                m_pImpl->GetTopContext()->Insert( PROP_PARA_BOTTOM_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ) );
-            }
-            else
-            {
-                default_spacing = -1;
+                default_spacing = 100;
+                if (!m_pImpl->GetSettingsTable()->GetDoNotUseHTMLParagraphAutoSpacing())
+                {
+                    if (m_pImpl->GetSettingsTable()->GetView() == NS_ooxml::LN_Value_doc_ST_View_web)
+                        default_spacing = 49;
+                    else
+                        default_spacing = 280;
+                }
+                m_pImpl->GetTopContext()->Insert(PROP_PARA_BOTTOM_MARGIN, uno::makeAny(ConversionHelper::convertTwipToMM100(default_spacing)));
             }
             m_pImpl->GetTopContext()->Insert( PROP_PARA_BOTTOM_MARGIN_AFTER_AUTO_SPACING, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ),true, PARA_GRAB_BAG );
         }
@@ -2110,27 +2097,7 @@ void DomainMapper::sprmWithProps( Sprm& rSprm, const PropertyMapPtr& rContext )
         const OUString sConvertedStyleName = pStyleTable->ConvertStyleName( sStringValue, true );
         m_pImpl->SetCurrentParaStyleName( sConvertedStyleName );
         if (m_pImpl->GetTopContext() && m_pImpl->GetTopContextType() != CONTEXT_SECTION)
-        {
             m_pImpl->GetTopContext()->Insert( PROP_PARA_STYLE_NAME, uno::makeAny( sConvertedStyleName ));
-
-            if (m_pImpl->GetIsFirstParagraphInShape())
-            {
-                // First paragraph in shape: see if we need to disable
-                // paragraph top margin from style.
-                StyleSheetEntryPtr pEntry
-                    = m_pImpl->GetStyleSheetTable()->FindStyleSheetByConvertedStyleName(
-                        sConvertedStyleName);
-                if (pEntry)
-                {
-                    boost::optional<PropertyMap::Property> pParaAutoBefore
-                        = pEntry->pProperties->getProperty(
-                            PROP_PARA_TOP_MARGIN_BEFORE_AUTO_SPACING);
-                    if (pParaAutoBefore)
-                        m_pImpl->GetTopContext()->Insert(PROP_PARA_TOP_MARGIN,
-                                                         uno::makeAny(static_cast<sal_Int32>(0)));
-                }
-            }
-        }
     }
     break;
     case NS_ooxml::LN_EG_RPrBase_rStyle:
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index c1a579b308f8..12bd911f5e59 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -253,11 +253,9 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_vTextFramesForChaining(),
         m_bParaHadField(false),
         m_bParaAutoBefore(false),
-        m_bParaAutoAfter(false),
-        m_bPrevParaAutoAfter(false),
-        m_bParaChangedBottomMargin(false),
         m_bFirstParagraphInCell(true),
         m_bSaveFirstParagraphInCell(false)
+
 {
     m_aBaseUrl = rMediaDesc.getUnpackedValueOrDefault(
         utl::MediaDescriptor::PROP_DOCUMENTBASEURL(), OUString());
@@ -1236,6 +1234,48 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
             pParaContext->Insert( PROP_NUMBERING_LEVEL, uno::makeAny(pStyleSheetProperties->GetListLevel()), false);
     }
 
+    // apply AutoSpacing: it has priority over all other margin settings
+    // (note that numbering with autoSpacing is handled separately later on)
+    const bool bAllowAdjustments = !GetSettingsTable()->GetDoNotUseHTMLParagraphAutoSpacing();
+    sal_Int32 nBeforeAutospacing = -1;
+    bool bIsAutoSet = pParaContext && pParaContext->isSet(PROP_PARA_TOP_MARGIN_BEFORE_AUTO_SPACING);
+    // apply INHERITED autospacing only if top margin is not set
+    if ( bIsAutoSet || (pParaContext && !pParaContext->isSet(PROP_PARA_TOP_MARGIN)) )
+        GetAnyProperty(PROP_PARA_TOP_MARGIN_BEFORE_AUTO_SPACING, pPropertyMap) >>= nBeforeAutospacing;
+    if ( nBeforeAutospacing > -1 && pParaContext )
+    {
+        if ( bAllowAdjustments )
+        {
+            if ( GetIsFirstParagraphInShape() ||
+                 (GetIsFirstParagraphInSection() && GetSectionContext() && GetSectionContext()->IsFirstSection()) ||
+                 (m_bFirstParagraphInCell && m_nTableDepth > 0 && m_nTableDepth == m_nTableCellDepth) )
+            {
+                nBeforeAutospacing = 0;
+                // export requires grabbag to match top_margin, so keep them in sync
+                if ( bIsAutoSet )
+                    pParaContext->Insert( PROP_PARA_TOP_MARGIN_BEFORE_AUTO_SPACING, uno::makeAny( sal_Int32(0) ),true, PARA_GRAB_BAG );
+            }
+        }
+        pParaContext->Insert(PROP_PARA_TOP_MARGIN, uno::makeAny(nBeforeAutospacing));
+    }
+
+    sal_Int32 nAfterAutospacing = -1;
+    bIsAutoSet = pParaContext && pParaContext->isSet(PROP_PARA_BOTTOM_MARGIN_AFTER_AUTO_SPACING);
+    bool bApplyAutospacing = bIsAutoSet || (pParaContext && !pParaContext->isSet(PROP_PARA_BOTTOM_MARGIN));
+    if ( bApplyAutospacing )
+        GetAnyProperty(PROP_PARA_BOTTOM_MARGIN_AFTER_AUTO_SPACING, pPropertyMap) >>= nAfterAutospacing;
+    if ( nAfterAutospacing > -1 && pParaContext )
+    {
+        pParaContext->Insert(PROP_PARA_BOTTOM_MARGIN, uno::makeAny(nAfterAutospacing));
+        bApplyAutospacing = bAllowAdjustments;
+    }
+    else
+        bApplyAutospacing = false;
+
+    // tell TableManager to reset the bottom margin if it determines that this is the cell's last paragraph.
+    if ( hasTableManager() && getTableManager().isInCell() )
+        getTableManager().setCellLastParaAfterAutospacing( bApplyAutospacing );
+
 
     if (xTextAppend.is() && pParaContext && hasTableManager() && !getTableManager().isIgnore())
     {
@@ -1310,7 +1350,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                         lcl_AddRangeAndStyle(pToBeSavedProperties, xTextAppend, pPropertyMap, rAppendContext);
                     }
                 }
-
             }
             else
             {
@@ -1323,7 +1362,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                 }
             }
             std::vector<beans::PropertyValue> aProperties;
-            bool bNumberedParagraph = false;
             if (pPropertyMap.get())
                 aProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(pPropertyMap->GetPropertyValues());
             if( !bIsDropCap )
@@ -1368,7 +1406,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                     if (itNumberingRules != aProperties.end())
                     {
                         // This textnode has numbering. Look up the numbering style name of the current and previous paragraph.
-                        bNumberedParagraph = true;
                         OUString aCurrentNumberingRuleName;
                         uno::Reference<container::XNamed> xCurrentNumberingRules(itNumberingRules->Value, uno::UNO_QUERY);
                         if (xCurrentNumberingRules.is())
@@ -1450,30 +1487,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
                     uno::Reference< text::XTextRange > xParaEnd( xCur, uno::UNO_QUERY );
                     CheckParaMarkerRedline( xParaEnd );
                 }
-                // set top margin of the previous auto paragraph in cells, keeping zero top margin only at the first one
-                if (m_nTableDepth > 0 && m_nTableDepth == m_nTableCellDepth && m_xPreviousParagraph.is())
-                {
-                    bool bParaChangedTopMargin = std::any_of(aProperties.begin(), aProperties.end(), [](const beans::PropertyValue& rValue)
-                    {
-                        return rValue.Name == "ParaTopMargin";
-                    });
-
-                    uno::Sequence<beans::PropertyValue> aPrevPropertiesSeq;
-                    m_xPreviousParagraph->getPropertyValue("ParaInteropGrabBag") >>= aPrevPropertiesSeq;
-                    auto aPrevProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(aPrevPropertiesSeq);
-                    bool bPrevParaAutoBefore = std::any_of(aPrevProperties.begin(), aPrevProperties.end(), [](const beans::PropertyValue& rValue)
-                    {
-                        return rValue.Name == "ParaTopMarginBeforeAutoSpacing";
-                    });
-
-                    if ((bPrevParaAutoBefore && !bParaChangedTopMargin) || (bParaChangedTopMargin && m_bParaAutoBefore))
-                    {
-                        sal_Int32 nSize = (m_bFirstParagraphInCell || bNumberedParagraph) ? 0 : 280;
-                        // Previous before spacing is set to auto, set previous before space to 280, except in the first paragraph.
-                        m_xPreviousParagraph->setPropertyValue("ParaTopMargin",
-                                 uno::makeAny( ConversionHelper::convertTwipToMM100(nSize)));
-                    }
-                }
 
                 // tdf#118521 set paragraph top or bottom margin based on the paragraph style
                 // if we already set the other margin with direct formatting
@@ -1565,12 +1578,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
     if (m_bIsFirstParaInShape)
         m_bIsFirstParaInShape = false;
 
-    // keep m_bParaAutoAfter for table paragraphs
-    m_bPrevParaAutoAfter = m_bParaAutoAfter || m_bPrevParaAutoAfter;
-
-    // not auto margin in this paragraph
-    m_bParaChangedBottomMargin = (pParaContext && pParaContext->isSet(PROP_PARA_BOTTOM_MARGIN) && !m_bParaAutoAfter);
-
     if (pParaContext)
     {
         // Reset the frame properties for the next paragraph
@@ -1585,7 +1592,6 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
         m_bFirstParagraphInCell = false;
 
     m_bParaAutoBefore = false;
-    m_bParaAutoAfter = false;
 
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().endElement();
@@ -2508,21 +2514,12 @@ bool DomainMapper_Impl::IsDiscardHeaderFooter()
 void DomainMapper_Impl::ClearPreviousParagraph()
 {
     // in table cells, set bottom auto margin of last paragraph to 0, except in paragraphs with numbering
-    if ((m_nTableDepth == (m_nTableCellDepth + 1)) && m_xPreviousParagraph.is() && !m_bParaChangedBottomMargin)
+    if ((m_nTableDepth == (m_nTableCellDepth + 1))
+        && m_xPreviousParagraph.is()
+        && hasTableManager() && getTableManager().isCellLastParaAfterAutospacing())
     {
-        uno::Sequence<beans::PropertyValue> aPrevPropertiesSeq;
-        m_xPreviousParagraph->getPropertyValue("ParaInteropGrabBag") >>= aPrevPropertiesSeq;
-        auto aPrevProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(aPrevPropertiesSeq);
-        bool bPrevParaAutoAfter = std::any_of(aPrevProperties.begin(), aPrevProperties.end(), [](const beans::PropertyValue& rValue)
-        {
-            return rValue.Name == "ParaBottomMarginAfterAutoSpacing";
-        });
-
-        bool bPrevNumberingRules = false;
         uno::Reference<container::XNamed> xPreviousNumberingRules(m_xPreviousParagraph->getPropertyValue("NumberingRules"), uno::UNO_QUERY);
-        if (xPreviousNumberingRules.is())
-             bPrevNumberingRules = !xPreviousNumberingRules->getName().isEmpty();
-        if (!bPrevNumberingRules && (bPrevParaAutoAfter || m_bPrevParaAutoAfter))
+        if ( !xPreviousNumberingRules.is() || xPreviousNumberingRules->getName().isEmpty() )
             m_xPreviousParagraph->setPropertyValue("ParaBottomMargin", uno::makeAny(static_cast<sal_Int32>(0)));
     }
 
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 342fa4d0f006..5547af6b8cde 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -979,7 +979,6 @@ public:
     bool IsDiscardHeaderFooter();
 
     void SetParaAutoBefore(bool bParaAutoBefore) { m_bParaAutoBefore = bParaAutoBefore; }
-    void SetParaAutoAfter(bool bParaAutoAfter) { m_bParaAutoAfter = bParaAutoAfter; }
 
     /// Forget about the previous paragraph, as it's not inside the same
     /// start/end node.
@@ -993,11 +992,6 @@ private:
     css::uno::Reference<css::beans::XPropertySet> m_xPreviousParagraph;
     /// Current paragraph has automatic before spacing.
     bool m_bParaAutoBefore;
-    /// Current paragraph has automatic after spacing.
-    bool m_bParaAutoAfter;
-    /// Paragraph has direct top or bottom margin formattings
-    bool m_bPrevParaAutoAfter;
-    bool m_bParaChangedBottomMargin;
     /// Current paragraph in a table is first paragraph of a cell
     bool m_bFirstParagraphInCell;
     bool m_bSaveFirstParagraphInCell;
diff --git a/writerfilter/source/dmapper/PropertyMap.hxx b/writerfilter/source/dmapper/PropertyMap.hxx
index f43bb38cad77..660983a9eebe 100644
--- a/writerfilter/source/dmapper/PropertyMap.hxx
+++ b/writerfilter/source/dmapper/PropertyMap.hxx
@@ -306,6 +306,8 @@ public:
 
     explicit SectionPropertyMap( bool bIsFirstSection );
 
+    bool IsFirstSection() { return m_bIsFirstSection; }
+
     void SetStart( const css::uno::Reference< css::text::XTextRange >& xRange ) { m_xStartingRange = xRange; }
 
     const css::uno::Reference< css::text::XTextRange >& GetStartingRange() const { return m_xStartingRange; }
diff --git a/writerfilter/source/dmapper/TableManager.cxx b/writerfilter/source/dmapper/TableManager.cxx
index 486381a0f08d..bfe8618c8de8 100644
--- a/writerfilter/source/dmapper/TableManager.cxx
+++ b/writerfilter/source/dmapper/TableManager.cxx
@@ -463,6 +463,11 @@ void TableManager::setTableStartsAtCellStart(bool bTableStartsAtCellStart)
     m_bTableStartsAtCellStart = bTableStartsAtCellStart;
 }
 
+void TableManager::setCellLastParaAfterAutospacing(bool bIsAfterAutospacing)
+{
+    m_bCellLastParaAfterAutospacing = bIsAfterAutospacing;
+}
+
 TableManager::TableManager()
     : mnTableDepthNew(0), mnTableDepth(0), mbKeepUnfinishedRow(false),
       m_bTableStartsAtCellStart(false)
@@ -470,6 +475,7 @@ TableManager::TableManager()
     setRowEnd(false);
     setInCell(false);
     setCellEnd(false);
+    m_bCellLastParaAfterAutospacing = false;
 }
 
 }
diff --git a/writerfilter/source/dmapper/TableManager.hxx b/writerfilter/source/dmapper/TableManager.hxx
index 0be365ae4e33..02839c681e29 100644
--- a/writerfilter/source/dmapper/TableManager.hxx
+++ b/writerfilter/source/dmapper/TableManager.hxx
@@ -273,6 +273,8 @@ private:
     /// If this is a nested table, does it start at cell start?
     bool m_bTableStartsAtCellStart;
 
+    bool m_bCellLastParaAfterAutospacing;
+
     /**
        handler for resolveCurrentTable
      */
@@ -473,6 +475,8 @@ public:
 
 
     void setTableStartsAtCellStart(bool bTableStartsAtCellStart);
+    void setCellLastParaAfterAutospacing(bool bIsAfterAutospacing);
+    bool isCellLastParaAfterAutospacing() {return m_bCellLastParaAfterAutospacing;}
 };
 
 }