tdf#160814 writerfilter: insert comment after other comments

If multiple comments all have on the same range
(comment replies typically, but not necessarily)
then make sure that later replies are not inserted
in front of the earlier replies.

MSO can anchor their comments anywhere in the document,
so this is not a perfect, fool-proof solution.
However, it should handle the typical cases.

make CppunitTest_sw_ooxmlexport21 /
CPPUNIT_TEST_NAME=testTdf160814_commentOrder

Change-Id: I8ebdae83e4dec25ee34dffc3e84bbd3068a15f57
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166765
Reviewed-by: Justin Luth <jluth@mail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

3 files changed, 84 insertions, 0 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf160814_commentOrder.docx b/sw/qa/extras/ooxmlexport/data/tdf160814_commentOrder.docx
new file mode 100644
index 000000000000..f8d93e70b409
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf160814_commentOrder.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
index 15af73057d75..01c42d0ea34d 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport21.cxx
@@ -437,6 +437,20 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf159207_footerFramePrBorder)
     // TODO: there SHOULD BE a top border, and even if loaded, it would be lost on re-import...
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf160814_commentOrder)
+{
+    // given a document with a comment and 5 replies
+    loadAndSave("tdf160814_commentOrder.docx");
+
+    // make sure the order of the comments is imported and exported correctly
+    xmlDocUniquePtr pXmlComments = parseExport("word/comments.xml");
+    // This really should be "1. First comment", the 1. being list numbering...
+    assertXPathContent(pXmlComments, "//w:comment[1]//w:t"_ostr, "First comment");
+    assertXPathContent(pXmlComments, "//w:comment[2]//w:t"_ostr, "1.1 first reply.");
+    assertXPathContent(pXmlComments, "//w:comment[4]//w:t"_ostr, "1.3");
+    assertXPathContent(pXmlComments, "//w:comment[6]//w:t"_ostr, "1.5");
+}
+
 CPPUNIT_TEST_FIXTURE(Test, testPersonalMetaData)
 {
     // 1. Remove all personal info
diff --git a/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx b/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
index a0f0af0831f5..a26c517064ec 100644
--- a/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
+++ b/sw/source/writerfilter/dmapper/DomainMapper_Impl.cxx
@@ -4545,6 +4545,76 @@ void DomainMapper_Impl::PopAnnotation()
             }
 
             xCursor->gotoRange(aAnnotationPosition.m_xEnd, true);
+
+            // MS Word can anchor a comment anywhere in the document, but LO always anchors it
+            // immediately at the end of the range it spans.
+            // If multiple comments have the same end-point, we need to expand the range so that
+            // this later comment doesn't insert itself before any earlier ones.
+            const uno::Reference<frame::XModel> xModel(static_cast<SfxBaseModel*>(m_xTextDocument.get()));
+            const uno::Reference<text::XTextFieldsSupplier> xTFS(xModel, uno::UNO_QUERY);
+            if (xTFS.is())
+            {
+                // sadly, the enumeration is not correctly sorted, which really complicates things.
+                const uno::Reference<container::XEnumerationAccess> xFieldEA(xTFS->getTextFields());
+                bool bRetry(false);
+                // keep track of the relevant (unsorted) fields that might have the same end point
+                std::vector<uno::Reference<text::XTextRange>> xRetryFields;
+
+                uno::Reference<container::XEnumeration> xEnum = xFieldEA->createEnumeration();
+                // Although the primary interest is other comments, the same principle
+                // probably applies to any kind of field.
+                while (xEnum->hasMoreElements())
+                {
+                    try
+                    {
+                        // IMPORTANT: nextElement() MUST run before any possible exception...
+                        const uno::Reference<text::XTextField> xField(xEnum->nextElement(),
+                                                                      uno::UNO_QUERY_THROW);
+                        if (xTextRangeCompare->compareRegionStarts(xCursor->getEnd(),
+                                                                   xField->getAnchor()) == 1)
+                        {
+                            // Not interesting: our comment-to-insert ends before this field begins
+                            continue;
+                        }
+
+                        const sal_Int32 nCompare = xTextRangeCompare->compareRegionEnds(
+                            xCursor->getEnd(), xField->getAnchor());
+                        if (nCompare == 0) // both end at the same spot
+                        {
+                            bRetry = xCursor->goRight(1, true);
+                        }
+                        else if (nCompare == 1) // this field ends later than our comment-to-insert
+                        {
+                            // current implementation of SwModify::Add is basically in reverse order
+                            // so placing at the front of the list should effectively sort them.
+                            xRetryFields.emplace(xRetryFields.begin(), xField->getAnchor());
+                        }
+                    }
+                    catch (uno::Exception&)
+                    {
+                    }
+                }
+                // if the comment range expanded, retry with cached relevant fields
+                while (bRetry && xRetryFields.size())
+                {
+                    bRetry = false;
+                    auto iter = xRetryFields.cbegin();
+                    while (iter != xRetryFields.cend())
+                    {
+                        const sal_Int32 nCompare
+                            = xTextRangeCompare->compareRegionEnds(xCursor->getEnd(), *iter);
+                        if (nCompare == 1) // this field still ends later than our comment
+                            ++iter;
+                        else
+                        {
+                            iter = xRetryFields.erase(iter);
+                            if (nCompare == 0) // both end at the same spot
+                                bRetry = xCursor->goRight(1, true);
+                        }
+                    }
+                }
+            }
+
             uno::Reference<text::XTextRange> const xTextRange(xCursor, uno::UNO_QUERY_THROW);
 
             // Attach the annotation to the range.