sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt |binary
sw/qa/extras/odfexport/odfexport2.cxx | 10
+++++-----
sw/source/core/inc/frmtool.hxx | 2 +-
sw/source/core/layout/flowfrm.cxx | 9
+++------
sw/source/core/layout/frmtool.cxx | 4 +---
5 files changed, 10 insertions(+), 15 deletions(-)
New commits:
commit 22c4df0282dce9d56207a440c523c2f3a16bc9f2
Author: Justin Luth <jl...@mail.com>
AuthorDate: Thu Jun 15 16:32:37 2023 -0400
Commit: Justin Luth <jl...@mail.com>
CommitDate: Wed Jun 21 20:00:39 2023 +0200
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 <jl...@mail.com>
diff --git a/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt
index a2a9ebd47198..efc1c3f592c1 100644
Binary files
a/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt and
b/sw/qa/extras/odfexport/data/tdf133507_contextualSpacingSection.odt 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();
