[Libreoffice-commits] core.git: Branch 'libreoffice-7-0' - sw/source

Michael Stahl (via logerrit) logerrit at kemper.freedesktop.org
Tue Jul 28 20:09:18 UTC 2020


 sw/source/core/doc/DocumentRedlineManager.cxx |  148 +++++++++++++++++---------
 1 file changed, 97 insertions(+), 51 deletions(-)

New commits:
commit 49d1ef7907f5a7a0934fbfa03318ddc6d3ad69b8
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Jul 24 19:41:29 2020 +0200
Commit:     Xisco Fauli <xiscofauli at libreoffice.org>
CommitDate: Tue Jul 28 22:08:53 2020 +0200

    tdf#133967 sw_redlinehide: fix redline deletion of table containing redlines
    
    A delete redline is created from node 371 to node 1625; there are
    existing redlines in the range, so AppendRedline() splits the new
    redline into pieces between and on the existing redlines.
    
    Even worse, because some of the existing redlines are in a table,
    InsertWithValidRanges() then splits the pieces that overlap the table
    into even smaller pieces, such that there are now the table structural
    nodes that were in the initial new delete redline's range but are now
    covered by none of the pieces.
    
    This means that multiple merged text frames are created, with non-merged
    frames between them, and thus the functions
    UpdateFramesForAddDeleteRedline() and
    UpdateFramesForRemoveDeleteRedline() [for Undo] have to deal with this
    unexpected complication.
    
    (regression from sw_redlinehide)
    
    Change-Id: Ibf71c1deb2bc3fa2af16035954d5ef71b6827be5
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99411
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>
    (cherry picked from commit 14e87a4b15d31a34e6053f6194688f3aa23af991)
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99409
    Reviewed-by: Xisco Fauli <xiscofauli at libreoffice.org>

diff --git a/sw/source/core/doc/DocumentRedlineManager.cxx b/sw/source/core/doc/DocumentRedlineManager.cxx
index 6996ad067aef..143e20e21005 100644
--- a/sw/source/core/doc/DocumentRedlineManager.cxx
+++ b/sw/source/core/doc/DocumentRedlineManager.cxx
@@ -159,31 +159,55 @@ void UpdateFramesForAddDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
     }
     else
     {
-        std::vector<SwTextFrame*> frames;
-        SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
-        for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
+        SwTextNode * pNode(pStartNode);
+        do
         {
-            if (pFrame->getRootFrame()->IsHideRedlines())
+            std::vector<SwTextFrame*> frames;
+            SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pNode);
+            for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
             {
-                frames.push_back(pFrame);
+                if (pFrame->getRootFrame()->IsHideRedlines())
+                {
+                    frames.push_back(pFrame);
+                }
             }
+            if (frames.empty())
+            {
+                auto const& layouts(rDoc.GetAllLayouts());
+                assert(std::none_of(layouts.begin(), layouts.end(),
+                    [](SwRootFrame const*const pLayout) { return pLayout->IsHideRedlines(); }));
+                (void) layouts;
+                break;
+            }
+            auto eMode(sw::FrameMode::Existing);
+            SwTextNode * pLast(pNode);
+            for (SwTextFrame * pFrame : frames)
+            {
+                SwTextNode & rFirstNode(pFrame->GetMergedPara()
+                    ? *pFrame->GetMergedPara()->pFirstNode
+                    : *pNode);
+                assert(pNode == pStartNode
+                        ? rFirstNode.GetIndex() <= pNode->GetIndex()
+                        : &rFirstNode == pNode);
+                // clear old one first to avoid DelFrames confusing updates & asserts...
+                pFrame->SetMergedPara(nullptr);
+                pFrame->SetMergedPara(sw::CheckParaRedlineMerge(
+                    *pFrame, rFirstNode, eMode));
+                eMode = sw::FrameMode::New; // Existing is not idempotent!
+                // 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, *pNode, nullptr);
+                // if redline is split across table and table cell is empty, there's no redline in the cell and so no merged para
+                if (pFrame->GetMergedPara())
+                {
+                    pLast = const_cast<SwTextNode*>(pFrame->GetMergedPara()->pLastNode);
+                }
+            }
+            SwNodeIndex tmp(*pLast);
+            // skip over hidden sections!
+            pNode = static_cast<SwTextNode*>(pLast->GetNodes().GoNextSection(&tmp, /*bSkipHidden=*/true, /*bSkipProtect=*/false));
         }
