sw/qa/extras/ooxmlexport/data/tdf170003_bottomSpacing.docx |binary sw/qa/extras/ooxmlexport/data/tdf170119_bottomSpacing.docx |binary sw/qa/extras/ooxmlexport/ooxmlexport19.cxx | 5 sw/qa/extras/ooxmlexport/ooxmlexport25.cxx | 17 ++ sw/source/writerfilter/dmapper/PropertyMap.cxx | 90 ++++++++++--- sw/source/writerfilter/dmapper/PropertyMap.hxx | 2 6 files changed, 94 insertions(+), 20 deletions(-)
New commits: commit 582c032aaa4524abf5b5a630400cf7ecdc2acf8a Author: Justin Luth <[email protected]> AuthorDate: Wed Dec 24 17:13:44 2025 -0500 Commit: Justin Luth <[email protected]> CommitDate: Thu Dec 25 03:38:27 2025 +0100 tdf#170119 writerfilter: cont-sectPr w/ pgbrk == emulate bottomSpacing Technically this is a 25.2.6 regression from my commit 74c29345a7c179b048c157582a1145e381616e5c tdf#167657 writerfilter: only move sectPr bottomMargin after pageBreak Reviewed-on: https://gerrit.libreoffice.org/c/core/+/188298 It was just lucky before that it worked for that particular instance, and it only worked since 25.2.4... make CppunitTest_sw_ooxmlexport25 \ CPPUNIT_TEST_NAME=testTdf170119_bottomSpacing Change-Id: I9d0ae90087d79887489357eaa00eeabe53bcb58c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/196205 Tested-by: Jenkins Reviewed-by: Justin Luth <[email protected]> diff --git a/sw/qa/extras/ooxmlexport/data/tdf170119_bottomSpacing.docx b/sw/qa/extras/ooxmlexport/data/tdf170119_bottomSpacing.docx new file mode 100644 index 000000000000..74e9de4f5272 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf170119_bottomSpacing.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx index f59043f597b5..dadc90469f9a 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx @@ -107,6 +107,14 @@ DECLARE_OOXMLEXPORT_TEST(testTdf170003_bottomSpacing, "tdf170003_bottomSpacing.d getProperty<sal_Int32>(getParagraph(1), u"ParaBottomMargin"_ustr)); } +DECLARE_OOXMLEXPORT_TEST(testTdf170119_bottomSpacing, "tdf170119_bottomSpacing.docx") +{ + // Given a document with a page break and a sectPr with a huge bottom spacing + + // Without the fix, page 2 started with a 150pt gap, pushing content to the third page. + CPPUNIT_ASSERT_EQUAL(2, getPages()); +} + DECLARE_OOXMLEXPORT_TEST(testTdf167657_sectPr_bottomSpacing, "tdf167657_sectPr_bottomSpacing.docx") { // given with a continuous break sectPr with no belowSpacing diff --git a/sw/source/writerfilter/dmapper/PropertyMap.cxx b/sw/source/writerfilter/dmapper/PropertyMap.cxx index 6d7c2d4768fa..9125c36c69ce 100644 --- a/sw/source/writerfilter/dmapper/PropertyMap.cxx +++ b/sw/source/writerfilter/dmapper/PropertyMap.cxx @@ -1560,6 +1560,22 @@ void SectionPropertyMap::EmulateSectPrBelowSpacing(DomainMapper_Impl& rDM_Impl) // to the last paragraph in that section. // This emulation works because below spacing before a page break normally has no relevance. + if (m_nBreakType == NS_ooxml::LN_Value_ST_SectionMark_continuous) + { + // The complication with continuous breaks is that the below spacing of a sectPr + // does NOT directly affect the layout. + // [So (except before a page break) it MUST NOT be applied to the previous para.] + // However, it IS (indirectly) used to reduce the top margin of the following paragraph. + uno::Reference<beans::XPropertySet> const xPSet(m_xStartingRange, uno::UNO_QUERY); + if (!xPSet) + return; // TODO tdf#170119: emulation not possible without a page break + + style::BreakType eBreakType(style::BreakType_NONE); + xPSet->getPropertyValue(u"BreakType"_ustr) >>= eBreakType; + if (eBreakType != style::BreakType_PAGE_BEFORE) + return; // emulation not possible without a page break. + } + // m_xPreStartingRange may have skipped over a table as the last thing before the break! // If so, then the below spacing can be ignored since tables don't have below spacing. // Also, if m_xStartingRange starts with a table (which also doesn't have above spacing) @@ -1690,6 +1706,9 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl ) setHeaderFooterProperties(rDM_Impl); InheritOrFinalizePageStyles( rDM_Impl ); ApplySectionProperties( xSection, rDM_Impl ); //depends on InheritOrFinalizePageStyles + + EmulateSectPrBelowSpacing(rDM_Impl); + uno::Reference< beans::XPropertySet > xRangeProperties( lcl_GetRangeProperties( m_bIsFirstSection, rDM_Impl, m_xStartingRange ) ); if ( m_bIsFirstSection && !m_sPageStyleName.isEmpty() && xRangeProperties.is() ) { commit 82936569e0e17705b6de501d7151549c51520fb9 Author: Justin Luth <[email protected]> AuthorDate: Mon Dec 22 20:22:11 2025 -0500 Commit: Justin Luth <[email protected]> CommitDate: Thu Dec 25 03:38:12 2025 +0100 tdf#170003 writerfilter: no belowSpacing emulation with tables This patch fixes my 25.2.6 commit 1326e09d019f05a82265f15c26288b4ffb7dc0c2 tdf#167657 writerfilter: only move sectPr bottomMargin after pageBreak It turns out that m_xPreStartingRange is a bit of a disaster. First of all, it seems to be set even if gotoPreviousParagraph fails. That can't be a good thing. Secondly (and the problem for me), it jumps over tables, so if the page break happens after a table then I was applying the below spacing to an earlier paragraph. make CppunitTest_sw_ooxmlexport25 \ CPPUNIT_TEST_NAME=testTdf170003_bottomSpacing Change-Id: I883dd397413001fafb000e1e90c0e6738b3b4c87 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/196143 Reviewed-by: Justin Luth <[email protected]> Tested-by: Jenkins diff --git a/sw/qa/extras/ooxmlexport/data/tdf170003_bottomSpacing.docx b/sw/qa/extras/ooxmlexport/data/tdf170003_bottomSpacing.docx new file mode 100644 index 000000000000..9f48b950122b Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf170003_bottomSpacing.docx differ diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx index 5dd58fd57536..523169f604ee 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport19.cxx @@ -1097,6 +1097,11 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf155945) // - Actual : 423 CPPUNIT_ASSERT_EQUAL(sal_Int32(0), getProperty<sal_Int32>(getParagraph(2), u"ParaBottomMargin"_ustr)); + + //tdf#170003: sectPr emulation: move sectPr bottom margin to the paragraph before the page break + // Without a fix in place, this was 0. + CPPUNIT_ASSERT_EQUAL(sal_Int32(423), + getProperty<sal_Int32>(getParagraph(1), u"ParaBottomMargin"_ustr)); } CPPUNIT_TEST_FIXTURE(Test, testTdf133560) diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx index b1b88739f861..f59043f597b5 100644 --- a/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx +++ b/sw/qa/extras/ooxmlexport/ooxmlexport25.cxx @@ -98,6 +98,15 @@ DECLARE_OOXMLEXPORT_TEST(testTdf169986_bottomSpacing, "tdf169986_bottomSpacing.d assertXPath(pLayout, "/root/page", 1); } +DECLARE_OOXMLEXPORT_TEST(testTdf170003_bottomSpacing, "tdf170003_bottomSpacing.docx") +{ + // Given a document with a table before the page break, and a sectPr with a huge bottom spacing + + // This must be 200 twips / 0.35cm / 353 mm100, not sectPr's 2000 twips / 3.53 cm / 3530 mm100 + CPPUNIT_ASSERT_EQUAL(sal_Int32(353), + getProperty<sal_Int32>(getParagraph(1), u"ParaBottomMargin"_ustr)); +} + DECLARE_OOXMLEXPORT_TEST(testTdf167657_sectPr_bottomSpacing, "tdf167657_sectPr_bottomSpacing.docx") { // given with a continuous break sectPr with no belowSpacing diff --git a/sw/source/writerfilter/dmapper/PropertyMap.cxx b/sw/source/writerfilter/dmapper/PropertyMap.cxx index d04774fbab64..6d7c2d4768fa 100644 --- a/sw/source/writerfilter/dmapper/PropertyMap.cxx +++ b/sw/source/writerfilter/dmapper/PropertyMap.cxx @@ -1542,6 +1542,54 @@ void SectionPropertyMap::CreateEvenOddPageStyleCopy(DomainMapper_Impl& rDM_Impl, m_sPageStyleName = evenOddStyleName; // And use it instead of the original one (which is set as follow of this one). } +void SectionPropertyMap::EmulateSectPrBelowSpacing(DomainMapper_Impl& rDM_Impl) +{ + if (!m_xStartingRange || !m_xPreStartingRange || rDM_Impl.IsInFootOrEndnote()) + return; + + SectionPropertyMap* pPrevSection = rDM_Impl.GetLastSectionContext(); + if (!pPrevSection || !pPrevSection->GetBelowSpacing().has_value()) + return; // no consolidated spacing to emulate + + // MS Word is excessively consistent about consolidating paragraph top and bottom spacing. + // They even consolidate spacing between section breaks! + // The complication here is that the sectPr paragraph (which Writer needs to discard) + // defines the bottom spacing that exists at the end of the section. + + // Emulation: apply the previous section's belowSpacing + // to the last paragraph in that section. + // This emulation works because below spacing before a page break normally has no relevance. + + // m_xPreStartingRange may have skipped over a table as the last thing before the break! + // If so, then the below spacing can be ignored since tables don't have below spacing. + // Also, if m_xStartingRange starts with a table (which also doesn't have above spacing) + // then again the below spacing can be ignored since no consolidation is needed. + auto pCursor = dynamic_cast<SwXTextCursor*>(m_xStartingRange.get()); + if (!pCursor || pCursor->GetPaM()->GetPointNode().FindTableNode()) + return; // no emulation needed: section starts with a table (i.e. a zero top margin) + + SwPaM aPaM(pCursor->GetPaM()->GetPointNode()); // at start of section, contentIndex(0) + if (!aPaM.Move(fnMoveBackward, GoInContent)) // now at end of previous section + assert(false && "impossible: with a previous section, this must be able to move backwards"); + + if (aPaM.GetPointNode().FindTableNode()) + return; // no emulation possible: DOCX tables don't have bottom spacing + // TODO: somehow emulate on tables - but only if it is round-trippable... + + try + { + const uno::Any aBelowSpacingOfPrevSection(*pPrevSection->GetBelowSpacing()); + const OUString sProp(getPropertyName(PROP_PARA_BOTTOM_MARGIN)); + uno::Reference<beans::XPropertySet> xLastParaInPrevSection(m_xPreStartingRange, + uno::UNO_QUERY_THROW); + xLastParaInPrevSection->setPropertyValue(sProp, aBelowSpacingOfPrevSection); + } + catch (uno::Exception&) + { + TOOLS_WARN_EXCEPTION("writerfilter", "Failed to transfer below spacing to last para."); + } +} + void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl ) { SectionPropertyMap* pPrevSection = rDM_Impl.GetLastSectionContext(); @@ -1941,26 +1989,7 @@ void SectionPropertyMap::CloseSectionGroup( DomainMapper_Impl& rDM_Impl ) ApplyBorderToPageStyles( rDM_Impl, m_eBorderApply, m_eBorderOffsetFrom ); ApplyPaperSource(rDM_Impl); - // Emulation: now apply the previous section's unused belowSpacing - // to the last paragraph before the section page break - // so that the consolidation/collapse of this section's 1st paragraph's aboveSpacing - // works correctly (since below spacing before a page break otherwise has no relevance). - if (pPrevSection && pPrevSection->GetBelowSpacing().has_value() && m_xPreStartingRange.is() - && !rDM_Impl.IsInFootOrEndnote()) - { - try - { - const uno::Any aBelowSpacingOfPrevSection(*pPrevSection->GetBelowSpacing()); - const OUString sProp(getPropertyName(PROP_PARA_BOTTOM_MARGIN)); - uno::Reference<beans::XPropertySet> xLastParaInPrevSection(m_xPreStartingRange, - uno::UNO_QUERY_THROW); - xLastParaInPrevSection->setPropertyValue(sProp, aBelowSpacingOfPrevSection); - } - catch (uno::Exception&) - { - TOOLS_WARN_EXCEPTION("writerfilter", "Transfer below spacing to last para."); - } - } + EmulateSectPrBelowSpacing(rDM_Impl); try { @@ -2176,6 +2205,8 @@ void SectionPropertyMap::SetStart( const uno::Reference< text::XTextRange >& xRa { uno::Reference<text::XParagraphCursor> const xPCursor( m_xStartingRange->getText()->createTextCursorByRange(m_xStartingRange), uno::UNO_QUERY_THROW); + // CAUTION: gotoPreviousParagraph skips over tables, + // so this range does not necessarily indicate the end of the previous section xPCursor->gotoPreviousParagraph(false); m_xPreStartingRange = xPCursor; } diff --git a/sw/source/writerfilter/dmapper/PropertyMap.hxx b/sw/source/writerfilter/dmapper/PropertyMap.hxx index ad066ecf5d6a..31ee213ccc4b 100644 --- a/sw/source/writerfilter/dmapper/PropertyMap.hxx +++ b/sw/source/writerfilter/dmapper/PropertyMap.hxx @@ -341,6 +341,8 @@ private: void CreateEvenOddPageStyleCopy(DomainMapper_Impl& rDM_Impl, PageBreakType eBreakType); + void EmulateSectPrBelowSpacing(DomainMapper_Impl& rDM_Impl); + void PrepareHeaderFooterProperties(); bool HasHeader() const;
