[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-6-1+backports' - sw/qa sw/source
Michael Stahl (via logerrit)
logerrit at kemper.freedesktop.org
Tue Jun 2 21:41:12 UTC 2020
sw/qa/extras/uiwriter/uiwriter2.cxx | 68 ++++++++++++++++++++++++++++++++++++
sw/source/core/undo/undel.cxx | 6 ++-
sw/source/core/undo/undobj.cxx | 18 ++++-----
3 files changed, 80 insertions(+), 12 deletions(-)
New commits:
commit 0361bb27b6900fb055a77bcc987305442134334c
Author: Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Fri Apr 12 18:10:49 2019 +0200
Commit: Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Tue Jun 2 23:40:41 2020 +0200
tdf#109376 sw: fix SwUndoDelete with end pos on SwTableNode crash
Commit 6ff263b837831d46d0c215963b70543a9ea5bd2a added a check in
SwUndoSaveContent::DelContentIndex() to avoid moving the anchor of a
FLY_AT_PARA if its new position would be a table node, because
SwFlyAtContentFrame::Modify() requires a SwTextNode to be the anchor.
However, that doesn't actually avoid moving the anchor - later,
SwNodes::RemoveNode() relocates the anchor to the next node regardless
of type!
It's probably better to just delete the fly in the situation when the
end position is a SwTableNode, which fixes the reported crash.
Unfortunately on Redo, the SwUndoDelete::UndoImpl() does not recreate
the nodes correctly, hence the fly then is inserted on the wrong node,
which later crashes again.
The problem is that due to the table node, a dummy SwTextNode is inserted,
which should be at the end of the range, but ends up at the start due to
an erroneous ++aPos.nNode; - the result is that the fly is inserted on
the dummy node and is immediately deleted again, triggering another
assert. If there is a dummy node, it also doesn't make sense to call
SplitNode().
Yet another problem is that in SwUndoDelete::UndoImpl(), the frames for
the moved text nodes are not created, because the first node is skipped
with the wrong assumption that it already has frames.
Reportedly this started to crash with commit
e07feb9457f2ffb373ae69b73dda290140e4005f, previously it was just wrong.
Reviewed-on: https://gerrit.libreoffice.org/70683
Tested-by: Jenkins
Reviewed-by: Michael Stahl <Michael.Stahl at cib.de>
(cherry picked from commit 80b73dcc06c671a49fbf238be58c1cd086c5c5f9)
Reviewed-on: https://gerrit.libreoffice.org/70764
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
(cherry picked from commit 7bba93a99ebb4250f884a68a50aa1912d96f4ba8)
Change-Id: I5094638e34c6ed52c836e57691d377b8cd1608f9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/95364
Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 357df4f9bd4a..bf6cd3bc39f8 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -14,6 +14,10 @@
#include <wrtsh.hxx>
#include <redline.hxx>
#include <ndtxt.hxx>
+#include <UndoManager.hxx>
+#include <itabenum.hxx>
+#include <fmtfsize.hxx>
+#include <fmtanchr.hxx>
namespace
{
@@ -28,13 +32,18 @@ public:
void testRedlineInHiddenSection();
void testTdf101534();
void testTdf131684();
+ void testTdf109376();
CPPUNIT_TEST_SUITE(SwUiWriterTest2);
CPPUNIT_TEST(testRedlineMoveInsertInDelete);
CPPUNIT_TEST(testRedlineInHiddenSection);
CPPUNIT_TEST(testTdf101534);
CPPUNIT_TEST(testTdf131684);
+ CPPUNIT_TEST(testTdf109376);
CPPUNIT_TEST_SUITE_END();
+
+private:
+ SwDoc* createDoc(const char* pName = nullptr);
};
static void lcl_dispatchCommand(const uno::Reference<lang::XComponent>& xComponent,
@@ -54,6 +63,18 @@ static void lcl_dispatchCommand(const uno::Reference<lang::XComponent>& xCompone
xDispatchHelper->executeDispatch(xFrame, rCommand, OUString(), 0, rPropertyValues);
}
+SwDoc* SwUiWriterTest2::createDoc(const char* pName)
+{
+ if (!pName)
+ loadURL("private:factory/swriter", nullptr);
+ else
+ load(DATA_DIRECTORY, pName);
+
+ SwXTextDocument* pTextDoc = dynamic_cast<SwXTextDocument*>(mxComponent.get());
+ CPPUNIT_ASSERT(pTextDoc);
+ return pTextDoc->GetDocShell()->GetDoc();
+}
+
void SwUiWriterTest2::testTdf131684()
{
load(DATA_DIRECTORY, "tdf131684.docx");
@@ -216,6 +237,53 @@ void SwUiWriterTest2::testRedlineInHiddenSection()
CPPUNIT_ASSERT(pNode->GetNodes()[pNode->GetIndex() + 4]->IsEndNode());
}
+void SwUiWriterTest2::testTdf109376()
+{
+ SwDoc* pDoc = createDoc();
+ SwWrtShell* pWrtShell = pDoc->GetDocShell()->GetWrtShell();
+ CPPUNIT_ASSERT(pWrtShell);
+ // need 2 paragraphs to get to the bMoveNds case
+ pWrtShell->Insert("foo");
+ pWrtShell->SplitNode();
+ pWrtShell->Insert("bar");
+ pWrtShell->SplitNode();
+ pWrtShell->StartOfSection(false);
+
+ // add AT_PARA fly at 1st to be deleted node
+ SwFormatAnchor anchor(RndStdIds::FLY_AT_PARA);
+ anchor.SetAnchor(pWrtShell->GetCursor()->GetPoint());
+ SfxItemSet flySet(pDoc->GetAttrPool(),
+ svl::Items<RES_FRM_SIZE, RES_FRM_SIZE, RES_ANCHOR, RES_ANCHOR>{});
+ flySet.Put(anchor);
+ SwFormatFrameSize size(ATT_MIN_SIZE, 1000, 1000);
+ flySet.Put(size); // set a size, else we get 1 char per line...
+ SwFrameFormat const* pFly = pWrtShell->NewFlyFrame(flySet, /*bAnchValid=*/true);
+ CPPUNIT_ASSERT(pFly != nullptr);
+
+ pWrtShell->SttEndDoc(false);
+ SwInsertTableOptions tableOpt(SwInsertTableFlags::DefaultBorder, 0);
+ const SwTable& rTable = pWrtShell->InsertTable(tableOpt, 1, 1);
+
+ pWrtShell->StartOfSection(false);
+ SwPaM pam(*pWrtShell->GetCursor()->GetPoint());
+ pam.SetMark();
+ pam.GetPoint()->nNode = *rTable.GetTableNode();
+ pam.GetPoint()->nContent.Assign(nullptr, 0);
+ pam.Exchange(); // same selection direction as in doc compare...
+
+ // this used to assert/crash with m_pAnchoredFlys mismatch because the
+ // fly was not deleted but its anchor was moved to the SwTableNode
+ pDoc->getIDocumentContentOperations().DeleteRange(pam);
+ CPPUNIT_ASSERT_EQUAL(size_t(0), pWrtShell->GetFlyCount(FLYCNTTYPE_FRM));
+ sw::UndoManager& rUndoManager = pDoc->GetUndoManager();
+ rUndoManager.Undo();
+ CPPUNIT_ASSERT_EQUAL(size_t(1), pWrtShell->GetFlyCount(FLYCNTTYPE_FRM));
+ rUndoManager.Redo();
+ CPPUNIT_ASSERT_EQUAL(size_t(0), pWrtShell->GetFlyCount(FLYCNTTYPE_FRM));
+ rUndoManager.Undo();
+ CPPUNIT_ASSERT_EQUAL(size_t(1), pWrtShell->GetFlyCount(FLYCNTTYPE_FRM));
+}
+
CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest2);
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index e75fad6c1970..ada018e207be 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -909,7 +909,7 @@ void SwUndoDelete::UndoImpl(::sw::UndoRedoContext & rContext)
pTextNd->RestoreMetadata(m_pMetadataUndoEnd);
}
}
- else if( m_aSttStr && bNodeMove )
+ else if (m_aSttStr && bNodeMove && pInsNd == nullptr)
{
SwTextNode * pNd = aPos.nNode.GetNode().GetTextNode();
if( pNd )
@@ -1098,8 +1098,10 @@ void SwUndoDelete::UndoImpl(::sw::UndoRedoContext & rContext)
{
// tdf#121031 if the start node is a text node, it already has a frame;
// if it's a table, it does not
+ // tdf#109376 exception: end on non-text-node -> start node was inserted
SwNodeIndex const start(rDoc.GetNodes(), nSttNode +
- ((m_bDelFullPara || !rDoc.GetNodes()[nSttNode]->IsTextNode()) ? 0 : 1));
+ ((m_bDelFullPara || !rDoc.GetNodes()[nSttNode]->IsTextNode() || pInsNd)
+ ? 0 : 1));
// don't include end node in the range: it may have been merged already
// by the start node, or it may be merged by one of the moved nodes,
// but if it isn't merged, its current frame(s) should be good...
diff --git a/sw/source/core/undo/undobj.cxx b/sw/source/core/undo/undobj.cxx
index 8e1cad8dbbd3..1fb1cf957696 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -985,17 +985,15 @@ void SwUndoSaveContent::DelContentIndex( const SwPosition& rMark,
// Moving the anchor?
if( !( DelContentType::CheckNoCntnt & nDelContentType ) &&
- ( rPoint.nNode.GetIndex() == pAPos->nNode.GetIndex() ) )
- {
+ (rPoint.nNode.GetIndex() == pAPos->nNode.GetIndex())
// Do not try to move the anchor to a table!
- if( rMark.nNode.GetNode().GetTextNode() )
- {
- pHistory->Add( *pFormat );
- SwFormatAnchor aAnch( *pAnchor );
- SwPosition aPos( rMark.nNode );
- aAnch.SetAnchor( &aPos );
- pFormat->SetFormatAttr( aAnch );
- }
+ && rMark.nNode.GetNode().IsTextNode())
+ {
+ pHistory->Add( *pFormat );
+ SwFormatAnchor aAnch( *pAnchor );
+ SwPosition aPos( rMark.nNode );
+ aAnch.SetAnchor( &aPos );
+ pFormat->SetFormatAttr( aAnch );
}
else
{
More information about the Libreoffice-commits
mailing list