From 63dba5203e8bc7fc390943cb8208ae904f18e3bb Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Thu, 6 Dec 2018 12:32:20 +0100 Subject: 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 --- sw/source/core/doc/DocumentRedlineManager.cxx | 150 ++++++++++++++++---------- sw/source/core/doc/docnum.cxx | 39 +++++-- sw/source/core/layout/frmtool.cxx | 26 +++++ sw/source/core/layout/wsfrm.cxx | 18 ++++ sw/source/core/text/redlnitr.cxx | 5 + 5 files changed, 174 insertions(+), 64 deletions(-) (limited to 'sw') 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 #include #include +#include #include #include #include @@ -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 frames; - SwIterator 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 frames; + SwIterator 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 frames; - std::set layouts; - SwIterator 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 frames; + std::set layouts; + SwIterator 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(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pFirstNode; + if (rIndex.GetNode().IsTextNode()) + { + if (rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None) + { + rIndex = *static_cast(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(rIndex.GetNode().GetTextNode()->getLayoutFrame(pLayout))->GetMergedPara()->pLastNode; + if (rIndex.GetNode().IsTextNode()) + { + if (rIndex.GetNode().GetRedlineMergeFlag() != SwNode::Merge::None) + { + rIndex = *static_cast(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 #include #include +#include #include #include #include @@ -1446,6 +1447,31 @@ void InsertCnt_( SwLayoutFrame *pLay, SwDoc *pDoc, else if ( pNd->IsTableNode() ) { //Should we have encountered a table? SwTableNode *pTableNode = static_cast(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 -- cgit