-        auto eMode(sw::FrameMode::Existing);
-        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, eMode));
-            eMode = sw::FrameMode::New; // Existing is not idempotent!
-            // 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);
-        }
+        while (pNode && pNode->GetIndex() <= rPam.End()->nNode.GetIndex());
     }
     // fields last - SwGetRefField::UpdateField requires up-to-date frames
     UpdateFieldsForRedline(rDoc.getIDocumentFieldsAccess()); // after footnotes
@@ -195,6 +219,7 @@ void UpdateFramesForAddDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
 
 void UpdateFramesForRemoveDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
 {
+    bool isAppendObjsCalled(false);
     rDoc.GetFootnoteIdxs().UpdateFootnote(rPam.Start()->nNode);
     SwTextNode *const pStartNode(rPam.Start()->nNode.GetNode().GetTextNode());
     if (!pStartNode)
@@ -210,35 +235,44 @@ void UpdateFramesForRemoveDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
             // note: this will also create frames for all currently hidden flys
             // because it calls AppendAllObjs
             ::MakeFrames(&rDoc, rPam.Start()->nNode, rPam.End()->nNode);
+            isAppendObjsCalled = true;
         }
     }
     else
     {
-        std::vector<SwTextFrame*> frames;
-        o3tl::sorted_vector<SwRootFrame*> layouts;
-        SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pStartNode);
-        for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
+        SwTextNode * pNode(pStartNode);
+        do
         {
-            if (pFrame->getRootFrame()->IsHideRedlines())
+            std::vector<SwTextFrame*> frames;
+            SwIterator<SwTextFrame, SwTextNode, sw::IteratorMode::UnwrapMulti> aIter(*pNode);
+            for (SwTextFrame * pFrame = aIter.First(); pFrame; pFrame = aIter.Next())
             {
-                frames.push_back(pFrame);
-                layouts.insert(pFrame->getRootFrame());
+                if (pFrame->getRootFrame()->IsHideRedlines())
+                {
+                    frames.push_back(pFrame);
+                }
             }
-        }
-        if (frames.empty())
-        {
-            return;
-        }
-        if (rPam.GetPoint()->nNode != rPam.GetMark()->nNode)
-        {
+            if (frames.empty())
+            {
+                auto const& layouts(rDoc.GetAllLayouts());
+                assert(std::none_of(layouts.begin(), layouts.end(),
+                    [](SwRootFrame const*const pLayout) { return pLayout->IsHideRedlines(); }));
+                (void) layouts;
+                break;
+            }
+
             // first, call CheckParaRedlineMerge on the first paragraph,
             // to init flag on new merge range (if any) + 1st node post the merge
             auto eMode(sw::FrameMode::Existing);
+            SwTextNode * pLast(pNode);
             for (SwTextFrame * pFrame : frames)
             {
                 if (auto const pMergedPara = pFrame->GetMergedPara())
                 {
-                    assert(pMergedPara->pFirstNode->GetIndex() <= pStartNode->GetIndex());
+                    pLast = const_cast<SwTextNode*>(pMergedPara->pLastNode);
+                    assert(pNode == pStartNode
+                        ? pMergedPara->pFirstNode->GetIndex() <= pNode->GetIndex()
+                        : pMergedPara->pFirstNode == pNode);
                     // clear old one first to avoid DelFrames confusing updates & asserts...
                     SwTextNode & rFirstNode(*pMergedPara->pFirstNode);
                     pFrame->SetMergedPara(nullptr);
@@ -247,24 +281,36 @@ void UpdateFramesForRemoveDeleteRedline(SwDoc & rDoc, SwPaM const& rPam)
                     eMode = sw::FrameMode::New; // Existing is not idempotent!
                 }
             }
-            // 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);
+            if (pLast != pNode)
+            {
+                // 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(*pNode, +1);
+                SwNodeIndex const end(*pLast, +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);
+                isAppendObjsCalled = true;
+                // 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, *pNode, *pLast, false);
+            }
+            SwNodeIndex tmp(*pLast);
+            // skip over hidden sections!
+            pNode = static_cast<SwTextNode*>(pLast->GetNodes().GoNextSection(&tmp, /*bSkipHidden=*/true, /*bSkipProtect=*/false));
         }
-        else
-        {   // recreate flys in the one node the hard way...
-            for (auto const& pLayout : layouts)
+        while (pNode && pNode->GetIndex() <= rPam.End()->nNode.GetIndex());
+    }
+
+    if (!isAppendObjsCalled)
+    {   // recreate flys in the one node the hard way...
+        for (auto const& pLayout : rDoc.GetAllLayouts())
+        {
+            if (pLayout->IsHideRedlines())
             {
                 AppendAllObjs(rDoc.GetSpzFrameFormats(), pLayout);
+                break;
             }
         }
     }


More information about the Libreoffice-commits mailing list