tdf#138479 tdf#137769 sw ChangesInMargin: fix Undo in paragraphs

Fix Undo of embedded tracked deletions in paragraphs, for example
Undo of deletions Lorem->Loem->Lm resulted only Loem, moreover
a crash in the case of typing (not tracked) text before the
tracked deletions.

Now the last tracked deletion is chosen by the biggest
SwRangeRedline ID during Undo instead of the lost/modified
redline range in case of hidden redlines of the ChangesInMargin
mode.

Note: revert commit 4ad0459494303745b377c848c681a747f294fc64
(tdf#138135: sw ChangesInMargin: join characters at backspac) to
avoid crash on extra Undo (tdf#137769). Showing deleted
character sequence in margin needs grouping of hidden redlines,
as for managing tracked changes.

Change-Id: Ia9dab5cbbc08f39e05ff8e499efff37bc825c624
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/106582
Tested-by: László Németh <nemeth@numbertext.org>
Reviewed-by: László Németh <nemeth@numbertext.org>

5 files changed, 85 insertions, 66 deletions

diff --git a/sw/inc/docary.hxx b/sw/inc/docary.hxx
index 4a3e2031420e..9b4a6e80ea26 100644
--- a/sw/inc/docary.hxx
+++ b/sw/inc/docary.hxx
@@ -259,11 +259,8 @@ public:
      @param tableIndex position in SwRedlineTable to start searching at, will be updated with the index of the returned
                        redline (or the next redline after the given position if not found)
      @param next true: redline starts at position and ends after, false: redline starts before position and ends at or after
-     @param visible    true: redline must be visible false: redline must be not visible
     */
-    const SwRangeRedline* FindAtPosition( const SwPosition& startPosition,
-                                          size_type& tableIndex,
-                                          bool next = true, bool visible = true ) const;
+    const SwRangeRedline* FindAtPosition( const SwPosition& startPosition, size_type& tableIndex, bool next = true ) const;
 
     bool                        empty() const { return maVector.empty(); }
     size_type                   size() const { return maVector.size(); }
diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 48a6aa3fdc99..28019c51df0b 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -1946,11 +1946,9 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf138135)
     }
     CPPUNIT_ASSERT(getParagraph(1)->getString().startsWith("support"));
 
-    // single Undo undoes the deletion of the whole word
-    //
-    // This was only a 1-character Undo because of missing
-    // joining of the deleted characters
-    dispatchCommand(mxComponent, ".uno:Undo", {});
+    // TODO group redlines for managing tracked changes/showing in margin
+    for (int i = 0; i <= 10; ++i)
+        dispatchCommand(mxComponent, ".uno:Undo", {});
 
     CPPUNIT_ASSERT(getParagraph(1)->getString().startsWith("Encryption"));
 
@@ -2008,10 +2006,13 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf137771)
     assertXPath(pXmlDoc, "/metafile/push/push/push/line", 13);
 
     // This was the content of the next <text> (missing deletion on margin)
-    assertXPathContent(pXmlDoc, "/metafile/push/push/push/textarray[16]/text", " saved.");
+    assertXPathContent(pXmlDoc, "/metafile/push/push/push/textarray[16]/text", " s");
 
     // this would crash due to bad redline range
-    dispatchCommand(mxComponent, ".uno:Undo", {});
+    for (int i = 0; i < 6; ++i)
+    {
+        dispatchCommand(mxComponent, ".uno:Undo", {});
+    }
     CPPUNIT_ASSERT(getParagraph(1)->getString().endsWith("to be saved."));
 
     // switch off "Show changes in margin" mode
