From 7ae84ed99b133cdb4c197ecb43e2454f90b0439a Mon Sep 17 00:00:00 2001 From: Justin Luth Date: Fri, 26 Apr 2024 14:23:34 -0400 Subject: 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 Reviewed-by: Miklos Vajna Tested-by: Jenkins --- .../ooxmlexport/data/tdf160814_commentOrder.docx | Bin 0 -> 16168 bytes sw/qa/extras/ooxmlexport/ooxmlexport21.cxx | 14 +++++ .../writerfilter/dmapper/DomainMapper_Impl.cxx | 70 +++++++++++++++++++++ 3 files changed, 84 insertions(+) create mode 100644 sw/qa/extras/ooxmlexport/data/tdf160814_commentOrder.docx (limited to 'sw') 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 Binary files /dev/null and b/sw/qa/extras/ooxmlexport/data/tdf160814_commentOrder.docx differ 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 xModel(static_cast(m_xTextDocument.get())); + const uno::Reference xTFS(xModel, uno::UNO_QUERY); + if (xTFS.is()) + { + // sadly, the enumeration is not correctly sorted, which really complicates things. + const uno::Reference xFieldEA(xTFS->getTextFields()); + bool bRetry(false); + // keep track of the relevant (unsorted) fields that might have the same end point + std::vector> xRetryFields; + + uno::Reference 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 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 const xTextRange(xCursor, uno::UNO_QUERY_THROW); // Attach the annotation to the range. -- cgit