[Libreoffice-commits] core.git: Branch 'distro/lhm/libreoffice-5-2+backports' - sw/qa sw/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Fri Aug 31 11:03:11 UTC 2018


 sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott |binary
 sw/qa/extras/uiwriter/uiwriter.cxx                      |   62 ++++++++++++++++
 sw/source/core/unocore/unoportenum.cxx                  |   16 ++--
 3 files changed, 70 insertions(+), 8 deletions(-)

New commits:
commit ae6a0cac1fc367f1a5e34a2139d45b17f365de29
Author:     Serge Krot <Serge.Krot at cib.de>
AuthorDate: Fri Aug 24 13:31:54 2018 +0200
Commit:     Thorsten Behrens <Thorsten.Behrens at CIB.de>
CommitDate: Fri Aug 31 13:02:42 2018 +0200

    sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames
    
    Added fix for
     Change-Id: Ic1f173c85d3824afabb5b7ebf3a8594311eb9007
     Reviewed-on: https://gerrit.libreoffice.org/46889
     Reviewed-by: Miklos Vajna <vmiklos at collabora.co.uk>
     Tested-by: Jenkins <ci at libreoffice.org>
    
    The problem was (the same condition of the bOnlyFrameStarts parameter
    was used during output of Start and End bookmarks):
      if (BkmType::Start == pPtr->nBkmType && !bOnlyFrameStarts)
      ...
      if (BkmType::End   == pPtr->nBkmType && !bOnlyFrameStarts)
      ...
    Should be:
      if (BkmType::Start == pPtr->nBkmType &&  bOnlyFrameStarts)
      ...
      if (BkmType::End   == pPtr->nBkmType && !bOnlyFrameStarts)
      ...
    
    I assume this was a simple copy-paste bug.
    
    Reviewed-on: https://gerrit.libreoffice.org/59556
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    
    Conflicts:
            sw/source/core/unocore/unoportenum.cxx
    
     It looks like you may be committing a cherry-pick.
     If this is not correct, please remove the file
            .git/CHERRY_PICK_HEAD
     and try again.
    
    Change-Id: I712a0dccc1638fed3b81c65628033a4dc06c1ca4
    
    sw: fix inconsistent bookmark behavior around at-char/as-char anchored frames
    
    Added unit test for Added fix for
       Change-Id: Ic1f173c85d3824afabb5b7ebf3a8594311eb9007
    
    Reviewed-on: https://gerrit.libreoffice.org/59828
    Tested-by: Jenkins
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    
     Conflicts:
            sw/qa/extras/uiwriter/uiwriter.cxx
    
     Changes to be committed:
            new file:   sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott
            modified:   sw/qa/extras/uiwriter/uiwriter.cxx
    
    Change-Id: I38444587d00b96d52ff725dc7c5852e057bc6bd9
    Reviewed-on: https://gerrit.libreoffice.org/59854
    Reviewed-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>
    Tested-by: Thorsten Behrens <Thorsten.Behrens at CIB.de>

diff --git a/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott b/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott
new file mode 100644
index 000000000000..ff3970a27b27
Binary files /dev/null and b/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott differ
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index 2410ab136a8e..c07369a36450 100755
--- a/sw/qa/extras/uiwriter/uiwriter.cxx
+++ b/sw/qa/extras/uiwriter/uiwriter.cxx
@@ -217,6 +217,7 @@ public:
     void testTdf113877NoMerge();
     void testTdf113877_default_style();
     void testTdf113877_Standard_style();
+    void testInconsistentBookmark();
 
     CPPUNIT_TEST_SUITE(SwUiWriterTest);
     CPPUNIT_TEST(testReplaceForward);
@@ -332,6 +333,7 @@ public:
     CPPUNIT_TEST(testTdf113877NoMerge);
     CPPUNIT_TEST(testTdf113877_default_style);
     CPPUNIT_TEST(testTdf113877_Standard_style);
+    CPPUNIT_TEST(testInconsistentBookmark);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -4021,6 +4023,66 @@ void SwUiWriterTest::testTdf113877_Standard_style()
     CPPUNIT_ASSERT_EQUAL(listId1, listId3);
 }
 
