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@collabora.co.uk>
Tested-by: Jenkins <ci@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@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.

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@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

38444587d00b96d52ff725dc7c5852e057bc6bd9
Reviewed-on: https://gerrit.libreoffice.org/59854
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>
Tested-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>

Change-Id: I712a0dccc1638fed3b81c65628033a4dc06c1ca4

3 files changed, 68 insertions, 7 deletions

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
--- /dev/null
+++ b/sw/qa/extras/uiwriter/data/testInconsistentBookmark.ott
diff --git a/sw/qa/extras/uiwriter/uiwriter.cxx b/sw/qa/extras/uiwriter/uiwriter.cxx
index 2410ab136a8e..99e7bc5b20f7 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,65 @@ 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(GetfnParaCurr(), GetfnParaStart());
+        aPaM.MovePara(GetfnParaCurr(), GetfnParaEnd());
+        rIDMA.makeMark(aPaM, "Mark", IDocumentMarkAccess::MarkType::BOOKMARK);
+        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(pos1 < pos2);
+            CPPUNIT_ASSERT(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..601c53305c04 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,8 +639,7 @@ static void lcl_ExportBookmark(
         if ( nIndex < pPtr->getIndex() )
             break;
 
-        SwXTextPortion* pPortion = nullptr;
-        if ((BKM_TYPE_START == pPtr->nBkmType && !bOnlyFrameStarts) ||
+        if ((BKM_TYPE_START == pPtr->nBkmType && bOnlyFrameStarts) ||
             (BKM_TYPE_START_END == pPtr->nBkmType))
         {
             bool bFrameStart = rFramePositions.find(nIndex) != rFramePositions.end();
@@ -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 (BKM_TYPE_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