tdf#89405 DOC export: fix corrupted comment order

Code for correctly exporting a Writer document with comments to a MS Word 97-2003 .doc format.
Also includes a unit test.

Reviewed on:
	https://gerrit.libreoffice.org/14841

Change-Id: I9f8f4fa466908335d98c43bd7d3e387bfe9baf6a

4 files changed, 80 insertions, 9 deletions

diff --git a/sw/qa/extras/ww8export/data/comment-export.odt b/sw/qa/extras/ww8export/data/comment-export.odt
new file mode 100644
index 000000000000..66873d869aa5
--- /dev/null
+++ b/sw/qa/extras/ww8export/data/comment-export.odt
diff --git a/sw/qa/extras/ww8export/ww8export.cxx b/sw/qa/extras/ww8export/ww8export.cxx
index 7d8a5b8e06bb..3a50b3ea953a 100644
--- a/sw/qa/extras/ww8export/ww8export.cxx
+++ b/sw/qa/extras/ww8export/ww8export.cxx
@@ -464,6 +464,71 @@ DECLARE_WW8EXPORT_TEST(testCommentedTable, "commented-table.doc")
     CPPUNIT_ASSERT_EQUAL(OUString("fore." SAL_NEWLINE_STRING "A1" SAL_NEWLINE_STRING "B1" SAL_NEWLINE_STRING "Afte"), xField->getAnchor()->getString());
 }
 
+DECLARE_WW8EXPORT_TEST(testCommentExport, "comment-export.odt")
+{
+    struct TextPortionInfo {
+        OUString sKind;
+        OUString sText;
+        int nAnnotationID;
+    };
+
+    const TextPortionInfo aTextPortions[] = {
+        {OUString("Annotation"), OUString("Comment on [A...A]"), 0},
+        {OUString("Text"), OUString("[A xx "), 0},
+        {OUString("Annotation"), OUString("Comment on [B...B]"), 1},
+        {OUString("Text"), OUString("[B x "), 0},
+        {OUString("Annotation"), OUString("Comment on [C..C]"), 2},
+        {OUString("Text"), OUString("[C x B]"), 0},
+        {OUString("AnnotationEnd"), OUString(""), 1},
+        {OUString("Text"), OUString(" x C]"), 0},
+        {OUString("AnnotationEnd"), OUString(""), 2},
+        {OUString("Text"), OUString(" xx A]"), 0},
+        {OUString("AnnotationEnd"), OUString(""), 0},
+        {OUString("Text"), OUString(" Comment on a point"), 0},
+        {OUString("Annotation"), OUString("Comment on point"), 3},
+        {OUString("Text"), OUString("x "), 0},
+        {OUString("Annotation"), OUString("Comment on AA...BB"), 4},
+        {OUString("Annotation"), OUString("Comment on AAAAAA"), 5},
+        {OUString("Text"), OUString("AAAAAA"), 0},
+        {OUString("AnnotationEnd"), OUString(""), 5},
+        {OUString("Text"), OUString(" BBBBBB"), 0},
+        {OUString("AnnotationEnd"), OUString(""), 4}
+    };
+
+    OUString sNames[6];
+
+    const int nNumberOfTextPortions = sizeof(aTextPortions) / (sizeof(TextPortionInfo));
+
+    uno::Reference<text::XTextRange> xPara = getParagraph(1);
+
+    for (int i = 0; i < nNumberOfTextPortions; ++i)
+    {
+        OUString sKind = aTextPortions[i].sKind;
+        uno::Reference<text::XTextRange> xRun(getRun(xPara, i + 1), uno::UNO_QUERY);
+        uno::Reference<beans::XPropertySet> xPropertySet(xRun, uno::UNO_QUERY);
+        CPPUNIT_ASSERT_EQUAL(sKind, getProperty<OUString>(xPropertySet, "TextPortionType"));
+
+        if (sKind == OUString("Text"))
+        {
+            // Check if textportion has the correct text
+            CPPUNIT_ASSERT_EQUAL(aTextPortions[i].sText, xRun->getString());
+        }
+        else if (sKind == OUString("Annotation"))
+        {
+            // Check if the comment text is correct and save the name of the comment
+            uno::Reference<beans::XPropertySet> xComment(getProperty< uno::Reference<beans::XPropertySet> >(xRun, "TextField"), uno::UNO_QUERY);
+            CPPUNIT_ASSERT_EQUAL(aTextPortions[i].sText, getProperty<OUString>(xComment, "Content"));
+            sNames[aTextPortions[i].nAnnotationID] = getProperty<OUString>(xComment, "Name");
+        }
+        else // if (sKind == OUString("AnnotationEnd"))
+        {
+            // Check if the correct Annotation ends here (by Name)
+            uno::Reference<container::XNamed> xBookmark(getProperty< uno::Reference<beans::XPropertySet> >(xRun, "Bookmark"), uno::UNO_QUERY);
+            CPPUNIT_ASSERT_EQUAL(sNames[aTextPortions[i].nAnnotationID], xBookmark->getName());
+        }
+    }
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/ww8/wrtw8sty.cxx b/sw/source/filter/ww8/wrtw8sty.cxx
index cad77acd9296..3f853e516fb9 100644
--- a/sw/source/filter/ww8/wrtw8sty.cxx
+++ b/sw/source/filter/ww8/wrtw8sty.cxx
@@ -2289,16 +2289,20 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                 std::vector< std::pair<WW8_CP, int> > aRangeStartPos; // The second of the pair is the original index before sorting.
                 std::vector< std::pair<WW8_CP, int> > aRangeEndPos; // Same, so we can map between the indexes before/after sorting.
                 std::map<int, int> aAtnStartMap; // Maps from annotation index to start index.
+                std::map<int, int> aStartAtnMap; // Maps from start index to annotation index.
                 std::map<int, int> aStartEndMap; // Maps from start index to end index.
                 // then write first the GrpXstAtnOwners
+                int nIdx = 0;
                 for ( i = 0; i < nLen; ++i )
                 {
                     const WW8_Annotation& rAtn = *(const WW8_Annotation*)aCntnt[i];
                     aStrArr.push_back(std::pair<OUString,OUString>(rAtn.msOwner,rAtn.m_sInitials));
+                    // record start and end positions for ranges
                     if( rAtn.m_nRangeStart != rAtn.m_nRangeEnd )
                     {
-                        aRangeStartPos.push_back(std::make_pair(rAtn.m_nRangeStart, i));
-                        aRangeEndPos.push_back(std::make_pair(rAtn.m_nRangeEnd, i));
+                        aRangeStartPos.push_back(std::make_pair(rAtn.m_nRangeStart, nIdx));
+                        aRangeEndPos.push_back(std::make_pair(rAtn.m_nRangeEnd, nIdx));
+                        ++nIdx;
                     }
                 }
 
@@ -2313,10 +2317,13 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                 // that knows what index to reference, after sorting.
                 std::sort(aRangeStartPos.begin(), aRangeStartPos.end(), &lcl_PosComp);
                 for (i = 0; i < aRangeStartPos.size(); ++i)
+                {
                     aAtnStartMap[aRangeStartPos[i].second] = i;
+                    aStartAtnMap[i] = aRangeStartPos[i].second;
+                }
                 std::sort(aRangeEndPos.begin(), aRangeEndPos.end(), &lcl_PosComp);
                 for (i = 0; i < aRangeEndPos.size(); ++i)
-                    aStartEndMap[aRangeEndPos[ aAtnStartMap[i] ].second] = i;
+                    aStartEndMap[aAtnStartMap[ aRangeEndPos[i].second ]] = i;
 
                 if ( rWrt.bWrtWW8 )
                 {
@@ -2354,7 +2361,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                         {
                             SwWW8Writer::WriteLong( *rWrt.pTableStrm, aRangeStartPos[i].first );
                         }
-                        SwWW8Writer::WriteLong( *rWrt.pTableStrm, aRangeStartPos[i-1].first + 1);
+                        SwWW8Writer::WriteLong( *rWrt.pTableStrm, rFib.ccpText + 1);
 
                         // Commented text ranges additional informations (Plcfbkf.aFBKF)
                         for ( i = 0; i < aRangeStartPos.size(); ++i )
@@ -2372,7 +2379,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                         {
                             SwWW8Writer::WriteLong( *rWrt.pTableStrm, aRangeEndPos[i].first );
                         }
-                        SwWW8Writer::WriteLong( *rWrt.pTableStrm, aRangeEndPos[i-1].first + 1);
+                        SwWW8Writer::WriteLong( *rWrt.pTableStrm, rFib.ccpText + 1);
 
                         nFcStart = rWrt.pTableStrm->Tell();
                         rFib.lcbPlcfAtnbkl = nFcStart - rFib.fcPlcfAtnbkl;
@@ -2388,7 +2395,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                             SwWW8Writer::WriteShort( *rWrt.pTableStrm, 0 );         // SttbfAtnBkmk.cchData
                             // One ATNBE structure for all text ranges
                             SwWW8Writer::WriteShort( *rWrt.pTableStrm, 0x0100 );    // ATNBE.bmc
-                            SwWW8Writer::WriteLong( *rWrt.pTableStrm, aAtnStartMap[i] );          // ATNBE.lTag
+                            SwWW8Writer::WriteLong( *rWrt.pTableStrm, aStartAtnMap[i] );          // ATNBE.lTag
                             SwWW8Writer::WriteLong( *rWrt.pTableStrm, -1 );         // ATNBE.lTagOld
                         }
 
@@ -2397,7 +2404,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                     }
                 }
 
