[Libreoffice-commits] core.git: sw/inc sw/qa sw/source

[Michael Stahl (via logerrit)]

 sw/inc/ndtxt.hxx                                        |    2 -
 sw/qa/extras/uiwriter/data3/tdf100018-1.odt             |binary
 sw/qa/extras/uiwriter/uiwriter3.cxx                     |   15 +++++++++++++
 sw/source/core/doc/DocumentContentOperationsManager.cxx |    6 +++--
 sw/source/core/docnode/ndcopy.cxx                       |   18 ++++++++++++++--
 5 files changed, 36 insertions(+), 5 deletions(-)

New commits:
commit 6c1e6b188eeb5868cafe3f1605f4213d627a452c
Author:     Michael Stahl <Michael.Stahl at cib.de>
AuthorDate: Thu Dec 17 19:48:22 2020 +0100
Commit:     Michael Stahl <michael.stahl at cib.de>
CommitDate: Fri Dec 18 09:39:28 2020 +0100

    tdf#138897 sw: avoid creating SwUndoResetAttr in CopyImplImpl()
    
    The problem is that SwTextNode::CopyCollFormat() both creates the
    SwTextFormatColl with undo and applies it with undo.
    
    The first is desirable, the second causes a problem because it
    necessarily happens after SplitNode() and currently happens before
    copying the non-start/end nodes, so the node-index may not match in
    Undo, regardless if it runs before or after SwUndoCpyDoc.
    
    But SwUndoInserts restores the SwTextFormatColl on the node itself,
    so it can just be suppressed, which looks easier than refactoring this
    to call SplitNode() with Undo enabled.
    
    (regression from b4365b985178e1866c74afd757a104aad1d405a9)
    
    Change-Id: I4d15fb88cd5ae4cc53d9afb3397dec8fcf7635fa
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107921
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl at cib.de>

diff --git a/sw/inc/ndtxt.hxx b/sw/inc/ndtxt.hxx
index 5733a5f2c3d6..7692dc5b6470 100644
--- a/sw/inc/ndtxt.hxx
+++ b/sw/inc/ndtxt.hxx
@@ -413,7 +413,7 @@ public:
     /** Copy collection with all auto formats to dest-node.
         The latter might be in another document!
        (Method in ndcopy.cxx!!). */
-    void CopyCollFormat( SwTextNode& rDestNd );
+    void CopyCollFormat(SwTextNode& rDestNd, bool bUndoForChgFormatColl = true);
 
     // BEGIN OF BULLET/NUMBERING/OUTLINE STUFF:
 
diff --git a/sw/qa/extras/uiwriter/data3/tdf100018-1.odt b/sw/qa/extras/uiwriter/data3/tdf100018-1.odt
new file mode 100644
index 000000000000..5cd36efcee77
Binary files /dev/null and b/sw/qa/extras/uiwriter/data3/tdf100018-1.odt differ
diff --git a/sw/qa/extras/uiwriter/uiwriter3.cxx b/sw/qa/extras/uiwriter/uiwriter3.cxx
index 3f89bba3d171..7da4e05b03f9 100644
--- a/sw/qa/extras/uiwriter/uiwriter3.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter3.cxx
@@ -2118,4 +2118,19 @@ CPPUNIT_TEST_FIXTURE(SwUiWriterTest3, testTdf137964)
     CPPUNIT_ASSERT_EQUAL(sal_Int32(3090), xShape->getPosition().Y);
 }
 
+CPPUNIT_TEST_FIXTURE(SwUiWriterTest3, testTdf138897)
+{
+    load(DATA_DIRECTORY, "tdf100018-1.odt");
+
+    dispatchCommand(mxComponent, ".uno:SelectAll", {});
+    dispatchCommand(mxComponent, ".uno:Cut", {});
+    dispatchCommand(mxComponent, ".uno:Paste", {});
+    // this was crashing
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    dispatchCommand(mxComponent, ".uno:Redo", {});
+    dispatchCommand(mxComponent, ".uno:Undo", {});
+    dispatchCommand(mxComponent, ".uno:Redo", {});
+    Scheduler::ProcessEventsToIdle();
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index ea18f8717509..19701083ba38 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -4888,7 +4888,8 @@ bool DocumentContentOperationsManager::CopyImplImpl(SwPaM& rPam, SwPosition& rPo
                 {
                     if (bCopyCollFormat)
                     {
-                        pSttTextNd->CopyCollFormat( *pDestTextNd );
+                        // tdf#138897 no Undo for applying style, SwUndoInserts does it
+                        pSttTextNd->CopyCollFormat(*pDestTextNd, false);
                         POP_NUMRULE_STATE
                     }
 
@@ -4987,7 +4988,8 @@ bool DocumentContentOperationsManager::CopyImplImpl(SwPaM& rPam, SwPosition& rPo
             // Also copy all format templates
             if( bCopyCollFormat && ( bOneNode || bEmptyDestNd ))
             {
-                pEndTextNd->CopyCollFormat( *pDestTextNd );
+                // tdf#138897 no Undo for applying style, SwUndoInserts does it
+                pEndTextNd->CopyCollFormat(*pDestTextNd, false);
                 if ( bOneNode )
                 {
                     POP_NUMRULE_STATE
diff --git a/sw/source/core/docnode/ndcopy.cxx b/sw/source/core/docnode/ndcopy.cxx
index 6ccc17003f53..5b3c9edbd9db 100644
--- a/sw/source/core/docnode/ndcopy.cxx
+++ b/sw/source/core/docnode/ndcopy.cxx
@@ -18,6 +18,7 @@
  */
 #include <doc.hxx>
 #include <IDocumentFieldsAccess.hxx>
+#include <IDocumentUndoRedo.hxx>
 #include <node.hxx>
 #include <frmfmt.hxx>
 #include <swtable.hxx>
@@ -330,7 +331,7 @@ SwTableNode* SwTableNode::MakeCopy( SwDoc& rDoc, const SwNodeIndex& rIdx ) const
     return pTableNd;
 }
 
-void SwTextNode::CopyCollFormat( SwTextNode& rDestNd )
+void SwTextNode::CopyCollFormat(SwTextNode& rDestNd, bool const bUndoForChgFormatColl)
 {
     // Copy the formats into the other document:
     // Special case for PageBreak/PageDesc/ColBrk
@@ -350,10 +351,23 @@ void SwTextNode::CopyCollFormat( SwTextNode& rDestNd )
             aPgBrkSet.Put( *pAttr );
     }
 
-    rDestNd.ChgFormatColl( rDestDoc.CopyTextColl( *GetTextColl() ));
+    // this may create undo action SwUndoFormatCreate
+    auto const pCopy( rDestDoc.CopyTextColl( *GetTextColl() ) );
+    if (bUndoForChgFormatColl)
+    {
+        rDestNd.ChgFormatColl(pCopy);
+    }
+    else // tdf#138897
+    {
+        ::sw::UndoGuard const ug(rDestDoc.GetIDocumentUndoRedo());
+        rDestNd.ChgFormatColl(pCopy);
+    }
     pSet = GetpSwAttrSet();
     if( nullptr != pSet )
+    {
+        // note: this may create undo actions but not for setting the items
         pSet->CopyToModify( rDestNd );
+    }
 
     if( aPgBrkSet.Count() )
         rDestNd.SetAttr( aPgBrkSet );