[Libreoffice-commits] core.git: Branch 'libreoffice-5-0' - sw/qa sw/source

Michael Stahl mstahl at redhat.com
Thu Oct 8 00:38:11 PDT 2015


 sw/qa/extras/uiwriter/uiwriter.cxx                      |   57 ++++++++++++++++
 sw/source/core/doc/DocumentContentOperationsManager.cxx |   43 ++++++++++--
 sw/source/core/inc/DocumentContentOperationsManager.hxx |    2 
 3 files changed, 94 insertions(+), 8 deletions(-)

New commits:
commit 06a66af0b754b62f18783c77410c7929f8134941
Author: Michael Stahl <mstahl at redhat.com>
Date:   Fri Oct 2 15:13:18 2015 +0200

    sw: fix copying of bookmarks in CopyRange
    
    If the copied range starts with a not fully selected paragraph, the
    bookmarks that are copied will be created on the wrong paragraphs,
    on the node after the correct one.
    
    This also happens when hinding the redlines, and causes asserts from
    attempting to create CrossRefBookmarks on table nodes on WW8 export of
    fdo66302-1.odt and fdo66312-1.odt.
    
    Change-Id: Id576be3e38a89527d967f02b39d9aabbf6368354
    (cherry picked from commit c95ba3ef2613e9d5abd2f19ab2432c7bc1a40fe7)
    Reviewed-on: https://gerrit.libreoffice.org/19104
    Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
    Tested-by: Miklos Vajna <vmiklos at collabora.co.uk>

diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index 006cb46..364439b 100644
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -67,6 +67,7 @@ public:
     //EDITING: undo search&replace corrupt text when searching backward
     void testReplaceBackward();
     void testRedlineFrame();
+    void testBookmarkCopy();
     void testFdo69893();
     void testFdo70807();
     void testImportRTF();
@@ -108,6 +109,7 @@ public:
     CPPUNIT_TEST(testReplaceForward);
     CPPUNIT_TEST(testReplaceBackward);
     CPPUNIT_TEST(testRedlineFrame);
+    CPPUNIT_TEST(testBookmarkCopy);
     CPPUNIT_TEST(testFdo69893);
     CPPUNIT_TEST(testFdo70807);
     CPPUNIT_TEST(testImportRTF);
@@ -221,6 +223,61 @@ void SwUiWriterTest::testRedlineFrame()
     CPPUNIT_ASSERT_EQUAL(sal_Int32(1), xDrawPage->getCount());
 }
 
