summaryrefslogtreecommitdiff
path: root/sw
diff options
context:
space:
mode:
authorJustin Luth <jluth@mail.com>2023-06-15 16:32:37 -0400
committerJustin Luth <jluth@mail.com>2023-06-21 20:00:39 +0200
commit22c4df0282dce9d56207a440c523c2f3a16bc9f2 (patch)
treed88984089f4088a436d6ba07eb33d2a8b8e36d0a /sw
parentb8ccf3dacb19e157d9b2c35cc2b083cb39e76da0 (diff)
tdf#133507 sw layout: fix missing contextual spacing at section end
Most of the fix was included in 12f256cd1ae3e625c7b7f48987b7e843046924fa which was hunting for examples. Bad idea. Merge these two together. So this patch is just removing the document hunting aspect and enabling the fix itself. (Yeah, really bad idea.) When a paragraph style has "contextualspacing" with Spacing Above, 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. Miklos agreed that 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: I5db903e0f1ba41aab54f881256c50d3d1927d6d1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153187 Tested-by: Jenkins Reviewed-by: Justin Luth <jluth@mail.com>
Diffstat (limited to 'sw')
-rw-r--r--sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odtbin28346 -> 40288 bytes
-rw-r--r--sw/qa/extras/odfexport/odfexport2.cxx10
-rw-r--r--sw/source/core/inc/frmtool.hxx2
-rw-r--r--sw/source/core/layout/flowfrm.cxx9
-rw-r--r--sw/source/core/layout/frmtool.cxx4
5 files changed, 10 insertions, 15 deletions
diff --git a/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
index a2a9ebd47198..efc1c3f592c1 100644
--- a/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
+++ b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
Binary files differ
diff --git a/sw/qa/extras/odfexport/odfexport2.cxx b/sw/qa/extras/odfexport/odfexport2.cxx
index 0da6c7865c99..bf638819a527 100644
--- a/sw/qa/extras/odfexport/odfexport2.cxx
+++ b/sw/qa/extras/odfexport/odfexport2.cxx
@@ -242,11 +242,11 @@ 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());
-// }
+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 a497ea09e590..ffc218b5504e 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 bIdenticalSections = true );
+ bool bIdenticalStyles );
/** 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 a2b9da301678..fbcd6e14d159 100644
--- a/sw/source/core/layout/flowfrm.cxx
+++ b/sw/source/core/layout/flowfrm.cxx
@@ -1464,7 +1464,7 @@ const SwFrame* SwFlowFrame::GetPrevFrameForUpperSpaceCalc_( const SwFrame* _pPro
return pPrevFrame;
}
-// This should be renamed to something like lcl_ApplyULSpacing
+// This should be renamed to something like lcl_UseULSpacing
/// Compare styles attached to these text frames.
static bool lcl_IdenticalStyles(const SwFrame* pPrevFrame, const SwFrame* pFrame)
{
@@ -1473,10 +1473,7 @@ static bool lcl_IdenticalStyles(const SwFrame* pPrevFrame, const SwFrame* pFrame
// 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;
- }
+ return false;
SwTextFormatColl *pPrevFormatColl = nullptr;
if (pPrevFrame && pPrevFrame->IsTextFrame())
@@ -1570,7 +1567,7 @@ SwTwips SwFlowFrame::CalcUpperSpace( const SwBorderAttrs *pAttrs,
GetSpacingValuesOfFrame( (*pPrevFrame),
nPrevLowerSpace, nPrevLineSpacing,
bPrevLineSpacingProportional,
- bIdenticalStyles, pPrevFrame->FindSctFrame() == m_rThis.FindSctFrame());
+ bIdenticalStyles);
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 9bf71fb8ddb9..6b1cccc647b5 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 bIdenticalSections )
+ bool bIdenticalStyles )
{
if ( !rFrame.IsFlowFrame() )
{
@@ -3973,8 +3973,6 @@ 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();