sw_redlinehide_4b: fix some problems with pathological table redlines

As seen in ooo95711-1.odt, the ODF import may create a redline that
starts on a SwTableNode - though i haven't been able to figure out how
such an odd creature might arise from UI actions.

Try to fix the obvious places so we don't crash easily; there might be
more trouble elsewhere though with code that assumes that a redline
starts on a SwTextNode.

Change-Id: I8431c1416ac4503ff0209a946398656f1c28366d

5 files changed, 174 insertions, 64 deletions

diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx
index b9e3094bc297..7487aec23e7e 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -25,6 +25,7 @@
 #include <fmtfld.hxx>
 #include <IDocumentUndoRedo.hxx>
 #include <IDocumentFieldsAccess.hxx>
+#include <IDocumentLayoutAccess.hxx>
 #include <IDocumentState.hxx>
 #include <redline.hxx>
 #include <UndoRedline.hxx>
@@ -143,28 +144,47 @@ void UpdateFramesForAddDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
     // the AppendFootnote/RemoveFootnote will do it by itself!
     rDoc.GetFootnoteIdxs().UpdateFootnote(rPam.Start()->nNode);
     SwTextNode *const pStartNode(rPam.Start()->nNode.GetNode().GetTextNode());
-    std::vector<SwTextFrame*> frames;
-    SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
-    for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
+    if (!pStartNode)
     {
-        if (pFrame->getRootFrame()->IsHideRedlines())
+        SwTableNode *const pTableNode(rPam.Start()->nNode.GetNode().GetTableNode());
+        assert(pTableNode); // known pathology
+        for (sal_uLong j = pTableNode->GetIndex(); j <= pTableNode->EndOfSectionIndex(); ++j)
         {
-            frames.push_back(pFrame);
+            pTableNode->GetNodes()[j]->SetRedlineMergeFlag(SwNode::Merge::Hidden);
+        }
+        for (SwRootFrame const*const pLayout : rDoc.GetAllLayouts())
+        {
+            if (pLayout->IsHideRedlines())
+            {
+                pTableNode->DelFrames(pLayout);
+            }
         }
     }
-    for (SwTextFrame * pFrame : frames)
+    else
     {
-        SwTextNode & rFirstNode(pFrame->GetMergedPara()
-            ? *pFrame->GetMergedPara()->pFirstNode
-            : *pStartNode);
-        assert(rFirstNode.GetIndex() <= pStartNode->GetIndex());
-        // clear old one first to avoid DelFrames confusing updates & asserts...
-        pFrame->SetMergedPara(nullptr);
-        pFrame->SetMergedPara(sw::CheckParaRedlineMerge(
-            *pFrame, rFirstNode, sw::FrameMode::Existing));
-        // the first node of the new redline is not necessarily the first
-        // node of the merged frame, there could be another redline nearby
-        sw::AddRemoveFlysAnchoredToFrameStartingAtNode(*pFrame, *pStartNode, nullptr);
+        std::vector<SwTextFrame*> frames;
+        SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
+        for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
+        {
+            if (pFrame->getRootFrame()->IsHideRedlines())
+            {
+                frames.push_back(pFrame);
+            }
+        }
+        for (SwTextFrame * pFrame : frames)
+        {
+            SwTextNode & rFirstNode(pFrame->GetMergedPara()
+                ? *pFrame->GetMergedPara()->pFirstNode
+                : *pStartNode);
+            assert(rFirstNode.GetIndex() <= pStartNode->GetIndex());
+            // clear old one first to avoid DelFrames confusing updates & asserts...
+            pFrame->SetMergedPara(nullptr);
+            pFrame->SetMergedPara(sw::CheckParaRedlineMerge(
+                *pFrame, rFirstNode, sw::FrameMode::Existing));
+            // the first node of the new redline is not necessarily the first
+            // node of the merged frame, there could be another redline nearby
+            sw::AddRemoveFlysAnchoredToFrameStartingAtNode(*pFrame, *pStartNode, nullptr);
+        }
     }
     // fields last - SwGetRefField::UpdateField requires up-to-date frames
     UpdateFieldsForRedline(rDoc.getIDocumentFieldsAccess()); // after footnotes
@@ -178,55 +198,73 @@ void UpdateFramesForRemoveDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
 {
     rDoc.GetFootnoteIdxs().UpdateFootnote(rPam.Start()->nNode);
     SwTextNode *const pStartNode(rPam.Start()->nNode.GetNode().GetTextNode());
-    std::vector<SwTextFrame*> frames;
-    std::set<SwRootFrame*> layouts;
-    SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
-    for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
+    if (!pStartNode)
     {
-        if (pFrame->getRootFrame()->IsHideRedlines())
+        SwTableNode const*const pTableNode(rPam.Start()->nNode.GetNode().GetTableNode());
+        assert(pTableNode); // known pathology
+        for (sal_uLong j = pTableNode->GetIndex(); j <= pTableNode->EndOfSectionIndex(); ++j)
         {
-            frames.push_back(pFrame);
-            layouts.insert(pFrame->getRootFrame());
+            pTableNode->GetNodes()[j]->SetRedlineMergeFlag(SwNode::Merge::None);
+        }
+        if (rDoc.getIDocumentLayoutAccess().GetCurrentLayout()->IsHideRedlines())
+        {
+            // note: this will also create frames for all currently hidden flys
+            // because it calls AppendAllObjs
+            ::MakeFrames(&rDoc, rPam.Start()->nNode, rPam.End()->nNode);
         }
     }
-    if (frames.empty())
-    {
-        return;
-    }
-    if (rPam.GetPoint()->nNode != rPam.GetMark()->nNode)
+    else
     {
-        // first, call CheckParaRedlineMerge on the first paragraph,
-        // to init flag on new merge range (if any) + 1st node post the merge
-        for (SwTextFrame * pFrame : frames)
+        std::vector<SwTextFrame*> frames;
+        std::set<SwRootFrame*> layouts;
+        SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
+        for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
         {
-            if (auto const pMergedPara = pFrame->GetMergedPara())
+            if (pFrame->getRootFrame()->IsHideRedlines())
             {
-                assert(pMergedPara->pFirstNode->GetIndex() <= pStartNode->GetIndex());
-                // clear old one first to avoid DelFrames confusing updates & asserts...
-                SwTextNode & rFirstNode(*pMergedPara->pFirstNode);
-                pFrame->SetMergedPara(nullptr);
-                pFrame->SetMergedPara(sw::CheckParaRedlineMerge(
-                    *pFrame, rFirstNode, sw::FrameMode::Existing));
+                frames.push_back(pFrame);
+                layouts.insert(pFrame->getRootFrame());
             }
         }
-        // now start node until end of merge + 1 has proper flags; MakeFrames
-        // should pick up from the next node in need of frames by checking flags
-        SwNodeIndex const start(*pStartNode, +1);
-        SwNodeIndex const end(rPam.End()->nNode, +1); // end is exclusive
-        // note: this will also create frames for all currently hidden flys
-        // both on first and non-first nodes because it calls AppendAllObjs
-        ::MakeFrames(&rDoc, start, end);
-        // re-use this to move flys that are now on the wrong frame, with end
-        // of redline as "second" node; the nodes between start and end should
-        // be complete with MakeFrames already
-        sw::MoveMergedFlysAndFootnotes(frames, *pStartNode,
-                *rPam.End()->nNode.GetNode().GetTextNode(), false);
-    }
-    else
-    {   // recreate flys in the one node the hard way...
-        for (auto const& pLayout : layouts)
+        if (frames.empty())
+        {
+            return;
+        }
+        if (rPam.GetPoint()->nNode != rPam.GetMark()->nNode)
         {
-            AppendAllObjs(rDoc.GetSpzFrameFormats(), pLayout);
+            // first, call CheckParaRedlineMerge on the first paragraph,
+            // to init flag on new merge range (if any) + 1st node post the merge
+            for (SwTextFrame * pFrame : frames)
+            {
+                if (auto const pMergedPara = pFrame->GetMergedPara())
+                {
+                    assert(pMergedPara->pFirstNode->GetIndex() <= pStartNode->GetIndex());
+                    // clear old one first to avoid DelFrames confusing updates & asserts...
+                    SwTextNode & rFirstNode(*pMergedPara->pFirstNode);
+                    pFrame->SetMergedPara(nullptr);
+                    pFrame->SetMergedPara(sw::CheckParaRedlineMerge(
+                        *pFrame, rFirstNode, sw::FrameMode::Existing));
+                }
+            }
+            // now start node until end of merge + 1 has proper flags; MakeFrames
+            // should pick up from the next node in need of frames by checking flags
+            SwNodeIndex const start(*pStartNode, +1);
+            SwNodeIndex const end(rPam.End()->nNode, +1); // end is exclusive
+            // note: this will also create frames for all currently hidden flys
+            // both on first and non-first nodes because it calls AppendAllObjs
+            ::MakeFrames(&rDoc, start, end);
+            // re-use this to move flys that are now on the wrong frame, with end
+            // of redline as "second" node; the nodes between start and end should
+            // be complete with MakeFrames already
+            sw::MoveMergedFlysAndFootnotes(frames, *pStartNode,
+                    *rPam.End()->nNode.GetNode().GetTextNode(), false);
+        }
+        else
+        {   // recreate flys in the one node the hard way...
+            for (auto const& pLayout : layouts)
+            {
+                AppendAllObjs(rDoc.GetSpzFrameFormats(), pLayout);
+            }
         }
     }
     // fields last - SwGetRefField::UpdateField requires up-to-date frames
diff --git a/sw/source/core/doc/docnum.cxx b/sw/source/core/doc/docnum.cxx
index 8e67ea7e17ec..4904d0f15c94 100644
--- a/sw/source/core/doc/docnum.cxx
+++ b/sw/source/core/doc/docnum.cxx
@@ -1440,11 +1440,23 @@ namespace sw {
 void
 GotoPrevLayoutTextFrame(SwNodeIndex & rIndex, SwRootFrame const*const pLayout)
 {
-   if (pLayout && pLayout->IsHideRedlines()
-       && rIndex.GetNode().IsTextNode()
-       && rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None)
+    if (pLayout && pLayout->IsHideRedlines())
     {
-        rIndex = *static_cast<SwTextFrame*>(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pFirstNode;
+        if (rIndex.GetNode().IsTextNode())
+        {
+            if (rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None)
+            {
+                rIndex = *static_cast<SwTextFrame*>(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pFirstNode;
+            }
+        }
+        else if (rIndex.GetNode().IsEndNode())
+        {
+            if (rIndex.GetNode().GetRedlineMergeFlag() == SwNode::Merge::Hidden)
+            {
+                rIndex = *rIndex.GetNode().StartOfSectionNode();
+                assert(rIndex.GetNode().IsTableNode());
+            }
+        }
     }
     --rIndex;
     if (pLayout && rIndex.GetNode().IsTextNode())
@@ -1456,11 +1468,22 @@ GotoPrevLayoutTextFrame(SwNodeIndex & rIndex, SwRootFrame const*const pLayout)
 void
 GotoNextLayoutTextFrame(SwNodeIndex & rIndex, SwRootFrame const*const pLayout)
 {
-   if (pLayout && pLayout->IsHideRedlines()
-       && rIndex.GetNode().IsTextNode()
-       && rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None)
+    if (pLayout && pLayout->IsHideRedlines())
     {
-        rIndex = *static_cast<SwTextFrame*>(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pLastNode;
+        if (rIndex.GetNode().IsTextNode())
+        {
+            if (rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None)
+            {
+                rIndex = *static_cast<SwTextFrame*>(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pLastNode;
+            }
+        }
+        else if (rIndex.GetNode().IsTableNode())
+        {
+            if (rIndex.GetNode().GetRedlineMergeFlag() == SwNode::Merge::Hidden)
+            {
+                rIndex = *rIndex.GetNode().EndOfSectionNode();
+            }
+        }
     }
     ++rIndex;
     if (pLayout && rIndex.GetNode().IsTextNode())
diff --git a/sw/source/core/layout/frmtool.cxx b/sw/source/core/layout/frmtool.cxx
index f1c11bb23103..dda68bc72473 100644
--- a/sw/source/core/layout/frmtool.cxx
+++ b/sw/source/core/layout/frmtool.cxx
@@ -59,6 +59,7 @@
 #include <laycache.hxx>
 #include <rootfrm.hxx>
 #include <paratr.hxx>
+#include <redline.hxx>
 #include <sortedobjs.hxx>
 #include <objectformatter.hxx>
 #include <calbck.hxx>
@@ -1446,6 +1447,31 @@ void InsertCnt_( SwLayoutFrame *pLay, SwDoc *pDoc,
         else if ( pNd->IsTableNode() )
         {   //Should we have encountered a table?
             SwTableNode *pTableNode = static_cast<SwTableNode*>(pNd);
+            if (pLayout->IsHideRedlines())
+            {
+                // in the problematic case, there can be only 1 redline...
+                SwPosition const tmp(*pNd);
+                SwRangeRedline const*const pRedline(
+                    pDoc->getIDocumentRedlineAccess().GetRedline(tmp, nullptr));
+                // pathology: redline that starts on a TableNode; cannot
+                // be created in UI but by import filters...
+                if (pRedline
+                    && pRedline->GetType() == nsRedlineType_t::REDLINE_DELETE
+                    && &pRedline->Start()->nNode.GetNode() == pNd)
+                {
+                    SAL_WARN("sw.pageframe", "skipping table frame creation on bizarre redline");
+                    while (true)
+                    {
+                        pTableNode->GetNodes()[nIndex]->SetRedlineMergeFlag(SwNode::Merge::Hidden);
+                        if (nIndex == pTableNode->EndOfSectionIndex())
+                        {
+                            break;
+                        }
+                        ++nIndex;
+                    }
+                    continue;
+                }
+            }
             if (pLayout->IsHideRedlines() && !pNd->IsCreateFrameWhenHidingRedlines())
             {
                 assert(pNd->GetRedlineMergeFlag() == SwNode::Merge::Hidden);
diff --git a/sw/source/core/layout/wsfrm.cxx b/sw/source/core/layout/wsfrm.cxx
index a123f055e36c..73387e4741ca 100644
--- a/sw/source/core/layout/wsfrm.cxx
+++ b/sw/source/core/layout/wsfrm.cxx
@@ -4349,6 +4349,24 @@ static void UnHideRedlines(SwRootFrame & rLayout,
                 rTextNode.NumRuleChgd();
             }
         }
+        else if (rNode.IsTableNode() && rLayout.IsHideRedlines())
+        {
+            SwPosition const tmp(rNode);
+            SwRangeRedline const*const pRedline(
+                rLayout.GetFormat()->GetDoc()->getIDocumentRedlineAccess().GetRedline(tmp, nullptr));
+            // pathology: redline that starts on a TableNode; cannot
+            // be created in UI but by import filters...
+            if (pRedline
+                && pRedline->GetType() == nsRedlineType_t::REDLINE_DELETE
+                && &pRedline->Start()->nNode.GetNode() == &rNode)
+            {
+                for (sal_uLong j = rNode.GetIndex(); j <= rNode.EndOfSectionIndex(); ++j)
+                {
+                    rNode.GetNodes()[j]->SetRedlineMergeFlag(SwNode::Merge::Hidden);
+                }
+                rNode.GetTableNode()->DelFrames(&rLayout);
+            }
+        }
         if (!rNode.IsCreateFrameWhenHidingRedlines())
         {
             if (rLayout.IsHideRedlines())
diff --git a/sw/source/core/text/redlnitr.cxx b/sw/source/core/text/redlnitr.cxx
index 0b92335a7e97..4f522ec3c171 100644
--- a/sw/source/core/text/redlnitr.cxx
+++ b/sw/source/core/text/redlnitr.cxx
@@ -82,6 +82,11 @@ CheckParaRedlineMerge(SwTextFrame & rFrame, SwTextNode & rTextNode,
             assert(IDocumentRedlineAccess::IsHideChanges(rIDRA.GetRedlineFlags()));
             continue;
         }
+        if (pStart->nNode.GetNode().IsTableNode())
+        {
+            assert(&pEnd->nNode.GetNode() == &rTextNode && pEnd->nContent.GetIndex() == 0);
+            continue; // known pathology, ignore it
+        }
         bHaveRedlines = true;
         assert(pNode != &rTextNode || &pStart->nNode.GetNode() == &rTextNode); // detect calls with wrong start node
         if (pStart->nContent != nLastEnd) // not 0 so we eliminate adjacent deletes