sw/qa/extras/uiwriter/uiwriter2.cxx |   51 ++++++++++++++++++++++++++++++++++++
 sw/source/core/undo/undel.cxx       |    6 ++--
 sw/source/core/undo/undobj.cxx      |   18 +++++-------
 3 files changed, 63 insertions(+), 12 deletions(-)

New commits:
commit 80b73dcc06c671a49fbf238be58c1cd086c5c5f9
Author:     Michael Stahl <michael.st...@cib.de>
AuthorDate: Fri Apr 12 18:10:49 2019 +0200
Commit:     Michael Stahl <michael.st...@cib.de>
CommitDate: Mon Apr 15 12:27:41 2019 +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.
    
    Change-Id: I5094638e34c6ed52c836e57691d377b8cd1608f9
    Reviewed-on: https://gerrit.libreoffice.org/70683
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@cib.de>

diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx 
b/sw/qa/extras/uiwriter/uiwriter2.cxx
index ddebd9a1a662..f58819aee41c 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -30,6 +30,8 @@
 #include <sortedobjs.hxx>
 #include <anchoredobject.hxx>
 #include <swtypes.hxx>
+#include <itabenum.hxx>
+#include <fmtfsize.hxx>
 #include <fmtornt.hxx>
 #include <xmloff/odffields.hxx>
 #include <txtfrm.hxx>
@@ -49,6 +51,7 @@ public:
     void testTdf47471_paraStyleBackground();
     void testTdf101534();
     void testTdf54819();
+    void testTdf109376();
     void testTdf64242_optimizeTable();
     void testTdf108687_tabstop();
     void testTdf119571();
@@ -79,6 +82,7 @@ public:
     CPPUNIT_TEST(testTdf47471_paraStyleBackground);
     CPPUNIT_TEST(testTdf101534);
     CPPUNIT_TEST(testTdf54819);
+    CPPUNIT_TEST(testTdf109376);
     CPPUNIT_TEST(testTdf64242_optimizeTable);
     CPPUNIT_TEST(testTdf108687_tabstop);
     CPPUNIT_TEST(testTdf119571);
@@ -343,6 +347,53 @@ void SwUiWriterTest2::testTdf54819()
                          getProperty<OUString>(getParagraph(1), 
"ParaStyleName"));
 }
 
+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));
+}
+
 void SwUiWriterTest2::testTdf64242_optimizeTable()
 {
     SwDoc* pDoc = createDoc("tdf64242_optimizeTable.odt");
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index 1ec3a95be66e..daa062536d88 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -903,7 +903,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 )
@@ -1114,8 +1114,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 eb909378556b..0d17568e3b09 100644
--- a/sw/source/core/undo/undobj.cxx
+++ b/sw/source/core/undo/undobj.cxx
@@ -987,17 +987,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
                                 {
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to