sw/qa/extras/ooxmlexport/data/tdf106690.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport9.cxx | 9 ++++ writerfilter/source/dmapper/DomainMapper.cxx | 7 --- writerfilter/source/dmapper/DomainMapper_Impl.cxx | 45 +++++++++++++++++++++- writerfilter/source/dmapper/DomainMapper_Impl.hxx | 5 ++ 5 files changed, 59 insertions(+), 7 deletions(-)
New commits: commit 1bf7f6a1a50ee9f24a3687240fe6ae390b905a6b Author: Miklos Vajna <vmik...@collabora.co.uk> Date: Tue Apr 4 09:12:34 2017 +0200 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 <vmik...@collabora.co.uk> Tested-by: Jenkins <c...@libreoffice.org> 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 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf106690.docx differ 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 _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits