SW: fixed redline SwPosition update problem

When nodes are removed, SwPosition nNode is updated,
but its nContent is not.
If nNode change from a non-ContentNodo to a ContentNode, then it is
a problem, as nContent 's m_pContentNode remains nullptr, so the
Position seems to be inconsistent.

Now when redline remove nodes, it check what redlines may effected it,
and update them after the node deletion happened.

Probably this bug should be handled deeper, as this problem probably
effect every SwPosition.. not sure if it can be a problem elsewhere.

A special case when it happens, if there is a Table between 2 text.
And there are 2 redlines..
1: any redline positioned 'text start'-'table start'
2: delete redline: 'table begin'-'table end'
now if we accept the 2. redline .. that remove the table
the 1. redline position will change to: 'text start'-'next text start'
but its end's nContent.m_pContentNode remain nullptr
so lcl_CheckPosition(...) will assert

Change-Id: I2981fd0218a375994d3f55cb5d3463b17ca35849
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158456
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/158614
Tested-by: Jenkins

4 files changed, 78 insertions, 3 deletions

diff --git a/sw/inc/IDocumentRedlineAccess.hxx b/sw/inc/IDocumentRedlineAccess.hxx
index c2b71aa1005a..6e28f1340deb 100644
--- a/sw/inc/IDocumentRedlineAccess.hxx
+++ b/sw/inc/IDocumentRedlineAccess.hxx
@@ -168,6 +168,11 @@ public:
         /*[in]*/const SwNode& rNode,
         /*[in]*/RedlineType nType) const = 0;
 