@@ -2019,6 +2020,58 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf137771)
     CPPUNIT_ASSERT(!pWrtShell->GetViewOptions()->IsShowChangesInMargin());
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf138479)
+{
+    SwDoc* const pDoc = createDoc();
+
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+    CPPUNIT_ASSERT(pTextDoc);
+
+    SwWrtShell* const pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+
+    pWrtShell->Insert("Lorem");
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem"), getParagraph(1)->getString());
+
+    //turn on red-lining and show changes
+    pDoc->getIDocumentRedlineAccess().SetRedlineFlags(RedlineFlags::On | RedlineFlags::ShowDelete
+                                                      | RedlineFlags::ShowInsert);
+    pDoc->getIDocumentRedlineAccess().SetRedlineFlags(RedlineFlags::On);
+    CPPUNIT_ASSERT_MESSAGE("redlining should be on",
+                           pDoc->getIDocumentRedlineAccess().IsRedlineOn());
+    CPPUNIT_ASSERT_MESSAGE("redlines shouldn't be visible",
+                           !IDocumentRedlineAccess::IsShowChanges(
+                               pDoc->getIDocumentRedlineAccess().GetRedlineFlags()));
+
+    // switch on "Show changes in margin" mode
+    dispatchCommand(mxComponent, ".uno:ShowChangesInMargin", {});
+
+    // delete "r" in "Lorem"
+    pWrtShell->Left(CRSR_SKIP_CHARS, /*bSelect=*/false, 3, /*bBasicCall=*/false);
+    dispatchCommand(mxComponent, ".uno:Delete", {});
+    CPPUNIT_ASSERT_EQUAL(OUString("Loem"), getParagraph(1)->getString());
+
+    // delete "oe" in "Loem"
+    pWrtShell->Left(CRSR_SKIP_CHARS, /*bSelect=*/false, 1, /*bBasicCall=*/false);
+    pWrtShell->Right(CRSR_SKIP_CHARS, /*bSelect=*/true, 2, /*bBasicCall=*/false);
+    dispatchCommand(mxComponent, ".uno:Delete", {});
+    CPPUNIT_ASSERT_EQUAL(OUString("Lm"), getParagraph(1)->getString());
+
+    // test embedded Undo in ChangesInMargin mode
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    CPPUNIT_ASSERT_EQUAL(OUString("Loem"), getParagraph(1)->getString());
+
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem"), getParagraph(1)->getString());
+
+    // this would crash due to bad redline range
+    for (int i = 0; i < 5; ++i)
+        dispatchCommand(mxComponent, ".uno:Undo", {});
+
+    // switch off "Show changes in margin" mode
+    dispatchCommand(mxComponent, ".uno:ShowChangesInMargin", {});
+    CPPUNIT_ASSERT(!pWrtShell->GetViewOptions()->IsShowChangesInMargin());
+}
+
 CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf126206)
 {
     load(DATA_DIRECTORY, "tdf126206.docx");
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 62e5b078f73e..40850e25fd40 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -3988,30 +3988,9 @@ bool DocumentContentOperationsManager::DeleteAndJoinWithRedlineImpl( SwPaM & rPa
         m_rDoc.getIDocumentRedlineAccess().SetRedlineFlags(
             RedlineFlags::On | RedlineFlags::ShowInsert | RedlineFlags::ShowDelete);
 
-        SwViewShell *pSh = m_rDoc.getIDocumentLayoutAccess().GetCurrentViewShell();
-        bool bShowChangesInMargin = pSh && pSh->GetViewOptions()->IsShowChangesInMargin();
-        const SwRedlineTable& rTable = m_rDoc.getIDocumentRedlineAccess().GetRedlineTable();
         for (SwRangeRedline * pRedline : redlines)
         {
             assert(pRedline->HasValidRange());
-
-            // deletions shown in margin
-            if (bShowChangesInMargin &&
-                    // within a paragraph TODO: fix also for paragraph join
-                    pRedline->GetPoint()->nNode == pRedline->GetMark()->nNode)
-            {
-                // show hidden previous deletion for joining
-                SwRedlineTable::size_type index = 0;
-                const SwRangeRedline* pPrevRedline = rTable.FindAtPosition(
-                                *pRedline->End(), index, /*bNext=*/false, /*bGetVisible=*/false );
-                if ( pPrevRedline && RedlineType::Delete == pPrevRedline->GetType() )
-                {
-                    SwRangeRedline* pPrevRed = rTable[ index ];
-                    pPrevRed->Show(1, index, /*bForced=*/true);
-                }
-                pRedline->Show(0, rTable.GetPos(pRedline), /*bForced=*/false);
-                pRedline->Show(1, rTable.GetPos(pRedline), /*bForced=*/false);
-            }
             undos.emplace_back(std::make_unique<SwUndoRedlineDelete>(
                         *pRedline, SwUndoId::DELETE));
         }
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 8a0f8d4b3597..354aa058d9ed 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -710,14 +710,13 @@ SwRedlineTable::size_type SwRedlineTable::FindPrevSeqNo( sal_uInt16 nSeqNo, size
 
 const SwRangeRedline* SwRedlineTable::FindAtPosition( const SwPosition& rSttPos,
                                         size_type& rPos,
-                                        bool bNext, bool bGetVisible ) const
+                                        bool bNext ) const
 {
     const SwRangeRedline* pFnd = nullptr;
     for( ; rPos < maVector.size() ; ++rPos )
     {
         const SwRangeRedline* pTmp = (*this)[ rPos ];
-        bool bIsVisible(pTmp->IsVisible());
-        if( (pTmp->HasMark()) && bIsVisible && bGetVisible )
+        if( pTmp->HasMark() && pTmp->IsVisible() )
         {
             const SwPosition* pRStt = pTmp->Start(),
                       * pREnd = pRStt == pTmp->GetPoint() ? pTmp->GetMark()
@@ -733,11 +732,6 @@ const SwRangeRedline* SwRedlineTable::FindAtPosition( const SwPosition& rSttPos,
             else
                 break;
         }
-        else if ( !bIsVisible && !bGetVisible && *pTmp->Start() == rSttPos )
-        {
-            pFnd = pTmp;
-            break;
-        }
     }
     return pFnd;
 }
diff --git a/sw/source/core/undo/unredln.cxx b/sw/source/core/undo/unredln.cxx
index c592c0bbb1f4..d870935c6613 100644
--- a/sw/source/core/undo/unredln.cxx
+++ b/sw/source/core/undo/unredln.cxx
@@ -92,37 +92,33 @@ void SwUndoRedline::UndoImpl(::sw::UndoRedoContext & rContext)
     SwPaM& rPam(AddUndoRedoPaM(rContext));
 
     // fix PaM for deletions shown in margin
-    SwRedlineTable::size_type nCurRedlinePos;
-    const SwRangeRedline * pRedline =
-            rDoc.getIDocumentRedlineAccess().GetRedline( *rPam.GetPoint(), &nCurRedlinePos );
-    if ( pRedline && !pRedline->IsVisible() )
+    bool bIsDeletion = dynamic_cast<SwUndoRedlineDelete*>(this);
+    if ( bIsDeletion )
     {
+        SwRedlineTable::size_type nCurRedlinePos = 0;
         const SwRedlineTable& rTable = rDoc.getIDocumentRedlineAccess().GetRedlineTable();
-        // count invisible (DELETE) redlines in the same position
-        SwRedlineTable::size_type nPos = nCurRedlinePos + 1;
-        while ( nPos < rTable.size() && !rTable[nPos]->IsVisible() &&
-            *pRedline->GetPoint() == *rTable[nPos]->GetPoint() )
+        SwRangeRedline * pRedline(rTable[nCurRedlinePos]);
+        // search last redline by its biggest id
+        // TODO handle multiple nodes
+        for( SwRedlineTable::size_type n = 1; n < rTable.size(); ++n )
         {
-            ++nPos;
+            SwRangeRedline *pRed(rTable[n]);
+            if ( pRedline->GetId() < pRed->GetId() )
+            {
+                nCurRedlinePos = n;
+                pRedline = pRed;
+            }
         }
-        SwRangeRedline * pHiddenRedline( rTable[nCurRedlinePos] );
-        pHiddenRedline->Show(0, rTable.GetPos(pHiddenRedline), /*bForced=*/true);
-        pHiddenRedline->Show(1, rTable.GetPos(pHiddenRedline), /*bForced=*/true);
-        rPam = *pHiddenRedline;
-
-        SwContentNode *pNd = rPam.GetContentNode();
-        const sal_Int32 nStart = rPam.Start()->nContent.GetIndex();
-        UndoRedlineImpl(rDoc, rPam);
-
-        // restore redline ranges to the start of the hidden deletion
-        // TODO fix the other cases
-        for (SwRedlineTable::size_type nIdx = nCurRedlinePos; nIdx + 1 < nPos; ++nIdx) {
-            SwRangeRedline * pHiddenRedline2( rTable[nIdx] );
-            pHiddenRedline2->GetPoint()->nContent.Assign(pNd, nStart);
+
+        if ( !pRedline->IsVisible() )
+        {
+            pRedline->Show(0, rTable.GetPos(pRedline), /*bForced=*/true);
+            pRedline->Show(1, rTable.GetPos(pRedline), /*bForced=*/true);
+            rPam = *pRedline;
         }
     }
-    else
-        UndoRedlineImpl(rDoc, rPam);
+
+    UndoRedlineImpl(rDoc, rPam);
 
     if( mpRedlSaveData )
     {
@@ -140,7 +136,7 @@ void SwUndoRedline::UndoImpl(::sw::UndoRedoContext & rContext)
     }
 
     // update frames after calling SetSaveData
-    if (dynamic_cast<SwUndoRedlineDelete*>(this))
+    if ( bIsDeletion )
     {
         sw::UpdateFramesForRemoveDeleteRedline(rDoc, rPam);
     }