sw/inc/IDocumentRedlineAccess.hxx             |   12 ++++++++++--
 sw/source/core/doc/DocumentRedlineManager.cxx |   17 ++++++++++++-----
 sw/source/core/doc/doccomp.cxx                |    9 ++++++---
 sw/source/core/inc/DocumentRedlineManager.hxx |    2 +-
 sw/source/core/undo/undobj.cxx                |    6 +++---
 sw/source/core/unocore/unocrsrhelper.cxx      |    4 ++--
 6 files changed, 34 insertions(+), 16 deletions(-)

New commits:
commit 9cabd72ef14e19897f4d6f078758ac8b1aa6c02f
Author: Michael Stahl <mst...@redhat.com>
Date:   Fri Aug 25 22:55:22 2017 +0200

    tdf#109267 sw: fix confusing return value of AppendRedline()
    
    AppendRedline() has a boolen return value which is rather
    unclear and confusing: most callers don't even check it, but
    SaveMergeRedline::InsertRedline() expects "true" to mean that
    its redline hasn't been deleted, whereas makeRedline()
    expects "true" to mean that the redline was somehow "valid",
    even if it has been deleted and merged with an existing one.
    
    The "bMerged" variable in AppendRedline(), which is the source
    of the confusion, was introduced with commit
    81286906d0b76a3b6c4443378877828290c3e5f0 "docx import fixes
    for: redlines".
    
    Split these differing expectations into different return
    values of a new enum type.
    
    Change-Id: If81631bde49ee52a249f5ba1dd64ab8e92fffaf7

diff --git a/sw/inc/IDocumentRedlineAccess.hxx 
b/sw/inc/IDocumentRedlineAccess.hxx
index ac5d017eda4c..3f42f32d98ea 100644
--- a/sw/inc/IDocumentRedlineAccess.hxx
+++ b/sw/inc/IDocumentRedlineAccess.hxx
@@ -159,15 +159,23 @@ public:
 
     virtual bool IsInRedlines(const SwNode& rNode) const = 0;
 
+    enum class AppendResult { IGNORED, MERGED, APPENDED };
     /** Append a new redline
 
-        @param pPtr
+        @param pNewRedl redline to insert
 
         @param bCallDelete
+            if set, then for a new DELETE redline that is inserted so that it
+            overlaps an existing INSERT redline with the same author, the
+            overlapping range is deleted, i.e. the new DELETE removes
+            existing INSERT for that range
 
         @returns
+            APPENDED if pNewRedl is still alive and was appended
+            MERGED if pNewRedl was deleted but has been merged with existing 
one
+            IGNORED if pNewRedl was deleted and ignored/invalid
     */
-    virtual bool AppendRedline(/*[in]*/SwRangeRedline* pPtr, /*[in]*/bool 
bCallDelete) = 0;
+    virtual AppendResult AppendRedline(/*[in]*/SwRangeRedline* pNewRedl, 
/*[in]*/bool bCallDelete) = 0;
 
     virtual bool AppendTableRowRedline(/*[in]*/SwTableRowRedline* pPtr, 
/*[in]*/bool bCallDelete) = 0;
     virtual bool AppendTableCellRedline(/*[in]*/SwTableCellRedline* pPtr, 
/*[in]*/bool bCallDelete) = 0;
diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx 
b/sw/source/core/doc/DocumentRedlineManager.cxx
index 1a62a0b04c64..1d9a24be3153 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -1,4 +1,3 @@
-
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
 /*
  * This file is part of the LibreOffice project.
@@ -730,7 +729,8 @@ Behaviour of Delete-Redline:
                                           other Insert is overlapped by
                                           the Delete
 */
