tdf#140982 sw ChangesInMargin: fix annotation ranges

Annotation ranges of tracked deletions collapsed in
ChangesInMargin mode (as before saving the document,
see commit d325cd0c69b7c0cc4f47105749a98995de81cc9d
"tdf#115815 sw: fix lost annotation ranges of redlines").

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

7 files changed, 131 insertions, 33 deletions

diff --git a/sw/inc/IDocumentMarkAccess.hxx b/sw/inc/IDocumentMarkAccess.hxx
index 801ce98ba503..49ce72ad28b4 100644
--- a/sw/inc/IDocumentMarkAccess.hxx
+++ b/sw/inc/IDocumentMarkAccess.hxx
@@ -343,7 +343,7 @@ class IDocumentMarkAccess
         virtual sw::mark::IMark* getAnnotationMarkFor(const SwPosition& rPosition) const = 0;
         // restore text ranges of annotations of tracked deletions
         // based on the helper bookmarks (which can survive I/O and hiding redlines)
-        virtual void restoreAnnotationMarks() = 0;
+        virtual void restoreAnnotationMarks(bool bDelete = true) = 0;
         /** Finds the first mark that is starting after.
 
             @returns
diff --git a/sw/qa/extras/uiwriter/data2/tdf115815.odt b/sw/qa/extras/uiwriter/data2/tdf115815.odt
new file mode 100644
index 000000000000..7c2aad0da638
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data2/tdf115815.odt
diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 0bc22e23aca4..97fea5d32797 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -2390,7 +2390,7 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf138666)
     CPPUNIT_ASSERT_EQUAL(OUString("Loremm"), getParagraph(1)->getString());
     CPPUNIT_ASSERT_EQUAL(OUString("dolsit"), getParagraph(2)->getString());
 
-    // switch on "Show changes in margin" mode
+    // switch off "Show changes in margin" mode
     dispatchCommand(mxComponent, ".uno:ShowChangesInMargin", {});
 
     // show deletions inline again
@@ -2398,6 +2398,78 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf138666)
     CPPUNIT_ASSERT_EQUAL(OUString("dolor sit"), getParagraph(2)->getString());
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf140982)
+{
+    SwDoc* pDoc = createDoc("tdf115815.odt");
+
+    SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+    CPPUNIT_ASSERT(pTextDoc);
+
+    //turn on red-lining and show changes
+    pDoc->getIDocumentRedlineAccess().SetRedlineFlags(RedlineFlags::On | RedlineFlags::ShowDelete
+                                                      | RedlineFlags::ShowInsert);
+    CPPUNIT_ASSERT_MESSAGE("redlining should be on",
+                           pDoc->getIDocumentRedlineAccess().IsRedlineOn());
+    CPPUNIT_ASSERT_MESSAGE(
+        "redlines should be visible",
+        IDocumentRedlineAccess::IsShowChanges(pDoc->getIDocumentRedlineAccess().GetRedlineFlags()));
+
+    // show deletions inline
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum dolor sit amet..."), getParagraph(1)->getString());
+
+    // switch on "Show changes in margin" mode
+    dispatchCommand(mxComponent, ".uno:ShowChangesInMargin", {});
+
+    // show deletions in margin
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem  amet..."), getParagraph(1)->getString());
+
+    // switch off "Show changes in margin" mode
+    dispatchCommand(mxComponent, ".uno:ShowChangesInMargin", {});
+
+    // show deletions inline again
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem ipsum dolor sit amet..."), getParagraph(1)->getString());
+
+    // Save it and load it back.
+    reload("writer8", "tdf115815.odt");
+
+    // Test comment range feature on tracked deletion.
+    uno::Reference<text::XTextDocument> xTextDocument(mxComponent, uno::UNO_QUERY);
+    uno::Reference<container::XEnumerationAccess> xParaEnumAccess(xTextDocument->getText(),
+                                                                  uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xParaEnum = xParaEnumAccess->createEnumeration();
+    uno::Reference<container::XEnumerationAccess> xRunEnumAccess(xParaEnum->nextElement(),
+                                                                 uno::UNO_QUERY);
+    uno::Reference<container::XEnumeration> xRunEnum = xRunEnumAccess->createEnumeration();
+    bool bAnnotationStart = false;
+    bool bBeforeAnnotation = true;
+    OUString sTextBeforeAnnotation;
+    while (xRunEnum->hasMoreElements())
+    {
+        uno::Reference<beans::XPropertySet> xPropertySet(xRunEnum->nextElement(), uno::UNO_QUERY);
+        OUString aType = getProperty<OUString>(xPropertySet, "TextPortionType");
+        // there is no AnnotationEnd with preceding AnnotationStart,
+        // i.e. annotation with lost range
+        CPPUNIT_ASSERT(aType != "AnnotationEnd" || !bAnnotationStart);
+
+        bAnnotationStart = (aType == "Annotation");
+
+        // collect paragraph text before the first annotation
+        if (bBeforeAnnotation)
+        {
+            if (bAnnotationStart)
+                bBeforeAnnotation = false;
+            else if (aType == "Text")
+            {
+                uno::Reference<text::XTextRange> xRun(xPropertySet, uno::UNO_QUERY);
+                sTextBeforeAnnotation += xRun->getString();
+            }
+        }
+    }
+
+    // This was "Lorem ipsum" (collapsed annotation range)
+    CPPUNIT_ASSERT_EQUAL(OUString("Lorem "), sTextBeforeAnnotation);
+}
+
 CPPUNIT_TEST_FIXTURE(SwUiWriterTest2, testTdf126206)
 {
     load(DATA_DIRECTORY, "tdf126206.docx");
diff --git a/sw/source/core/doc/docbm.cxx b/sw/source/core/doc/docbm.cxx
index 9061e7b853d1..82579d079445 100644
--- a/sw/source/core/doc/docbm.cxx
+++ b/sw/source/core/doc/docbm.cxx
@@ -1643,7 +1643,7 @@ namespace sw::mark
 
     // restore text ranges of annotations of tracked deletions
     // based on the helper bookmarks (which can survive I/O and hiding redlines)
-    void MarkManager::restoreAnnotationMarks()
+    void MarkManager::restoreAnnotationMarks(bool bDelete)
     {
         for (auto iter = getBookmarksBegin();
               iter != getBookmarksEnd(); )
@@ -1660,9 +1660,14 @@ namespace sw::mark
                     const SwPaM aPam((**iter).GetMarkStart(), (**pMark).GetMarkEnd());
                     repositionMark(*pMark, aPam);
                 }
-                deleteMark(&**iter);
-                // this invalidates iter, have to start over...
-                iter = getBookmarksBegin();
+                if (bDelete)
+                {
+                    deleteMark(&**iter);
+                    // this invalidates iter, have to start over...
+                    iter = getBookmarksBegin();
+                }
+                else
+                    ++iter;
             }
             else
                 ++iter;
diff --git a/sw/source/core/doc/docredln.cxx b/sw/source/core/doc/docredln.cxx
index 292600a171d6..d4d5cce88483 100644
--- a/sw/source/core/doc/docredln.cxx
+++ b/sw/source/core/doc/docredln.cxx
@@ -1335,6 +1335,42 @@ void SwRangeRedline::CalcStartEnd( sal_uLong nNdIdx, sal_Int32& rStart, sal_Int3
     }
 }
 
+static void lcl_storeAnnotationMarks(SwDoc& rDoc, const SwPosition* pStt, const SwPosition* pEnd)
+{
+    // tdf#115815 keep original start position of collapsed annotation ranges
+    // as temporary bookmarks (removed after file saving and file loading)
+    IDocumentMarkAccess& rDMA(*rDoc.getIDocumentMarkAccess());
+    for (auto iter = rDMA.getAnnotationMarksBegin();
+          iter != rDMA.getAnnotationMarksEnd(); )
+    {
+        SwPosition const& rStartPos((**iter).GetMarkStart());
+        if ( *pStt <= rStartPos && rStartPos < *pEnd )
+        {
+            OUString sBookmarkName((**iter).GetName() + "____");
+            IDocumentMarkAccess::const_iterator_t pOldMark = rDMA.findBookmark(sBookmarkName);
+            if ( pOldMark == rDMA.getBookmarksEnd() )
+            {
+                // at start of redlines use a 1-character length bookmark range
+                // instead of a 0-character length bookmark position to avoid its losing
+                sal_Int32 nLen = (*pStt == rStartPos) ? 1 : 0;
+                SwPaM aPam( rStartPos.nNode, rStartPos.nContent.GetIndex(),
+                                rStartPos.nNode, rStartPos.nContent.GetIndex() + nLen);
+                ::sw::mark::IMark* pMark = rDMA.makeMark(
+                    aPam,
+                    sBookmarkName,
+                    IDocumentMarkAccess::MarkType::BOOKMARK, sw::mark::InsertMode::New);
+                ::sw::mark::IBookmark* pBookmark = dynamic_cast< ::sw::mark::IBookmark* >(pMark);
+                if (pBookmark)
+                {
+                    pBookmark->SetKeyCode(vcl::KeyCode());
+                    pBookmark->SetShortName(OUString());
+                }
+            }
+        }
+        ++iter;
+    }
+}
+
 void SwRangeRedline::MoveToSection()
 {
     if( !m_pContentSect )
@@ -1378,7 +1414,11 @@ void SwRangeRedline::MoveToSection()
             SwNodeIndex aNdIdx( *pTextNd );
             SwPosition aPos( aNdIdx, SwIndex( pTextNd ));
             if( pCSttNd && pCEndNd )
+            {
+                // tdf#140982 keep annotation ranges in deletions in margin mode
+                lcl_storeAnnotationMarks( rDoc, pStt, pEnd );
                 rDoc.getIDocumentContentOperations().MoveAndJoin( aPam, aPos );
+            }
             else
             {
                 if( pCSttNd && !pCEndNd )
@@ -1445,32 +1485,7 @@ void SwRangeRedline::CopyToSection()
 
         // tdf#115815 keep original start position of collapsed annotation ranges
         // as temporary bookmarks (removed after file saving and file loading)
-        auto & rDMA(*rDoc.getIDocumentMarkAccess());
-        for (auto iter = rDMA.getAnnotationMarksBegin();
-              iter != rDMA.getAnnotationMarksEnd(); )
-        {
-            SwPosition const& rStartPos((**iter).GetMarkStart());
-            if ( *pStt <= rStartPos && rStartPos < *pEnd )
-            {
-                // at start of redlines use a 1-character length bookmark range
-                // instead of a 0-character length bookmark position to avoid its losing
-                sal_Int32 nLen = (*pStt == rStartPos) ? 1 : 0;
-                SwPaM aPam( rStartPos.nNode, rStartPos.nContent.GetIndex(),
-                                rStartPos.nNode, rStartPos.nContent.GetIndex() + nLen);
-                ::sw::mark::IMark* pMark = rDMA.makeMark(
-                    aPam,
-                    (**iter).GetName() + "____",
-                    IDocumentMarkAccess::MarkType::BOOKMARK, sw::mark::InsertMode::New);
-                ::sw::mark::IBookmark* pBookmark = dynamic_cast< ::sw::mark::IBookmark* >(pMark);
-                if (pBookmark)
-                {
-                    pBookmark->SetKeyCode(vcl::KeyCode());
-                    pBookmark->SetShortName(OUString());
-                }
-            }
-            ++iter;
-        }
-
+        lcl_storeAnnotationMarks( rDoc, pStt, pEnd );
         rDoc.getIDocumentContentOperations().CopyRange(*this, aPos, SwCopyFlags::CheckPosInFly);
 
         // Take over the style from the EndNode if needed
diff --git a/sw/source/core/inc/MarkManager.hxx b/sw/source/core/inc/MarkManager.hxx
index 4bc216ff3c7d..4e1547f00731 100644
--- a/sw/source/core/inc/MarkManager.hxx
+++ b/sw/source/core/inc/MarkManager.hxx
@@ -117,7 +117,7 @@ namespace sw::mark {
 
             typedef std::vector<sw::mark::MarkBase*> container_t;
 
-            virtual void restoreAnnotationMarks() override;
+            virtual void restoreAnnotationMarks(bool bDelete = true) override;
 
         private:
 
diff --git a/sw/source/uibase/shells/basesh.cxx b/sw/source/uibase/shells/basesh.cxx
index c91fab016851..41e1c96f2711 100644
--- a/sw/source/uibase/shells/basesh.cxx
+++ b/sw/source/uibase/shells/basesh.cxx
@@ -1065,6 +1065,12 @@ void SwBaseShell::Execute(SfxRequest &rReq)
             if ( !bAllInText )
                 aViewOption.SetShowChangesInMargin2( FN_SET_TRACKED_INSERTIONS_IN_MARGIN == nSlot );
             rSh.ApplyViewOptions( aViewOption );
+
+            // tdf#140982 restore annotation ranges stored in temporary bookmarks
+            // (only remove temporary bookmarks during file saving to avoid possible
+            // conflict with lazy deletion of the bookmarks of the moved tracked deletions)
+            if ( bAllInText || FN_SET_TRACKED_INSERTIONS_IN_MARGIN == nSlot )
+                rSh.GetDoc()->getIDocumentMarkAccess()->restoreAnnotationMarks(false);
         }
         break;
         case SID_CONTOUR_DLG: