tdf#106690 DOCX import: fix automatic spacing before/after numbered para block

The context is text nodes with automatic before/after spacing and
numbering rules set, like:

A
* B
* C
* D
E

The correct behavior seems to be (though I haven't found this explicitly
written in the OOXML spec) to drop spacing between B and C and C and D,
but not before B and not after D. Originally no spacing was dropped,
then commit c486e875de7c8e845594f5043a37ee8800865782 (tdf#95031 DOCX
import: auto spacing inside numbering means no spacing, 2016-10-18)
removed spacing around all B/C/D.

Fix the problem by checking the numbering rules and automatic after
spacing of the previous paragraph, so spacing before B and after D is
not removed.

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

5 files changed, 59 insertions, 7 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf106690.docx b/sw/qa/extras/ooxmlexport/data/tdf106690.docx
new file mode 100644
index 000000000000..b233ef81c6cf
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf106690.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
index e7bc9cbb03ff..178c71b6fcc0 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport9.cxx
@@ -61,6 +61,15 @@ DECLARE_OOXMLEXPORT_TEST(testTdf95031, "tdf95031.docx")
     CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(0), getProperty<sal_Int32>(getParagraph(3), "ParaTopMargin"));
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf106690, "tdf106690.docx")
+{
+    // This was 0, numbering rules with automatic spacing meant 0
+    // before/autospacing for all text nodes, even for ones at the start/end of
+    // a numbered text node block.
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraph(2), "ParaBottomMargin"));
+    CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(494), getProperty<sal_Int32>(getParagraph(2), "ParaTopMargin"));
+}
+
 DECLARE_OOXMLEXPORT_TEST(testTdf89377, "tdf89377_tableWithBreakBeforeParaStyle.docx")
 {
     // the paragraph style should set table's text-flow break-before-page
diff --git a/writerfilter/source/dmapper/DomainMapper.cxx b/writerfilter/source/dmapper/DomainMapper.cxx
index b899810c8444..ca2091f77a0b 100644
--- a/writerfilter/source/dmapper/DomainMapper.cxx
+++ b/writerfilter/source/dmapper/DomainMapper.cxx
@@ -620,9 +620,7 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
             }
             if  (nIntValue) // If auto spacing is set, then only store set value in InteropGrabBag
             {
-                if (m_pImpl->GetTopContext()->isSet(PROP_NUMBERING_RULES))
-                    // Numbering is set -> auto space is 0.
-                    default_spacing = 0;
+                m_pImpl->SetParaAutoBefore(true);
                 m_pImpl->GetTopContext()->Insert( PROP_PARA_TOP_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ) );
             }
             else
@@ -645,9 +643,6 @@ void DomainMapper::lcl_attribute(Id nName, Value & val)
             }
             if  (nIntValue) // If auto spacing is set, then only store set value in InteropGrabBag
             {
-                if (m_pImpl->GetTopContext()->isSet(PROP_NUMBERING_RULES))
-                    // Numbering is set -> auto space is 0.
-                    default_spacing = 0;
                 m_pImpl->GetTopContext()->Insert( PROP_PARA_BOTTOM_MARGIN, uno::makeAny( ConversionHelper::convertTwipToMM100(default_spacing) ) );
             }
             else
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index 03e172cc32ac..866b42e5d85a 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -242,7 +242,8 @@ DomainMapper_Impl::DomainMapper_Impl(
         m_bFrameBtLr(false),
         m_bIsSplitPara(false),
         m_vTextFramesForChaining(),
-        m_bParaHadField(false)
+        m_bParaHadField(false),
+        m_bParaAutoBefore(false)
 
 {
     m_aBaseUrl = rMediaDesc.getUnpackedValueOrDefault(
@@ -1171,7 +1172,48 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
                         appendTextPortion(sMarker, pEmpty);
                     }
 
+                    // Check if top / bottom margin has to be updated, now that we know the numbering status of both the previous and
+                    // the current text node.
+                    auto itNumberingRules = std::find_if(aProperties.begin(), aProperties.end(), [](const beans::PropertyValue& rValue)
+                    {
+                        return rValue.Name == "NumberingRules";
+                    });
+                    if (itNumberingRules != aProperties.end())
+                    {
+                        // This textnode has numbering.
+                        if (m_xPreviousParagraph.is() && m_xPreviousParagraph->getPropertyValue("NumberingRules").hasValue())
+                        {
+                            // There was a previous textnode and it had numbering.
+                            if (m_bParaAutoBefore)
+                            {
+                                // This before spacing is set to auto, set before space to 0.
+                                auto itParaTopMargin = std::find_if(aProperties.begin(), aProperties.end(), [](const beans::PropertyValue& rValue)
+                                {
+                                    return rValue.Name == "ParaTopMargin";
+                                });
+                                if (itParaTopMargin != aProperties.end())
+                                    itParaTopMargin->Value <<= static_cast<sal_Int32>(0);
+                                else
+                                    aProperties.push_back(comphelper::makePropertyValue("ParaTopMargin", static_cast<sal_Int32>(0)));
+                            }
+                            uno::Sequence<beans::PropertyValue> aPrevPropertiesSeq;
+                            m_xPreviousParagraph->getPropertyValue("ParaInteropGrabBag") >>= aPrevPropertiesSeq;
+                            auto aPrevProperties = comphelper::sequenceToContainer< std::vector<beans::PropertyValue> >(aPrevPropertiesSeq);
+                            auto itPrevParaAutoAfter = std::find_if(aPrevProperties.begin(), aPrevProperties.end(), [](const beans::PropertyValue& rValue)
+                            {
+                                return rValue.Name == "ParaBottomMarginAfterAutoSpacing";
+                            });
+                            bool bPrevParaAutoAfter = itPrevParaAutoAfter != aPrevProperties.end();
+                            if (bPrevParaAutoAfter)
+                            {
+                                // Previous after spacing is set to auto, set previous after space to 0.
+                                m_xPreviousParagraph->setPropertyValue("ParaBottomMargin", uno::makeAny(static_cast<sal_Int32>(0)));
+                            }
+                        }
+                    }
+
                     xTextRange = xTextAppend->finishParagraph( comphelper::containerToSequence(aProperties) );
+                    m_xPreviousParagraph.set(xTextRange, uno::UNO_QUERY);
 
                     if (xCursor.is())
                     {
@@ -1229,6 +1271,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap )
 
     SetIsOutsideAParagraph(true);
     m_bParaHadField = false;
+    m_bParaAutoBefore = false;
 #ifdef DEBUG_WRITERFILTER
     TagLogger::getInstance().endElement();
 #endif
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.hxx b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
index 18ccb047d94e..82547b14325b 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.hxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.hxx
@@ -880,11 +880,16 @@ public:
 
     bool IsDiscardHeaderFooter();
 
+    void SetParaAutoBefore(bool bParaAutoBefore) { m_bParaAutoBefore = bParaAutoBefore; }
+
 private:
     void PushPageHeaderFooter(bool bHeader, SectionPropertyMap::PageType eType);
     std::vector<css::uno::Reference< css::drawing::XShape > > m_vTextFramesForChaining ;
     /// Current paragraph had at least one field in it.
     bool m_bParaHadField;
+    css::uno::Reference<css::beans::XPropertySet> m_xPreviousParagraph;
+    /// Current paragraph has automatic before spacing.
+    bool m_bParaAutoBefore;
 };
 
 } //namespace dmapper