tdf#75996: sw: correct bookmark positions when deleting text

The problem is that SwUndoDelete will move the fully selected nodes to
the UndoNodes but it leaves bookmarks with their SwIndex pointing to the
deleted nodes.  The SwNodeIndex are corrected by SwNodes::_MoveNodes()
so they point to a different node than the SwIndex.

This only happens if only one position of the bookmark is inside the
deletion range; if both are, the bookmark will be deleted by
SwUndoSaveCntnt::DelCntntIndex().

Also joining the 2 start/end nodes of the selection will accidentally
correct the bookmarks but only if it happens to delete the end node.
(and apparently there is also a DeleteRange method that doesn't join)

Change-Id: I91ec362bb833328f8d681fd9458cb915c4efb267
(cherry picked from commit 370febbf19a5f362394d1c9e69b12dcb218f6501)
Reviewed-on: https://gerrit.libreoffice.org/14240
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
Tested-by: Caolán McNamara <caolanm@redhat.com>

2 files changed, 47 insertions, 0 deletions

diff --git a/sw/qa/core/macros-test.cxx b/sw/qa/core/macros-test.cxx
index e2194e59f616..3101de7ed048 100644
--- a/sw/qa/core/macros-test.cxx
+++ b/sw/qa/core/macros-test.cxx
@@ -43,6 +43,8 @@
 
 #include <doc.hxx>
 #include <IDocumentLayoutAccess.hxx>
+#include <IDocumentUndoRedo.hxx>
+#include <IDocumentContentOperations.hxx>
 #include "docsh.hxx"
 
 typedef tools::SvRef<SwDocShell> SwDocShellRef;
@@ -66,6 +68,7 @@ public:
 #if !defined MACOSX && !defined WNT
     void testVba();
 #endif
+    void testBookmarkDeleteAndJoin();
     void testFdo55289();
     void testFdo68983();
     CPPUNIT_TEST_SUITE(SwMacrosTest);
@@ -75,6 +78,7 @@ public:
     //CPPUNIT_TEST(testStarBasic);
     CPPUNIT_TEST(testVba);
 #endif
+    CPPUNIT_TEST(testBookmarkDeleteAndJoin);
     CPPUNIT_TEST(testFdo55289);
     CPPUNIT_TEST(testFdo68983);
 
@@ -161,6 +165,45 @@ void SwMacrosTest::testVba()
 }
 #endif
 
+void SwMacrosTest::testBookmarkDeleteAndJoin()
+{
+    SwDoc *const pDoc = new SwDoc;
+    pDoc->GetIDocumentUndoRedo().DoUndo(true); // bug is in SwUndoDelete
+    SwNodeIndex aIdx(pDoc->GetNodes().GetEndOfContent(), -1);
+    SwPaM aPaM(aIdx);
+
+    IDocumentContentOperations & rIDCO(pDoc->getIDocumentContentOperations());
+    rIDCO.AppendTxtNode(*aPaM.GetPoint());
+    rIDCO.InsertString(aPaM, OUString("A"));
+    rIDCO.AppendTxtNode(*aPaM.GetPoint());
+    rIDCO.InsertString(aPaM, OUString("A"));
+    rIDCO.AppendTxtNode(*aPaM.GetPoint());
+    aPaM.Move(fnMoveBackward, fnGoNode);
+    aPaM.Move(fnMoveBackward, fnGoNode);
+    aPaM.Move(fnMoveBackward, fnGoCntnt);
+    aPaM.SetMark();
+    aPaM.Move(fnMoveForward, fnGoDoc);
+    IDocumentMarkAccess & rIDMA = *pDoc->getIDocumentMarkAccess();
+    sw::mark::IMark *pMark =
+            rIDMA.makeMark(aPaM, "test", IDocumentMarkAccess::BOOKMARK);
+    CPPUNIT_ASSERT(pMark);
+    // select so pMark start position is on a node that is fully deleted
+    aPaM.Move(fnMoveBackward, fnGoNode);
+    // must leave un-selected content in last node to get the bJoinPrev flag!
+    aPaM.Move(fnMoveBackward, fnGoCntnt);
+    aPaM.Exchange();
+    aPaM.Move(fnMoveBackward, fnGoDoc);
+    // delete
+    rIDCO.DeleteAndJoin(aPaM, false);
+
+    for (IDocumentMarkAccess::const_iterator_t i = rIDMA.getAllMarksBegin(); i != rIDMA.getAllMarksEnd(); ++i)
+    {
+        // problem was that the nContent was pointing at deleted node
+        CPPUNIT_ASSERT((*i)->GetMarkStart().nNode.GetNode().GetCntntNode() ==
+            static_cast<const SwCntntNode*>((*i)->GetMarkStart().nContent.GetIdxReg()));
+    }
+}
+
 void SwMacrosTest::testFdo55289()
 {
     SwDoc *const pDoc = new SwDoc;
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index b486ad82b7ac..253ef993dca6 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -153,7 +153,11 @@ SwUndoDelete::SwUndoDelete(
         _DelBookmarks(pStt->nNode, pEnd->nNode);
     }
     else
+    {
         DelCntntIndex( *rPam.GetMark(), *rPam.GetPoint() );
+        ::sw::UndoGuard const undoGuard(pDoc->GetIDocumentUndoRedo());
+        _DelBookmarks(pStt->nNode, pEnd->nNode, nullptr, &pStt->nContent, &pEnd->nContent);
+    }
 
     nSetPos = pHistory ? pHistory->Count() : 0;