-                // Write the extended >= Word XP ATLD records
+                // Write the extended >= Word XP ATRD records
                 if( rWrt.bWrtWW8 )
                 {
                     for( i = 0; i < nLen; ++i )
@@ -2541,7 +2548,7 @@ void WW8_WrPlcSubDoc::WriteGenericPlc( WW8Export& rWrt, sal_uInt8 nTTyp,
                 // SVBT16 ibst;      // index into GrpXstAtnOwners
                 // SVBT16 ak;        // not used
                 // SVBT16 grfbmc;    // not used
-                // SVBT32 ITagBkmk;  // when not -1, this tag identifies the
+                // SVBT32 ITagBkmk;  // when not -1, this tag identifies the ATNBE
 
                 SwWW8Writer::WriteShort( *rWrt.pTableStrm, nFndPos );
                 SwWW8Writer::WriteShort( *rWrt.pTableStrm, 0 );
diff --git a/sw/source/filter/ww8/wrtww8.cxx b/sw/source/filter/ww8/wrtww8.cxx
index 5998a155c285..021c0cf28195 100644
--- a/sw/source/filter/ww8/wrtww8.cxx
+++ b/sw/source/filter/ww8/wrtww8.cxx
@@ -1380,7 +1380,6 @@ void WW8Export::AppendAnnotationMarks(const SwTxtNode& rNode, sal_Int32 nAktPos,
             if (nStart == nAktPos)
             {
                 pAtn->AddRangeStartPosition(pMark->GetName(), Fc2Cp(Strm().Tell()));
-                return;
             }
         }
     }