+    virtual SwRedlineTable::size_type GetRedlineEndPos(
+        /*[in]*/ SwRedlineTable::size_type nStartPos,
+        /*[in]*/ const SwNode& rNode,
+        /*[in]*/ RedlineType nType) const = 0;
+
     virtual bool HasRedline(
         /*[in]*/const SwPaM& rPam,
         /*[in]*/RedlineType nType,
@@ -226,6 +231,10 @@ public:
     virtual void SetRedlinePassword(
         /*[in]*/const css::uno::Sequence <sal_Int8>& rNewPassword) = 0;
 
+    virtual void UpdateRedlineContentNode(/*[in]*/ SwRedlineTable::size_type nStartPos,
+                                          /*[in]*/ SwRedlineTable::size_type nEndPos) const = 0;
+
+
 protected:
      virtual ~IDocumentRedlineAccess() {};
 };
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 60f9d1b96890..3f4c99a8f487 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -2142,8 +2142,17 @@ void DocumentContentOperationsManager::DeleteDummyChar(
 
 void DocumentContentOperationsManager::DeleteRange( SwPaM & rPam )
 {
+    // Seek all redlines that are in that PaM to be deleted..
+    SwRedlineTable::size_type nRedlStart = m_rDoc.getIDocumentRedlineAccess().GetRedlinePos(
+        rPam.Start()->GetNode(), RedlineType::Any);
+    SwRedlineTable::size_type nRedlEnd = m_rDoc.getIDocumentRedlineAccess().GetRedlineEndPos(
+        nRedlStart, rPam.End()->GetNode(), RedlineType::Any);
+
     lcl_DoWithBreaks(*this, rPam, SwDeleteFlags::Default, &DocumentContentOperationsManager::DeleteRangeImpl);
 
+    // update all redlines was in the Pam that is
+    m_rDoc.getIDocumentRedlineAccess().UpdateRedlineContentNode(nRedlStart, nRedlEnd);
+
     if (!m_rDoc.getIDocumentRedlineAccess().IsIgnoreRedline()
         && !m_rDoc.getIDocumentRedlineAccess().GetRedlineTable().empty())
     {
diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx
index b09ad5bedcaa..468449ecfbe9 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -54,9 +54,8 @@ using namespace com::sun::star;
         // 2. check that position is valid and doesn't point after text
         void lcl_CheckPosition( const SwPosition* pPos )
         {
-            // Commented out because of a random problem, that happened even before my patch
-            //assert(dynamic_cast<SwContentIndexReg*>(&pPos->GetNode())
-            //        == pPos->GetContentNode());
+            assert(dynamic_cast<SwContentIndexReg*>(&pPos->GetNode())
+                    == pPos->GetContentNode());
 
             SwTextNode* pTextNode = pPos->GetNode().GetTextNode();
             if( pTextNode == nullptr )
@@ -2801,6 +2800,54 @@ SwRedlineTable::size_type DocumentRedlineManager::GetRedlinePos( const SwNode& r
     // #TODO - add 'SwExtraRedlineTable' also ?
 }
 
+SwRedlineTable::size_type
+DocumentRedlineManager::GetRedlineEndPos(SwRedlineTable::size_type nStartPos, const SwNode& rNd,
+                                         RedlineType nType) const
+{
+    //if the start is already invalid
+    if (nStartPos >= maRedlineTable.size())
+        return nStartPos;
+
+    const SwNodeOffset nNdIdx = rNd.GetIndex();
+    SwRedlineTable::size_type nEndPos = nStartPos;
+    SwRedlineTable::size_type nEndPosTry = nEndPos + 1;
+
+    while (nEndPosTry < maRedlineTable.size()
+           && maRedlineTable[nEndPosTry]->Start()->GetNodeIndex() <= nNdIdx)
+    {
+        if (RedlineType::Any == nType || nType == maRedlineTable[nEndPosTry]->GetType())
+        {
+            nEndPos = nEndPosTry;
+        }
+        nEndPosTry++;
+    }
+    return nEndPos;
+}
+
+void DocumentRedlineManager::UpdateRedlineContentNode(SwRedlineTable::size_type nStartPos,
+                                                      SwRedlineTable::size_type nEndPos) const
+{
+    for (SwRedlineTable::size_type n = nStartPos; n <= nEndPos; ++n)
+    {
+        //just in case we got wrong input
+        if (n >= maRedlineTable.size())
+            return;
+
+        SwPosition* pStart = maRedlineTable[n]->Start();
+        SwPosition* pEnd = maRedlineTable[n]->End();
+        SwContentNode* pCont = pStart->GetNode().GetContentNode();
+        if (pCont)
+        {
+            pStart->nContent.Assign(pCont, pStart->nContent.GetIndex());
+        }
+        pCont = pEnd->GetNode().GetContentNode();
+        if (pCont)
+        {
+            pEnd->nContent.Assign(pCont, pEnd->nContent.GetIndex());
+        }
+    }
+}
+
 bool DocumentRedlineManager::HasRedline( const SwPaM& rPam, RedlineType nType, bool bStartOrEndInRange ) const
 {
     SwPosition currentStart(*rPam.Start());
diff --git a/sw/source/core/inc/DocumentRedlineManager.hxx b/sw/source/core/inc/DocumentRedlineManager.hxx
index fee4842d1115..106c0445897a 100644
--- a/sw/source/core/inc/DocumentRedlineManager.hxx
+++ b/sw/source/core/inc/DocumentRedlineManager.hxx
@@ -74,6 +74,11 @@ public:
         /*[in]*/const SwNode& rNode,
         /*[in]*/RedlineType nType) const override;
 
+    virtual SwRedlineTable::size_type GetRedlineEndPos(
+        /*[in]*/ SwRedlineTable::size_type nStartPos,
+        /*[in]*/ const SwNode& rNode,
+        /*[in]*/ RedlineType nType) const override;
+
     virtual bool HasRedline(
         /*[in]*/const SwPaM& rPam,
         /*[in]*/RedlineType nType,
@@ -124,6 +129,11 @@ public:
     virtual void SetRedlinePassword(
         /*[in]*/const css::uno::Sequence <sal_Int8>& rNewPassword) override;
 
+    // After nodes are removed, m_pContentNode's may not updated
+    virtual void UpdateRedlineContentNode(
+        /*[in]*/ SwRedlineTable::size_type nStartPos,
+        /*[in]*/ SwRedlineTable::size_type nEndPos) const override;
+
     //Non Interface methods;
 
     /** Set comment-text for Redline. It then comes in via AppendRedLine.