tdf#133507 sw: hunt for documents with ULSpacing at section end

When a paragraph style has "contextualspacing" with Spacing Above paragraph,
and option "Don't add space between paragraphs of the same style,
the spacing should apply after a section end.

Contextual spacing was officially added to 1.3 spec in 7.0-ish timeframe:
"The space between the paragraphs is zero,
if all of the following conditions hold:
...
The paragraphs belong to the same content area.
...

Contextual spacing was actually added in LO 3.6,
and it applies as expected when transitioning
from one section to another.

The missing case was when transitioning from a section
back to the main body text.

This was an implementation error. So the question is
whether we add yet another compatibility flag
to handle the old way of doing things,
or just fix it.

I'm leaning towards no compat flag because
-it is a recent official addition to ODF
-it is an implementation error
-it currently is inconsistent - works on section start, but not end
-rarely used: it is primarily an interoperability feature
-rarely encountered: sections aren't really needed in LO.
-no existing unit tests matched the condition.
-interoperability isn't affected - section end == document end.

This patch is hunting to prove some of the above points.
-shouldn't hit any DOC/DOCX/RTF files.
-only expect to find ODT files related to this bug report.

So, I don't think a compat flag is warranted in this case.
It is easy enough for a human to "fix" the document
by removing the undesired "spacing before/after" settings.

make CppunitTest_sw_odfexport2 \
CPPUNIT_TEST_NAME=tdf133507_contextualSpacingSection

Change-Id: Ia041741bbada8c2f4cd97aa6b7cd98d4d7ffb275
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153033
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>

5 files changed, 26 insertions, 12 deletions

diff --git a/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
new file mode 100644
index 000000000000..a2a9ebd47198
--- /dev/null
+++ b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
diff --git a/sw/qa/extras/odfexport/odfexport2.cxx b/sw/qa/extras/odfexport/odfexport2.cxx
index 103892a7723a..46e192eaf3eb 100644
--- a/sw/qa/extras/odfexport/odfexport2.cxx
+++ b/sw/qa/extras/odfexport/odfexport2.cxx
@@ -242,6 +242,12 @@ DECLARE_ODFEXPORT_TEST(testSpellOutNumberingTypes, "spellout-numberingtypes.odt"
     }
 }
 
