tdf#160700: Avoid both bookmark-start and bookmark-end at the same index

There is a special handling of CrossRefBookmark, which has no end position
in the document model, but must span the whole paragraph, and end position
is generated explicitly.

Since commit 1d7ce421480d9170316533de03feb8d04eb5c767 (tdf#159438: when
there's no frame, close previous bookmark first, 2024-01-30), end marks
of an index are sorted before start marks of the same index, with the
expectation that start / end marks represent non-empty span. Dun in case
of empty paragraphs with a CrossRefBookmark, both start and end mark were
emitted into the same index, and the new sorting resulted in the wrong
order of the elements.

Fix this by checking if the start index is less than node end, and don't
handle CrossRefBookmark specially, if the check is negative. This writes
a single text:bookmark, instead of a text:bookmark-start, followed by a
text:bookmark-end.

Change-Id: I533c4f7814edddc3cf24b1213490f251d60b2273
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166266
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Signed-off-by: Xisco Fauli <xiscofauli@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166280
(cherry picked from commit 63ebbc33e8b1194f1e0ef7b58a98c6e4236476a9)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/166219
Reviewed-by: Patrick Luby <guibomacdev@gmail.com>
Reviewed-by: Ilmari Lauhakangas <ilmari.lauhakangas@libreoffice.org>

3 files changed, 33 insertions, 0 deletions

diff --git a/sw/qa/extras/odfexport/data/tdf160700.odt b/sw/qa/extras/odfexport/data/tdf160700.odt
new file mode 100644
index 000000000000..bc1515da3f82
--- /dev/null
+++ b/sw/qa/extras/odfexport/data/tdf160700.odt
diff --git a/sw/qa/extras/odfexport/odfexport2.cxx b/sw/qa/extras/odfexport/odfexport2.cxx
index 49836082907c..f654821acf6a 100644
--- a/sw/qa/extras/odfexport/odfexport2.cxx
+++ b/sw/qa/extras/odfexport/odfexport2.cxx
@@ -21,6 +21,7 @@
 #include <com/sun/star/text/XDocumentIndex.hpp>
 #include <com/sun/star/text/XDocumentIndexesSupplier.hpp>
 #include <com/sun/star/text/XTextColumns.hpp>
+#include <com/sun/star/text/XTextField.hpp>
 #include <com/sun/star/text/XTextFieldsSupplier.hpp>
 #include <com/sun/star/text/XTextTable.hpp>
 #include <com/sun/star/text/XTextTablesSupplier.hpp>
@@ -1343,6 +1344,35 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf159438)
                 u"bookmark3"_ustr);
 }
 
+CPPUNIT_TEST_FIXTURE(Test, testTdf160700)
+{
+    // Given a document with an empty numbered paragraph, and a cross-reference to it
+    loadAndReload("tdf160700.odt");
+
+    // Refresh fields and ensure cross-reference to numbered para is okay
+    auto xTextFieldsSupplier(mxComponent.queryThrow<text::XTextFieldsSupplier>());
+    auto xFieldsAccess(xTextFieldsSupplier->getTextFields());
+
+    xFieldsAccess.queryThrow<util::XRefreshable>()->refresh();
+
+    auto xFields(xFieldsAccess->createEnumeration());
+    CPPUNIT_ASSERT(xFields->hasMoreElements());
+    auto xTextField(xFields->nextElement().queryThrow<text::XTextField>());
+    // Save must not create markup with text:bookmark-end element before text:bookmark-start
+    // Withoud the fix, this would fail with
+    // - Expected: 1
+    // - Actual  : Error: Reference source not found
+    // i.e., the bookmark wasn't imported, and the field had no proper source
+    CPPUNIT_ASSERT_EQUAL(u"1"_ustr, xTextField->getPresentation(false));
+
+    xmlDocUniquePtr pXmlDoc = parseExport("content.xml");
+    // Check that we export the bookmark in the empty paragraph as a single text:bookmark
+    // element. Another walid markup is text:bookmark-start followed by text:bookmark-end
+    // (in that order). The problem was, that text:bookmark-end was before text:bookmark-start.
+    assertXPathChildren(pXmlDoc, "//office:text/text:list/text:list-item/text:p"_ostr, 1);
+    assertXPath(pXmlDoc, "//office:text/text:list/text:list-item/text:p/text:bookmark"_ostr);
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/core/unocore/unoportenum.cxx b/sw/source/core/unocore/unoportenum.cxx
index 494cec746865..709d79ef4d4b 100644
--- a/sw/source/core/unocore/unoportenum.cxx
+++ b/sw/source/core/unocore/unoportenum.cxx
@@ -150,8 +150,11 @@ namespace
         bool const hasOther = isExpanded && rStartPos != rEndPos;
         bool const bStartPosInNode = rStartPos.GetNode() == rOwnNode;
         bool const bEndPosInNode = rEndPos.GetNode() == rOwnNode;
+        // tdf#160700: Crossrefbookmarks only need separate start and end, when the start
+        // isn't in the end position (so in empty nodes, no need to handle them specially)
         sw::mark::CrossRefBookmark* const pCrossRefMark
             = !isExpanded && bStartPosInNode
+                      && rStartPos.GetContentIndex() < rStartPos.GetContentNode()->Len()
                   ? dynamic_cast<sw::mark::CrossRefBookmark*>(pBkmk)
                   : nullptr;