+// Unit test for fix inconsistent bookmark behavior around at-char/as-char anchored frames
+//
+// We have a placeholder character in the sw doc model for as-char anchored frames,
+// so it's possible to have a bookmark before/after the frame or a non-collapsed bookmark
+// which covers the frame. The same is not true for at-char anchored frames,
+// where the anchor points to a doc model position, but there is no placeholder character.
+// If a bookmark is created covering the start and end of the anchor of the frame,
+// internally we create a collapsed bookmark which has the same position as the anchor of the frame.
+// When this doc model is handled by SwXParagraph::createEnumeration(),
+// first the frame and then the bookmark is appended to the text portion enumeration,
+// so your bookmark around the frame is turned into a collapsed bookmark after the frame.
+// (The same happens when we roundtrip an ODT document representing this doc model.)
+//
+// Fix the problem by inserting collapsed bookmarks with affected anchor positions
+// (same position is the anchor for an at-char frame) into the enumeration in two stages:
+// first the start of them before frames and then the end of them + other bookmarks.
+// This way UNO API users get their non-collapsed bookmarks around at-char anchored frames,
+// similar to as-char ones.
+void SwUiWriterTest::testInconsistentBookmark()
+{
+    // create test document with text and bookmark
+    {
+        SwDoc* pDoc(createDoc("testInconsistentBookmark.ott"));
+        IDocumentMarkAccess& rIDMA(*pDoc->getIDocumentMarkAccess());
+        SwNodeIndex aIdx(pDoc->GetNodes().GetEndOfContent(), -1);
+        SwCursor aPaM(SwPosition(aIdx), nullptr);
+        aPaM.SetMark();
+        aPaM.MovePara(GoCurrPara, fnParaStart);
+        aPaM.MovePara(GoCurrPara, fnParaEnd);
+        rIDMA.makeMark(aPaM, "Mark", IDocumentMarkAccess::MarkType::BOOKMARK,
+                       ::sw::mark::InsertMode::New);
+        aPaM.Exchange();
+        aPaM.DeleteMark();
+    }
+
+    // save document and verify the bookmark scoup
+    {
+        // save document
+        utl::TempFile aTempFile;
+        save("writer8", aTempFile);
+
+        // load only content.xml
+        if (xmlDocPtr pXmlDoc = parseExportInternal(aTempFile.GetURL(), "content.xml"))
+        {
+            const OString aPath("/office:document-content/office:body/office:text/text:p");
+
+            const OUString aTagBookmarkStart("bookmark-start");
+            const OUString aTagControl("control");
+            const OUString aTagBookmarkEnd("bookmark-end");
+
+            const int pos1 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkStart);
+            const int pos2 = getXPathPosition(pXmlDoc, aPath, aTagControl);
+            const int pos3 = getXPathPosition(pXmlDoc, aPath, aTagBookmarkEnd);
+
+            CPPUNIT_ASSERT_GREATER(pos1, pos2);
+            CPPUNIT_ASSERT_GREATER(pos2, pos3);
+        }
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(SwUiWriterTest);
 CPPUNIT_PLUGIN_IMPLEMENT();
 
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index 384d4cc6ed7a..ceff31905147 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -627,7 +627,7 @@ static void lcl_ExportBookmark(
 {
     for ( SwXBookmarkPortion_ImplList::iterator aIter = rBkmArr.begin(), aEnd = rBkmArr.end(); aIter != aEnd; )
     {
-        SwXBookmarkPortion_ImplSharedPtr pPtr = (*aIter);
+        const SwXBookmarkPortion_ImplSharedPtr& pPtr = (*aIter);
         if ( nIndex > pPtr->getIndex() )
         {
             if (bOnlyFrameStarts)
@@ -639,9 +639,8 @@ static void lcl_ExportBookmark(
         if ( nIndex < pPtr->getIndex() )
             break;
 
-        SwXTextPortion* pPortion = nullptr;
-        if ((BKM_TYPE_START == pPtr->nBkmType && !bOnlyFrameStarts) ||
-            (BKM_TYPE_START_END == pPtr->nBkmType))
+        if ((BkmType::Start == pPtr->nBkmType && bOnlyFrameStarts) ||
+            (BkmType::StartEnd == pPtr->nBkmType))
         {
             bool bFrameStart = rFramePositions.find(nIndex) != rFramePositions.end();
             bool bEnd = pPtr->nBkmType == BKM_TYPE_START_END && bFrameStart && !bOnlyFrameStarts;
@@ -654,21 +653,22 @@ static void lcl_ExportBookmark(
                 // - this is the start or end (depending on bOnlyFrameStarts)
                 //   of a collapsed bookmark at the same position as an at-char
                 //   anchored frame
-                pPortion =
+                SwXTextPortion* pPortion =
                     new SwXTextPortion(pUnoCursor, xParent, bEnd ? PORTION_BOOKMARK_END : PORTION_BOOKMARK_START);
                 rPortions.emplace_back(pPortion);
                 pPortion->SetBookmark(pPtr->xBookmark);
                 pPortion->SetCollapsed( BKM_TYPE_START_END == pPtr->nBkmType && !bFrameStart );
             }
-
         }
-        if (BKM_TYPE_END == pPtr->nBkmType && !bOnlyFrameStarts)
+        else if (BkmType::End == pPtr->nBkmType && !bOnlyFrameStarts)
         {
-            pPortion =
+            SwXTextPortion* pPortion =
                 new SwXTextPortion(pUnoCursor, xParent, PORTION_BOOKMARK_END);
             rPortions.push_back(pPortion);
             pPortion->SetBookmark(pPtr->xBookmark);
         }
+
+        // next bookmark
         if (bOnlyFrameStarts)
             ++aIter;
         else


More information about the Libreoffice-commits mailing list