sw/source/core/doc/DocumentContentOperationsManager.cxx | 22 +++++++++++++---
sw/source/core/doc/doccorr.cxx | 7 +++--
sw/source/filter/xml/XMLRedlineImportHelper.cxx | 5 +--
xmloff/source/text/XMLChangedRegionImportContext.cxx | 11 +++++---
xmloff/source/text/txtparai.cxx | 17 +++++++++---
5 files changed, 47 insertions(+), 15 deletions(-)
New commits:
commit 94246148b215b10062e1d425fec8f1647ff3ffea
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Jan 5 12:32:09 2023 +0100
Commit: Caolán McNamara <caol...@redhat.com>
CommitDate: Sun Jan 8 20:31:00 2023 +0000
tdf#152710 sw: invalidate SwUnoCursors properly in DeleteRangeImpl()
This crashes with:
list.cxx:44: corrupt document structure, bailing out of infinite loop
ndtxt.cxx:5437: void SwTextNode::TriggerNodeUpdate(const
sw::LegacyModifyHint&): Assertion `dynamic_cast<SwTextFormatColl
const*>(static_cast<const SwFormatChg*>(pOldValue)->pChangedFormat)' failed.
Because the redline from 7 to 9 is deleted, but then some cursor ends up
on node 10 which is invalid as it is an end node.
[ 6] 0x60666a0 StartNode ,
[ 7] 0x61195e0 StartNode ,
[ 8] 0x61197a8 TextNode "tainment",
[ 9] 0x6119670 EndNode ,
[ 10] 0x6066730 EndNode ,
The first problem is that DeleteRangeImpl() uses the point node as the
target position for PaMCorrAbs(), but in this case the point node will
be deleted.
PaMCorrAbs() has a check to invalidate SwUnoCursors that would be moved
out of their parent sections, but due to the first problem it can't
check it, and the second problem is that lcl_FindUnoCursorSection()
doesn't work on redline sections, as those have node type
SwNormalStartNode.
After fixing the invalidation, subsequent access to the SwXTextCursor
throws exceptions and importing the file fails.
(regression from commit 477e489e71b4a96ff10d9f2d2b802d91dec3e319)
Thanks to Dave Gilbert for identifying the problematic DeleteRange()
call.
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145077
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit 8e05bdd26f21fc304978ff3b454cf355841ec75f)
tdf#152710 sw: call and fix DeleteSection() instead
Turns out there's a function to delete a complete nodes array section -
and it has the same problem? Why does it move indexes only from
startnode + 1? Let's try to fix it to be more consistent.
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145078
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit 475e59d29b7a6cc7f058af8ff863b3bb1a2a84a5)
tdf#152710 xmloff: ignore exception in XMLChangedRegionImportContext
The xOldCursor must be restored in all cases.
Also XMLParaContext triggers an exception which ends up aborting the
import.
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145094
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.st...@allotropia.de>
(cherry picked from commit e5b5d9c8d33b1dd87e5a50856ad02f21df59dc5b)
Change-Id: Iedacc10e29c1646c4ccc85e53a479b0351f5cfcc
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/145104
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caol...@redhat.com>
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx
b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 962cea631e5f..4b4404f4550e 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -2094,6 +2094,18 @@ DocumentContentOperationsManager::CopyRange( SwPaM&
rPam, SwPosition& rPos,
return bRet;
}
+static auto GetCorrPosition(SwPaM const& rPam) -> SwPosition
+{
+ // tdf#152710 target position must be on node that survives deletion
+ // so that PaMCorrAbs can invalidate SwUnoCursors properly
+ return rPam.GetPoint()->nNode.GetNode().IsContentNode()
+ ? *rPam.GetPoint()
+ : rPam.GetMark()->nNode.GetNode().IsContentNode()
+ ? *rPam.GetMark()
+ // this would be the result in SwNodes::RemoveNode()
+ : SwPosition(SwNodeIndex(rPam.End()->nNode.GetNode(),
SwNodeOffset(+1)));
+}
+
/// Delete a full Section of the NodeArray.
/// The passed Node is located somewhere in the designated Section.
void DocumentContentOperationsManager::DeleteSection( SwNode *pNode )
@@ -2111,8 +2123,9 @@ void DocumentContentOperationsManager::DeleteSection(
SwNode *pNode )
{
// move all Cursor/StackCursor/UnoCursor out of the to-be-deleted area
- SwNodeIndex aMvStt( aSttIdx, 1 );
- SwDoc::CorrAbs( aMvStt, aEndIdx, SwPosition( aSttIdx ), true );
+ SwPaM const range(aSttIdx, aEndIdx);
+ SwPosition const pos(GetCorrPosition(range));
+ ::PaMCorrAbs(range, pos);
}
m_rDoc.GetNodes().DelNodes( aSttIdx, aEndIdx.GetIndex() -
aSttIdx.GetIndex() + 1 );
@@ -4272,7 +4285,10 @@ bool
DocumentContentOperationsManager::DeleteRangeImpl(SwPaM & rPam, SwDeleteFla
// Move all cursors out of the deleted range, but first copy the
// passed PaM, because it could be a cursor that would be moved!
SwPaM aDelPam( *rPam.GetMark(), *rPam.GetPoint() );
- ::PaMCorrAbs( aDelPam, *aDelPam.GetPoint() );
+ {
+ SwPosition const pos(GetCorrPosition(aDelPam));
+ ::PaMCorrAbs(aDelPam, pos);
+ }
bool const bSuccess( DeleteRangeImplImpl(aDelPam, flags) );
if (bSuccess)
diff --git a/sw/source/core/doc/doccorr.cxx b/sw/source/core/doc/doccorr.cxx
index e614732d9a52..3c3d8d7b56a2 100644
--- a/sw/source/core/doc/doccorr.cxx
+++ b/sw/source/core/doc/doccorr.cxx
@@ -34,11 +34,14 @@ namespace
/// returns NULL if no restrictions apply
const SwStartNode* lcl_FindUnoCursorSection( const SwNode& rNode )
{
- const SwStartNode* pStartNode = rNode.StartOfSectionNode();
+ const SwStartNode* pStartNode = rNode.IsStartNode() ?
rNode.GetStartNode() : rNode.StartOfSectionNode();
while( ( pStartNode != nullptr ) &&
( pStartNode->StartOfSectionNode() != pStartNode ) &&
- ( pStartNode->GetStartNodeType() == SwNormalStartNode ) )
+ // section node is only start node allowing overlapped delete
+ pStartNode->IsSectionNode() )
+ {
pStartNode = pStartNode->StartOfSectionNode();
+ }
return pStartNode;
}
diff --git a/sw/source/filter/xml/XMLRedlineImportHelper.cxx
b/sw/source/filter/xml/XMLRedlineImportHelper.cxx
index 20f508913074..3fc853652e7f 100644
--- a/sw/source/filter/xml/XMLRedlineImportHelper.cxx
+++ b/sw/source/filter/xml/XMLRedlineImportHelper.cxx
@@ -721,9 +721,8 @@ void
XMLRedlineImportHelper::InsertIntoDocument(RedlineInfo* pRedlineInfo)
SAL_WARN("sw.xml", "Recursive change tracking, removing");
// reuse aPaM to remove it from nodes that will be deleted
*aPaM.GetPoint() = SwPosition(pRedlineInfo->pContentIndex->GetNode());
- aPaM.SetMark();
- *aPaM.GetMark() =
SwPosition(*pRedlineInfo->pContentIndex->GetNode().EndOfSectionNode());
- pDoc->getIDocumentContentOperations().DeleteRange(aPaM);
+ aPaM.DeleteMark();
+
pDoc->getIDocumentContentOperations().DeleteSection(&aPaM.GetPoint()->nNode.GetNode());
}
else
{
diff --git a/xmloff/source/text/XMLChangedRegionImportContext.cxx
b/xmloff/source/text/XMLChangedRegionImportContext.cxx
index 4d43a8de3027..fe00c4a058b9 100644
--- a/xmloff/source/text/XMLChangedRegionImportContext.cxx
+++ b/xmloff/source/text/XMLChangedRegionImportContext.cxx
@@ -122,9 +122,14 @@ void
XMLChangedRegionImportContext::endFastElement(sal_Int32 )
{
// delete last paragraph
// (one extra paragraph was inserted in the beginning)
- rtl::Reference<XMLTextImportHelper> rHelper =
- GetImport().GetTextImport();
- rHelper->DeleteParagraph();
+ try
+ {
+ GetImport().GetTextImport()->DeleteParagraph();
+ }
+ catch (uno::Exception const&)
+ { // cursor may be disposed - must reset to old cursor!
+ SAL_INFO("xmloff.text", "XMLChangedRegionImportContext: delete
paragraph failed");
+ }
GetImport().GetTextImport()->SetCursor(xOldCursor);
xOldCursor = nullptr;
diff --git a/xmloff/source/text/txtparai.cxx b/xmloff/source/text/txtparai.cxx
index e0b8c8099ee5..99ffbc2810fa 100644
--- a/xmloff/source/text/txtparai.cxx
+++ b/xmloff/source/text/txtparai.cxx
@@ -1688,10 +1688,19 @@ void XMLParaContext::endFastElement(sal_Int32 )
{
rtl::Reference < XMLTextImportHelper > xTxtImport(
GetImport().GetTextImport());
- Reference < XTextRange > xCrsrRange( xTxtImport->GetCursorAsRange() );
- if( !xCrsrRange.is() )
- return; // Robust (defective file)
- Reference < XTextRange > xEnd(xCrsrRange->getStart());
+ Reference<XTextRange> xEnd;
+ try
+ {
+ Reference<XTextRange> const xCrsrRange(xTxtImport->GetCursorAsRange());
+ if (!xCrsrRange.is())
+ return; // Robust (defective file)
+ xEnd = xCrsrRange->getStart();
+ }
+ catch (uno::Exception const&)
+ {
+ SAL_INFO("xmloff.text", "XMLParaContext: cursor disposed?");
+ return;
+ }
// if we have an id set for this paragraph, get a cursor for this
// paragraph and register it with the given identifier