-bool DocumentRedlineManager::AppendRedline( SwRangeRedline* pNewRedl, bool 
bCallDelete )
+IDocumentRedlineAccess::AppendResult
+DocumentRedlineManager::AppendRedline(SwRangeRedline* pNewRedl, bool const 
bCallDelete)
 {
     bool bMerged = false;
     CHECK_REDLINE( *this )
@@ -792,7 +792,7 @@ bool DocumentRedlineManager::AppendRedline( SwRangeRedline* 
pNewRedl, bool bCall
             ( pNewRedl->GetContentIdx() == nullptr ) )
         {   // Do not insert empty redlines
             delete pNewRedl;
-            return false;
+            return AppendResult::IGNORED;
         }
         bool bCompress = false;
         SwRedlineTable::size_type n = 0;
@@ -1706,7 +1706,9 @@ bool DocumentRedlineManager::AppendRedline( 
SwRangeRedline* pNewRedl, bool bCall
     }
     CHECK_REDLINE( *this )
 
-    return ( nullptr != pNewRedl ) || bMerged;
+    return (nullptr != pNewRedl)
+        ? AppendResult::APPENDED
+        : ((bMerged) ? AppendResult::MERGED : AppendResult::IGNORED);
 }
 
 bool DocumentRedlineManager::AppendTableRowRedline( SwTableRowRedline* 
pNewRedl, bool )
diff --git a/sw/source/core/doc/doccomp.cxx b/sw/source/core/doc/doccomp.cxx
index 6dc6fb18db2d..1acbe7a9f306 100644
--- a/sw/source/core/doc/doccomp.cxx
+++ b/sw/source/core/doc/doccomp.cxx
@@ -1751,7 +1751,9 @@ void CompareData::SetRedlinesToDoc( bool bUseDocInfo )
         }
 
         do {
-            if( rDoc.getIDocumentRedlineAccess().AppendRedline( new 
SwRangeRedline( aRedlnData, *pTmp ), true) &&
+            if (IDocumentRedlineAccess::AppendResult::APPENDED ==
+                    rDoc.getIDocumentRedlineAccess().AppendRedline(
+                        new SwRangeRedline(aRedlnData, *pTmp), true) &&
                 rDoc.GetIDocumentUndoRedo().DoesUndo())
             {
                 SwUndo *const pUndo(new SwUndoCompDoc( *pTmp, true ));
@@ -2034,7 +2036,8 @@ sal_uInt16 SaveMergeRedline::InsertRedline(SwPaM* 
pLastDestRedline)
             ? new SwUndoCompDoc( *pDestRedl ) : nullptr;
 
         // now modify doc: append redline, undo (and count)
-        bool bRedlineAccepted = 
pDoc->getIDocumentRedlineAccess().AppendRedline( pDestRedl, true );
+        IDocumentRedlineAccess::AppendResult const result(
+            pDoc->getIDocumentRedlineAccess().AppendRedline(pDestRedl, true));
         if( pUndo )
         {
             pDoc->GetIDocumentUndoRedo().AppendUndo( pUndo );
@@ -2043,7 +2046,7 @@ sal_uInt16 SaveMergeRedline::InsertRedline(SwPaM* 
pLastDestRedline)
 
         // if AppendRedline has deleted our redline, we may not keep a
         // reference to it
-        if( ! bRedlineAccepted )
+        if (IDocumentRedlineAccess::AppendResult::APPENDED != result)
             pDestRedl = nullptr;
     }
     return nIns;
diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx 
b/sw/source/core/inc/DocumentRedlineManager.hxx
index eeea5f23ace9..9ba7e5bd7f1d 100644
--- a/sw/source/core/inc/DocumentRedlineManager.hxx
+++ b/sw/source/core/inc/DocumentRedlineManager.hxx
@@ -51,7 +51,7 @@ public:
 
     virtual bool IsInRedlines(const SwNode& rNode) const override;
 
-    virtual bool AppendRedline(/*[in]*/SwRangeRedline* pPtr, /*[in]*/bool 
bCallDelete) override;
+    virtual AppendResult AppendRedline(/*[in]*/SwRangeRedline* pPtr, 
/*[in]*/bool bCallDelete) override;
 
     virtual bool AppendTableRowRedline(/*[in]*/SwTableRowRedline* pPtr, 
/*[in]*/bool bCallDelete) override;
     virtual bool AppendTableCellRedline(/*[in]*/SwTableCellRedline* pPtr, 
/*[in]*/bool bCallDelete) override;
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index d974d093dcef..0f62c97737cb 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -1372,9 +1372,9 @@ void SwRedlineSaveData::RedlineToDoc( SwPaM const & rPam )
     if (rDoc.GetDocShell() && (!pRedl->GetComment().isEmpty()) )
         rDoc.GetDocShell()->Broadcast(SwRedlineHint());
 
-    bool const bSuccess = rDoc.getIDocumentRedlineAccess().AppendRedline( 
pRedl, true );
-    assert(bSuccess); // SwRedlineSaveData::RedlineToDoc: insert redline failed
-    (void) bSuccess; // unused in non-debug
+    auto const result(rDoc.getIDocumentRedlineAccess().AppendRedline(pRedl, 
true));
+    assert(result != IDocumentRedlineAccess::AppendResult::IGNORED); // 
SwRedlineSaveData::RedlineToDoc: insert redline failed
+    (void) result; // unused in non-debug
     rDoc.getIDocumentRedlineAccess().SetRedlineFlags_intern( eOld );
 }
 
diff --git a/sw/source/core/unocore/unocrsrhelper.cxx 
b/sw/source/core/unocore/unocrsrhelper.cxx
index 02df4ff1e85b..4b1925eb70cb 100644
--- a/sw/source/core/unocore/unocrsrhelper.cxx
+++ b/sw/source/core/unocore/unocrsrhelper.cxx
@@ -1267,9 +1267,9 @@ void makeRedline( SwPaM const & rPaM,
     pRedline->SetExtraData( pRedlineExtraData );
 
     pRedlineAccess->SetRedlineFlags_intern(RedlineFlags::On);
-    bool bRet = pRedlineAccess->AppendRedline( pRedline, false );
+    auto const result(pRedlineAccess->AppendRedline(pRedline, false));
     pRedlineAccess->SetRedlineFlags_intern( nPrevMode );
-    if( !bRet )
+    if (IDocumentRedlineAccess::AppendResult::IGNORED == result)
         throw lang::IllegalArgumentException();
 }
 
commit 355715d333bced59256afb005ac9f243d37aa23e
Author: Rosemary Sebastian <rosemary.s...@gmail.com>
Date:   Sat Jul 22 15:24:45 2017 +0530

    tdf#109267: Fix crash during undo of delete inside redline insert
    
    Change-Id: I070ce600c10f469b914cc1d6c036a55f33dc9529
    (cherry picked from commit bd37233020266a5892d6ec7022688e3dfb9cef75)
    Signed-off-by: Michael Stahl <mst...@redhat.com>

diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx 
b/sw/source/core/doc/DocumentRedlineManager.cxx
index 0baf6586c449..1a62a0b04c64 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -888,7 +888,12 @@ bool DocumentRedlineManager::AppendRedline( 
SwRangeRedline* pNewRedl, bool bCall
                                 ( pNewRedl->GetContentIdx() == nullptr ) )
                                 bDelete = true;
                         }
-                        else if( SwComparePosition::Inside == eCmpPos || 
SwComparePosition::Equal == eCmpPos)
+                        else if( SwComparePosition::Inside == eCmpPos )
+                        {
+                            bDelete = true;
+                            bMerged = true;
+                        }
+                        else if( SwComparePosition::Equal == eCmpPos )
                             bDelete = true;
 
                         if( bDelete )
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to