+void SwUiWriterTest::testBookmarkCopy()
+{
+    SwDoc * pDoc(createDoc());
+
+    // add text and bookmark
+    IDocumentMarkAccess & rIDMA(*pDoc->getIDocumentMarkAccess());
+    IDocumentContentOperations & rIDCO(pDoc->getIDocumentContentOperations());
+    SwNodeIndex aIdx(pDoc->GetNodes().GetEndOfContent(), -1);
+    SwCursor aPaM(SwPosition(aIdx), nullptr, false);
+    rIDCO.InsertString(aPaM, OUString("foo"));
+    rIDCO.SplitNode(*aPaM.GetPoint(), false);
+    rIDCO.InsertString(aPaM, OUString("bar"));
+    aPaM.SetMark();
+    aPaM.MovePara(GetfnParaCurr(), GetfnParaStart());
+    rIDMA.makeMark(aPaM, OUString("Mark"), IDocumentMarkAccess::MarkType::BOOKMARK);
+    aPaM.Exchange();
+    aPaM.DeleteMark();
+    rIDCO.SplitNode(*aPaM.GetPoint(), false);
+    rIDCO.InsertString(aPaM, OUString("baz"));
+
+    // copy range
+    rIDCO.SplitNode(*aPaM.GetPoint(), false);
+    SwPosition target(*aPaM.GetPoint());
+    aPaM.Move(fnMoveBackward, fnGoContent);
+    aPaM.SetMark();
+    aPaM.SttEndDoc(true/*start*/);
+    aPaM.Move(fnMoveForward, fnGoContent); // partially select 1st para
+
+    rIDCO.CopyRange(aPaM, target, /*bCopyAll=*/false, /*bCheckPos=*/true);
+
+    // check bookmark was copied to correct position
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(2), rIDMA.getBookmarksCount());
+    for (auto it(rIDMA.getBookmarksBegin()); it != rIDMA.getBookmarksEnd(); ++it)
+    {
+        OUString markText(SwPaM((*it)->GetMarkPos(), (*it)->GetOtherMarkPos()).GetText());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar"), markText);
+    }
+
+    // copy 2nd time, such that bCanMoveBack is false in CopyImpl
+    SwPaM aCopyPaM(*aPaM.GetMark(), *aPaM.GetPoint());
+    aPaM.SttEndDoc(true/*start*/);
+    rIDCO.SplitNode(*aPaM.GetPoint(), false);
+    aPaM.SttEndDoc(true/*start*/);
+
+    rIDCO.CopyRange(aCopyPaM, *aPaM.GetPoint(), /*bCopyAll=*/false, /*bCheckPos=*/true);
+
+    // check bookmark was copied to correct position
+    CPPUNIT_ASSERT_EQUAL(sal_Int32(3), rIDMA.getBookmarksCount());
+    for (auto it(rIDMA.getBookmarksBegin()); it != rIDMA.getBookmarksEnd(); ++it)
+    {
+        OUString markText(SwPaM((*it)->GetMarkPos(), (*it)->GetOtherMarkPos()).GetText());
+        CPPUNIT_ASSERT_EQUAL(OUString("bar"), markText);
+    }
+}
+
 void SwUiWriterTest::testFdo75110()
 {
     SwDoc* pDoc = createDoc("fdo75110.odt");
diff --git a/sw/source/core/doc/DocumentContentOperationsManager.cxx b/sw/source/core/doc/DocumentContentOperationsManager.cxx
index 1f387f4..77228f0 100644
--- a/sw/source/core/doc/DocumentContentOperationsManager.cxx
+++ b/sw/source/core/doc/DocumentContentOperationsManager.cxx
@@ -3129,11 +3129,14 @@ void DocumentContentOperationsManager::CopyWithFlyInFly(
     const SwNodeRange& rRg,
     const sal_Int32 nEndContentIndex,
     const SwNodeIndex& rInsPos,
-    const SwPaM* pCopiedPaM,
+    const std::pair<const SwPaM&, const SwPosition&>* pCopiedPaM /*and real insert pos*/,
     const bool bMakeNewFrms,
     const bool bDelRedlines,
     const bool bCopyFlyAtFly ) const
 {
+    assert(!pCopiedPaM || pCopiedPaM->first.End()->nContent == nEndContentIndex);
+    assert(!pCopiedPaM || pCopiedPaM->first.End()->nNode == rRg.aEnd);
+
     SwDoc* pDest = rInsPos.GetNode().GetDoc();
 
     _SaveRedlEndPosForRestore aRedlRest( rInsPos, 0 );
@@ -3176,14 +3179,24 @@ void DocumentContentOperationsManager::CopyWithFlyInFly(
     SwNodeRange aCpyRange( aSavePos, rInsPos );
 
     // Also copy all bookmarks
+    // guess this must be done before the _DelDummyNodes below as that
+    // deletes nodes so would mess up the index arithmetic
     if( m_rDoc.getIDocumentMarkAccess()->getAllMarksCount() )
     {
         SwPaM aRgTmp( rRg.aStart, rRg.aEnd );
-        SwPaM aCpyTmp( aCpyRange.aStart, aCpyRange.aEnd );
+        SwPaM aCpyPaM(aCpyRange.aStart, aCpyRange.aEnd);
+        if (pCopiedPaM && rRg.aStart != pCopiedPaM->first.Start()->nNode)
+        {
+            // there is 1 (partially selected, maybe) paragraph before
+            assert(SwNodeIndex(rRg.aStart, -1) == pCopiedPaM->first.Start()->nNode);
+            // only use the passed in target SwPosition if the source PaM point
+            // is on a different node; if it was the same node then the target
+            // position was likely moved along by the copy operation and now
+            // points to the end of the range!
+            *aCpyPaM.GetPoint() = pCopiedPaM->second;
+        }
 
-        lcl_CopyBookmarks(
-            pCopiedPaM != NULL ? *pCopiedPaM : aRgTmp,
-            aCpyTmp );
+        lcl_CopyBookmarks((pCopiedPaM) ? pCopiedPaM->first : aRgTmp, aCpyPaM);
     }
 
     if( bDelRedlines && ( nsRedlineMode_t::REDLINE_DELETE_REDLINES & pDest->getIDocumentRedlineAccess().GetRedlineMode() ))
@@ -3192,6 +3205,12 @@ void DocumentContentOperationsManager::CopyWithFlyInFly(
     pDest->GetNodes()._DelDummyNodes( aCpyRange );
 }
 
+// TODO: there is a limitation here in that it's not possible to pass a start
+// content index - which means that at-character anchored frames inside
+// partial 1st paragraph of redline is not copied.
+// But the DelFlyInRange() that is called from DelCopyOfSection() does not
+// delete it either, and it also does not delete those on partial last para of
+// redline, so copying those is supressed here too ...
 void DocumentContentOperationsManager::CopyFlyInFlyImpl(
     const SwNodeRange& rRg,
     const sal_Int32 nEndContentIndex,
@@ -4388,16 +4407,26 @@ bool DocumentContentOperationsManager::CopyImpl( SwPaM& rPam, SwPosition& rPos,
                     pDestTextNd->ResetAttr( RES_PAGEDESC );
             }
 
+            SwPosition startPos(SwNodeIndex(pCopyPam->GetPoint()->nNode, +1),
+                SwIndex(SwNodeIndex(pCopyPam->GetPoint()->nNode, +1).GetNode().GetContentNode()));
+            if (bCanMoveBack)
+            {   // pCopyPam is actually 1 before the copy range so move it fwd
+                SwPaM temp(*pCopyPam->GetPoint());
+                temp.Move(fnMoveForward, fnGoContent);
+                startPos = *temp.GetPoint();
+            }
+            assert(startPos.nNode.GetNode().IsContentNode());
+            std::pair<SwPaM const&, SwPosition const&> tmp(rPam, startPos);
             if( aInsPos == pEnd->nNode )
             {
                 SwNodeIndex aSaveIdx( aInsPos, -1 );
-                CopyWithFlyInFly( aRg, 0,aInsPos, &rPam, bMakeNewFrms, false );
+                CopyWithFlyInFly( aRg, 0, aInsPos, &tmp, bMakeNewFrms, false );
                 ++aSaveIdx;
                 pEnd->nNode = aSaveIdx;
                 pEnd->nContent.Assign( aSaveIdx.GetNode().GetTextNode(), 0 );
             }
             else
-                CopyWithFlyInFly( aRg, pEnd->nContent.GetIndex(), aInsPos, &rPam, bMakeNewFrms, false );
+                CopyWithFlyInFly( aRg, pEnd->nContent.GetIndex(), aInsPos, &tmp, bMakeNewFrms, false );
 
             bCopyBookmarks = false;
 
diff --git a/sw/source/core/inc/DocumentContentOperationsManager.hxx b/sw/source/core/inc/DocumentContentOperationsManager.hxx
index 44560fc..b6753a5 100644
--- a/sw/source/core/inc/DocumentContentOperationsManager.hxx
+++ b/sw/source/core/inc/DocumentContentOperationsManager.hxx
@@ -103,7 +103,7 @@ public:
     void CopyWithFlyInFly( const SwNodeRange& rRg,
                             const sal_Int32 nEndContentIndex,
                             const SwNodeIndex& rInsPos,
-                            const SwPaM* pCopiedPaM = NULL,
+                            const std::pair<const SwPaM&, const SwPosition&> * pCopiedPaM = nullptr,
                             bool bMakeNewFrms = true,
                             bool bDelRedlines = true,
                             bool bCopyFlyAtFly = false ) const;


More information about the Libreoffice-commits mailing list