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 7bba93a99ebb4250f884a68a50aa1912d96f4ba8
Author:     Michael Stahl <michael.st...@cib.de>
AuthorDate: Fri Apr 12 18:10:49 2019 +0200
Commit:     Thorsten Behrens <thorsten.behr...@cib.de>
CommitDate: Tue Apr 16 00:37:19 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.
    
    Reviewed-on: https://gerrit.libreoffice.org/70683
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.st...@cib.de>
    (cherry picked from commit 80b73dcc06c671a49fbf238be58c1cd086c5c5f9)
    
    Change-Id: I5094638e34c6ed52c836e57691d377b8cd1608f9
    Reviewed-on: https://gerrit.libreoffice.org/70764
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de>

diff --git a/sw/qa/extras/uiwriter/uiwriter2.cxx 
b/sw/qa/extras/uiwriter/uiwriter2.cxx
index 2344aa743248..ba6caf3f8222 100644
--- a/sw/qa/extras/uiwriter/uiwriter2.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter2.cxx
@@ -22,6 +22,8 @@
 #include <wrtsh.hxx>
 #include <redline.hxx>
 #include <UndoManager.hxx>
+#include <itabenum.hxx>
+#include <fmtfsize.hxx>
 #include <fmtornt.hxx>
 
 namespace
@@ -37,6 +39,7 @@ public:
     void testRedlineInHiddenSection();
     void testTdf101534();
     void testTdf54819();
+    void testTdf109376();
     void testTdf108687_tabstop();
     void testTdf119571();
     void testTdf119019();
@@ -52,6 +55,7 @@ public:
     CPPUNIT_TEST(testRedlineInHiddenSection);
     CPPUNIT_TEST(testTdf101534);
     CPPUNIT_TEST(testTdf54819);
+    CPPUNIT_TEST(testTdf109376);
     CPPUNIT_TEST(testTdf108687_tabstop);
     CPPUNIT_TEST(testTdf119571);
     CPPUNIT_TEST(testTdf119019);
@@ -251,6 +255,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::testTdf108687_tabstop()
 {
     SwDoc* pDoc = createDoc("tdf108687_tabstop.odt");
diff --git a/sw/source/core/undo/undel.cxx b/sw/source/core/undo/undel.cxx
index 8352db8aa2a7..16fe58d462a9 100644
--- a/sw/source/core/undo/undel.cxx
+++ b/sw/source/core/undo/undel.cxx
@@ -906,7 +906,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 )
@@ -1117,8 +1117,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 8f45e20e514d..63a8bd949bcc 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
                                 {
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits

Reply via email to