+// DECLARE_ODFEXPORT_TEST(tdf133507_contextualSpacingSection, "tdf133507_contextualSpacingSection.odt")
+// {
+//     // Previously this was one page (no UL spacing) or three pages (every para had spacing)
+//     CPPUNIT_ASSERT_EQUAL(2, getPages());
+// }
+
 // MAILMERGE Add conditional to expand / collapse bookmarks
 DECLARE_ODFEXPORT_TEST(tdf101856_overlapped, "tdf101856_overlapped.odt")
 {
diff --git a/sw/source/core/inc/frmtool.hxx b/sw/source/core/inc/frmtool.hxx
index ffc218b5504e..a497ea09e590 100644
--- a/sw/source/core/inc/frmtool.hxx
+++ b/sw/source/core/inc/frmtool.hxx
@@ -583,7 +583,7 @@ void GetSpacingValuesOfFrame( const SwFrame& rFrame,
                             SwTwips& onLowerSpacing,
                             SwTwips& onLineSpacing,
                             bool& obIsLineSpacingProportional,
-                            bool bIdenticalStyles );
+                            bool bIdenticalStyles, bool bIdenticalSections = true );
 
 /** method to get the content of the table cell
 
diff --git a/sw/source/core/layout/flowfrm.cxx b/sw/source/core/layout/flowfrm.cxx
index 41b6b8fff953..a2b9da301678 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -1464,9 +1464,20 @@ const SwFrame* SwFlowFrame::GetPrevFrameForUpperSpaceCalc_( const SwFrame* _pPro
     return pPrevFrame;
 }
 
+// This should be renamed to something like lcl_ApplyULSpacing
 /// Compare styles attached to these text frames.
 static bool lcl_IdenticalStyles(const SwFrame* pPrevFrame, const SwFrame* pFrame)
 {
+    if (!pFrame || !pFrame->IsTextFrame())
+        return false;
+
+    // Identical styles only applies if "the paragraphs belong to the same content area".
+    if (pPrevFrame && pPrevFrame->FindSctFrame() != pFrame->FindSctFrame())
+    {
+        SAL_WARN("sw","DEBUG prev["<<pPrevFrame->FindSctFrame()<<"] sct["<<pFrame->FindSctFrame()<<"]");
+        // return false;
+    }
+
     SwTextFormatColl *pPrevFormatColl = nullptr;
     if (pPrevFrame && pPrevFrame->IsTextFrame())
     {
@@ -1475,15 +1486,10 @@ static bool lcl_IdenticalStyles(const SwFrame* pPrevFrame, const SwFrame* pFrame
             pTextFrame->GetTextNodeForParaProps()->GetFormatColl());
     }
 
-    bool bIdenticalStyles = false;
-    if (pFrame && pFrame->IsTextFrame())
-    {
-        const SwTextFrame *pTextFrame = static_cast< const SwTextFrame * >( pFrame );
-        SwTextFormatColl *const pFormatColl = dynamic_cast<SwTextFormatColl*>(
-            pTextFrame->GetTextNodeForParaProps()->GetFormatColl());
-        bIdenticalStyles = pPrevFormatColl == pFormatColl;
-    }
-    return bIdenticalStyles;
+    const SwTextFrame* pTextFrame = static_cast<const SwTextFrame*>(pFrame);
+    SwTextFormatColl* const pFormatColl
+        = dynamic_cast<SwTextFormatColl*>(pTextFrame->GetTextNodeForParaProps()->GetFormatColl());
+    return pPrevFormatColl == pFormatColl;
 }
 
 static bool lcl_getContextualSpacing(const SwFrame* pPrevFrame)
@@ -1564,7 +1570,7 @@ SwTwips SwFlowFrame::CalcUpperSpace( const SwBorderAttrs *pAttrs,
             GetSpacingValuesOfFrame( (*pPrevFrame),
                                    nPrevLowerSpace, nPrevLineSpacing,
                                    bPrevLineSpacingProportional,
-                                   bIdenticalStyles);
+                                   bIdenticalStyles, pPrevFrame->FindSctFrame() == m_rThis.FindSctFrame());
             if( rIDSA.get(DocumentSettingId::PARA_SPACE_MAX) )
             {
                 // FIXME: apply bHalfContextualSpacing for better portability?
diff --git a/sw/source/core/layout/frmtool.cxx b/sw/source/core/layout/frmtool.cxx
index 6b1cccc647b5..9bf71fb8ddb9 100644
--- a/sw/source/core/layout/frmtool.cxx
+++ b/sw/source/core/layout/frmtool.cxx
@@ -3955,7 +3955,7 @@ void GetSpacingValuesOfFrame( const SwFrame& rFrame,
                             SwTwips& onLowerSpacing,
                             SwTwips& onLineSpacing,
                             bool& obIsLineSpacingProportional,
-                            bool bIdenticalStyles )
+                            bool bIdenticalStyles, bool bIdenticalSections )
 {
     if ( !rFrame.IsFlowFrame() )
     {
@@ -3973,6 +3973,8 @@ void GetSpacingValuesOfFrame( const SwFrame& rFrame,
 
         onLineSpacing = 0;
         obIsLineSpacingProportional = false;
+        SAL_WARN("sw","DEBUG::GetSpacingValuesOfFrame Lower["<<onLowerSpacing<<"] context["<<rULSpace.GetContext()<<"]["<<rULSpace.GetLower()<<"] prev IsTextFrame["<<rFrame.IsTextFrame()<<"] bIdenticalStyles["<<bIdenticalStyles<<"] bIdenticalSections["<<bIdenticalSections<<"]");
+        assert (!bIdenticalStyles || bIdenticalSections || !rULSpace.GetContext() || !rULSpace.GetLower());
         if ( rFrame.IsTextFrame() )
         {
             onLineSpacing = static_cast<const SwTextFrame&>(rFrame).GetLineSpace();