diff options
author | Michael Stahl <Michael.Stahl@cib.de> | 2019-12-20 17:28:55 +0100 |
---|---|---|
committer | Michael Stahl <michael.stahl@cib.de> | 2019-12-23 10:04:57 +0100 |
commit | 68a5afaaabd0c75bba3439cfdff90fb75d1cdd3f (patch) | |
tree | 53894979f566e3d913a485be170a45c6e0e7ee10 | |
parent | 8723ac4e20eda87a82393f2f6c7d28ece8514238 (diff) |
sw: fix widow loop with as-char flys in text formatting
The document has a paragraph with 4 as-character anchored flys depicting
Zirbenholz; due to their size and an additional fly that is anchored at
the paragraph, there are 3 lines that do not fit onto a single page.
This situation causes a loop that proceeds like this:
text frame 80 is the follow of text frame 21.
when formatting 80:
the 1 line violates the widow rule (>=2) and PREP_WIDOWS is sent to
21, invalidating its FrameAreaSize
80 validates its FrameAreaSize
when formatting 21:
PREP_WIDOWS_ORPHANS sent to 21
CalcPreps() sees IsPrepWidows() and sets a huge height and calls
SetWidow(true)
SwTextFrame::WouldFit() sees IsWidow() true and resets it false
SwTextFrame::WouldFit() sees IsWidow() false and a huge but
insufficiently huge height
21 validates its FrameAreaSize
CalcPreps() sees IsPrepAdjust()
FindBreak() calls TruncLines() and because of as-char fly
invalidates FrameAreaSize of 80
The loop is most easily reproduced by printing via the API; it's
possible to get a loop when loading the document in the UI, but
typically the UI remains responsive even though the layout never
finishes.
As it happens, before commit ee299664940139f6f9543592ece3b3c0210b59f4
"SalInstance::DoYield: Don't drop SolarMutex when accessing user event
queue" the loop on printing via API was broken by releasing SolarMutex;
the result, however, was incorrect, with the last line of Zirbenholz
that should be on the second page missing in the PDF.
This loop is presumably a regression from commit
f2e3655255db4032738849cd4b77ce67a6e2c984 "Avoid
-fsanitize=signed-integer-overflow", which changed a magic number in
SwTextFrame::CalcPreps(), but didn't adapt the poorly documented
corresponding magic numbers in SwTextFrame::WouldFit(); in LO 5.1.6.2
the CPU is idle after loading the document.
Change-Id: Ib6563c21edb68945c14a61b51ba34f0ee3f2544a
Reviewed-on: https://gerrit.libreoffice.org/85623
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@cib.de>
-rw-r--r-- | sw/source/core/inc/txtfrm.hxx | 2 | ||||
-rw-r--r-- | sw/source/core/text/frmform.cxx | 2 | ||||
-rw-r--r-- | sw/source/core/text/txtfrm.cxx | 8 |
3 files changed, 7 insertions, 5 deletions
diff --git a/sw/source/core/inc/txtfrm.hxx b/sw/source/core/inc/txtfrm.hxx index 21297f872d35..11e975c6b004 100644 --- a/sw/source/core/inc/txtfrm.hxx +++ b/sw/source/core/inc/txtfrm.hxx @@ -1026,6 +1026,8 @@ public: }; +const SwTwips WIDOW_MAGIC = (SAL_MAX_INT32 - 1)/2; + } // namespace sw #endif diff --git a/sw/source/core/text/frmform.cxx b/sw/source/core/text/frmform.cxx index 0938a476f657..482823fc8dbf 100644 --- a/sw/source/core/text/frmform.cxx +++ b/sw/source/core/text/frmform.cxx @@ -838,7 +838,7 @@ bool SwTextFrame::CalcPreps() // the range of 'long', while the value (SAL_MAX_INT32 - 1)/2 (which matches the // old value on platforms where 'long' is 'sal_Int32') is empirically shown to // be large enough in practice even on platforms where 'long' is 'sal_Int64': - SwTwips nTmp = (SAL_MAX_INT32 - 1)/2 - (getFrameArea().Top()+10000); + SwTwips const nTmp = sw::WIDOW_MAGIC - (getFrameArea().Top()+10000); SwTwips nDiff = nTmp - getFrameArea().Height(); { diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx index 5100ae8eecca..cc32f19ac508 100644 --- a/sw/source/core/text/txtfrm.cxx +++ b/sw/source/core/text/txtfrm.cxx @@ -3251,9 +3251,9 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) // Because the Orphan flag only exists for a short moment, we also check // whether the Framesize is set to very huge by CalcPreps, in order to // force a MoveFwd - if( IsWidow() || ( aRectFnSet.IsVert() ? - ( 0 == getFrameArea().Left() ) : - ( LONG_MAX - 20000 < getFrameArea().Bottom() ) ) ) + if (IsWidow() || (aRectFnSet.IsVert() + ? (0 == getFrameArea().Left()) + : (sw::WIDOW_MAGIC - 20000 < getFrameArea().Bottom()))) { SetWidow(false); if ( GetFollow() ) @@ -3262,7 +3262,7 @@ bool SwTextFrame::WouldFit( SwTwips &rMaxHeight, bool &bSplit, bool bTst ) // whether there's a Follow with a real height at all. // Else (e.g. for newly created SctFrames) we ignore the IsWidow() and // still check if we can find enough room - if( ( ( ! aRectFnSet.IsVert() && LONG_MAX - 20000 >= getFrameArea().Bottom() ) || + if (((!aRectFnSet.IsVert() && getFrameArea().Bottom() <= sw::WIDOW_MAGIC - 20000) || ( aRectFnSet.IsVert() && 0 < getFrameArea().Left() ) ) && ( GetFollow()->IsVertical() ? !GetFollow()->getFrameArea().Width() : |