[Libreoffice-commits] core.git: Branch 'libreoffice-6-2-0' - sw/source

[Libreoffice Gerrit user]

 sw/source/core/text/txtfrm.cxx |   38 ++++++++++++++------------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

New commits:
commit 68c85f33520afb666f8eebd984e4ad2d7dce8fb1
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Mon Jan 21 12:30:20 2019 +0100
Commit:     Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
CommitDate: Wed Jan 23 16:22:33 2019 +0100

    sw_redlinehide: fix assert caused by buggy lcl_ModifyOfst
    
    converting ooo67907-1.doc to ODT asserts in fillSoftPageBreakList()
    while mapping an invalid (too large) GetOfst() of a follow frame.
    
    There are some problems with the lcl_ModifyOfst() implementation and
    usage:
    
    1. the implementation iterates over all follows; but the follows will
       get the same SwClientNotify() call as the frame itself, so in effect
       the follows' Ofsts will be modified multiple times
    
    2. the calls to lcl_ModifyOfst pass in a positive nLen whether the
       text is deleted or inserted; surely for deletions the Ofst should be
       subtracted from instead of added to
    
    3. the special-casing of RES_DEL_CHR to COMPLETE_STRING while
       RES_DEL_TXT uses the length does not appear to be justified
    
    The assert happens since b15f1ed2a1df45a57b587ac90efa90c1c18866fc
    but the lcl_ModifyOfst logic never made sense.
    
    Change-Id: Ie92628515bbee0fcc2123eb412057d8c0fd0b2f1
    Reviewed-on: https://gerrit.libreoffice.org/66679
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>
    (cherry picked from commit 0acde7514e666fc04805fd36503bd174162336ca)
    Reviewed-on: https://gerrit.libreoffice.org/66683
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    (cherry picked from commit d246374dc71ee557de5811e1971a82e0f6145509)
    Reviewed-on: https://gerrit.libreoffice.org/66722
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>
    Reviewed-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>
    Tested-by: Christian Lohmaier <lohmaier+LibreOffice at googlemail.com>

diff --git a/sw/source/core/text/txtfrm.cxx b/sw/source/core/text/txtfrm.cxx
index 4a44de62c9f8..69024f90a008 100644
--- a/sw/source/core/text/txtfrm.cxx
+++ b/sw/source/core/text/txtfrm.cxx
@@ -1788,17 +1788,15 @@ static void lcl_SetScriptInval(SwTextFrame& rFrame, TextFrameIndex const nPos)
         rFrame.GetPara()->GetScriptInfo().SetInvalidityA( nPos );
 }
 
-static void lcl_ModifyOfst(SwTextFrame* pFrame, TextFrameIndex const nPos, TextFrameIndex const nLen)
+// note: SwClientNotify will be called once for every frame => just fix own Ofst
+static void lcl_ModifyOfst(SwTextFrame & rFrame,
+        TextFrameIndex const nPos, TextFrameIndex const nLen,
+        TextFrameIndex (* op)(TextFrameIndex const&, TextFrameIndex const&))
 {
-    while( pFrame && pFrame->GetOfst() <= nPos )
-        pFrame = pFrame->GetFollow();
-    while( pFrame )
+    assert(nLen != TextFrameIndex(COMPLETE_STRING));
+    if (rFrame.IsFollow() && nPos < rFrame.GetOfst())
     {
-        if (nLen == TextFrameIndex(COMPLETE_STRING))
-            pFrame->ManipOfst(TextFrameIndex(pFrame->GetText().getLength()));
-        else
-            pFrame->ManipOfst( pFrame->GetOfst() + nLen );
-        pFrame = pFrame->GetFollow();
+        rFrame.ManipOfst( op(rFrame.GetOfst(), nLen) );
     }
 }
 
@@ -1859,11 +1857,8 @@ void UpdateMergedParaForMove(sw::MergedPara & rMerged,
             {
                 // InvalidateRange/lcl_SetScriptInval was called sufficiently for SwInsText
                 lcl_SetWrong(rTextFrame, rDestNode, nStart, it.first - it.second, false);
-                if (rTextFrame.HasFollow())
-                {
-                    TextFrameIndex const nIndex(sw::MapModelToView(rMerged, &rDestNode, nStart));
-                    lcl_ModifyOfst(&rTextFrame, nIndex, nDeleted); // FIXME why positive?
-                }
+                TextFrameIndex const nIndex(sw::MapModelToView(rMerged, &rDestNode, nStart));
+                lcl_ModifyOfst(rTextFrame, nIndex, nDeleted, &o3tl::operator-<sal_Int32, Tag_TextFrameIndex>);
             }
         }
     }
@@ -1994,8 +1989,7 @@ void SwTextFrame::SwClientNotify(SwModify const& rModify, SfxHint const& rHint)
             {
                 lcl_SetScriptInval( *this, nPos );
                 bSetFieldsDirty = bRecalcFootnoteFlag = true;
-                if (HasFollow())
-                    lcl_ModifyOfst( this, nPos, nLen );
+                lcl_ModifyOfst(*this, nPos, nLen, &o3tl::operator-<sal_Int32, Tag_TextFrameIndex>);
             }
         }
     }
@@ -2023,8 +2017,7 @@ void SwTextFrame::SwClientNotify(SwModify const& rModify, SfxHint const& rHint)
             lcl_SetWrong( *this, rNode, nNPos, nNLen, false );
             lcl_SetScriptInval( *this, nPos );
             bSetFieldsDirty = true;
-            if (HasFollow())
-                lcl_ModifyOfst( this, nPos, nLen );
+            lcl_ModifyOfst(*this, nPos, nLen, &o3tl::operator+<sal_Int32, Tag_TextFrameIndex>);
         }
     }
     else if (pMoveText)
@@ -2082,8 +2075,7 @@ void SwTextFrame::SwClientNotify(SwModify const& rModify, SfxHint const& rHint)
             lcl_SetWrong( *this, rNode, nNPos, nNLen, true );
             lcl_SetScriptInval( *this, nPos );
             bSetFieldsDirty = true;
-            if( HasFollow() )
-                lcl_ModifyOfst( this, nPos, nLen );
+            lcl_ModifyOfst(*this, nPos, nLen, &o3tl::operator+<sal_Int32, Tag_TextFrameIndex>);
         }
         break;
         case RES_DEL_CHR:
@@ -2104,8 +2096,7 @@ void SwTextFrame::SwClientNotify(SwModify const& rModify, SfxHint const& rHint)
                 InvalidateRange( SwCharRange(nPos, nLen), -1 );
                 lcl_SetScriptInval( *this, nPos );
                 bSetFieldsDirty = bRecalcFootnoteFlag = true;
-                if (HasFollow())
-                    lcl_ModifyOfst(this, nPos, TextFrameIndex(COMPLETE_STRING));
+                lcl_ModifyOfst(*this, nPos, nLen, &o3tl::operator-<sal_Int32, Tag_TextFrameIndex>);
             }
         }
         break;
@@ -2135,8 +2126,7 @@ void SwTextFrame::SwClientNotify(SwModify const& rModify, SfxHint const& rHint)
             {
                 lcl_SetScriptInval( *this, nPos );
                 bSetFieldsDirty = bRecalcFootnoteFlag = true;
-                if (HasFollow())
-                    lcl_ModifyOfst( this, nPos, nLen );
+                lcl_ModifyOfst(*this, nPos, nLen, &o3tl::operator-<sal_Int32, Tag_TextFrameIndex>);
             }
         }
         break;