ODF export: simplify restart handling to skip list id where possible

This continues to minimize cases where random ids are written, helping
to make the output more deterministic; it builds upon commits
8f48f91009caa86d896f247059874242ed18bf39 (ODT export: omit unreferenced
<text:list xml:id="...">, 2022-03-10), and
82bbf63582bdf28e7918e58ebf6657a9144bc9f3 (tdf#155823: Improve the check
if the list id is not required, 2023-06-14).

The previous code used to write 'text:continue-list' when the list is
restarted. It is unnecessary when there is no other condition requiring
such a reference (like style change, or interleaving lists); so relax
the conditions allowing to put simple 'text:continue-numbering="true"'.
This also allows to simplify a bit the code around 'ShouldSkipListId'.

Change-Id: Idf8be455953d08fd578266bda22f3a55d7b9ee23
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153104
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

3 files changed, 101 insertions, 18 deletions

diff --git a/xmloff/qa/unit/data/listRestartAfterBreak.fodt b/xmloff/qa/unit/data/listRestartAfterBreak.fodt
new file mode 100644
index 000000000000..7ae5c84d7060
--- /dev/null
+++ b/xmloff/qa/unit/data/listRestartAfterBreak.fodt
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<office:document xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+ <office:body>
+  <office:text>
+   <text:list text:style-name="Numbering 123">
+    <text:list-item>
+     <text:p>Item1</text:p>
+    </text:list-item>
+   </text:list>
+   <text:p/>
+   <text:list text:continue-numbering="true" text:style-name="Numbering 123">
+    <text:list-item>
+     <text:p>Item2</text:p>
+    </text:list-item>
+   </text:list>
+   <text:p/>
+   <text:list text:continue-numbering="true" text:style-name="Numbering 123">
+    <text:list-item text:start-value="1">
+     <text:p>Item3 (restarted)</text:p>
+    </text:list-item>
+   </text:list>
+  </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/xmloff/qa/unit/text.cxx b/xmloff/qa/unit/text.cxx
index bff3f8b14821..abd4a0e07dea 100644
--- a/xmloff/qa/unit/text.cxx
+++ b/xmloff/qa/unit/text.cxx
@@ -347,6 +347,67 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testListIdState)
     CPPUNIT_ASSERT(!id.isEmpty());
 }
 
+CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testListIdOnRestart)
+{
+    // Test that a restart of a continued list, by itself, does not introduce a unneeded xml:id
+    // and text:continue-list, but uses text:continue-numbering, and is imported correctly.
+
+    // Given a document with a list with a restart after break:
+    loadFromURL(u"listRestartAfterBreak.fodt");
+
+    auto xTextDocument(mxComponent.queryThrow<css::text::XTextDocument>());
+    auto xParaEnumAccess(xTextDocument->getText().queryThrow<css::container::XEnumerationAccess>());
+    auto xParaEnum(xParaEnumAccess->createEnumeration());
+
+    auto xPara(xParaEnum->nextElement().queryThrow<beans::XPropertySet>());
+    auto aActual(xPara->getPropertyValue("ListLabelString").get<OUString>());
+    CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+    OUString list_id = xPara->getPropertyValue("ListId").get<OUString>();
+    xParaEnum->nextElement(); // Skip empty intermediate paragraph
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+    CPPUNIT_ASSERT_EQUAL(OUString("2."), aActual);
+    CPPUNIT_ASSERT_EQUAL(list_id, xPara->getPropertyValue("ListId").get<OUString>());
+    xParaEnum->nextElement(); // Skip empty intermediate paragraph
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY);
+    aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+    // Check that restart was applied correctly, with simple 'text:continue-numbering="true"'
+    CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+    CPPUNIT_ASSERT_EQUAL(list_id, xPara->getPropertyValue("ListId").get<OUString>());
+
+    // When storing that document as ODF:
+    saveAndReload("writer8");
+
+    xTextDocument.set(mxComponent, uno::UNO_QUERY_THROW);
+    xParaEnumAccess.set(xTextDocument->getText(), uno::UNO_QUERY_THROW);
+    xParaEnum.set(xParaEnumAccess->createEnumeration());
+
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+    CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+    list_id = xPara->getPropertyValue("ListId").get<OUString>();
+    xParaEnum->nextElement(); // Skip empty intermediate paragraph
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+    CPPUNIT_ASSERT_EQUAL(OUString("2."), aActual);
+    CPPUNIT_ASSERT_EQUAL(list_id, xPara->getPropertyValue("ListId").get<OUString>());
+    xParaEnum->nextElement(); // Skip empty intermediate paragraph
+    xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+    aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+    CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+    CPPUNIT_ASSERT_EQUAL(list_id, xPara->getPropertyValue("ListId").get<OUString>());
+
+    // Then make sure that no xml:id="..." attribute is written, even in restarted case:
+    xmlDocUniquePtr pXmlDoc = parseExport("content.xml");
+    CPPUNIT_ASSERT(pXmlDoc);
+    assertXPath(pXmlDoc, "//text:list", 3);
+    assertXPathNoAttribute(pXmlDoc, "//text:list[1]", "id");
+    assertXPathNoAttribute(pXmlDoc, "//text:list[2]", "id");
+    assertXPathNoAttribute(pXmlDoc, "//text:list[3]", "id");
+    assertXPathNoAttribute(pXmlDoc, "//text:list[3]", "continue-list");
+    assertXPath(pXmlDoc, "//text:list[3]", "continue-numbering", "true");
+}
+
 CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testClearingBreakExport)
 {
     // Given a document with a clearing break:
diff --git a/xmloff/source/text/txtparae.cxx b/xmloff/source/text/txtparae.cxx
index c78c504bfa50..34e12aec02b2 100644
--- a/xmloff/source/text/txtparae.cxx
+++ b/xmloff/source/text/txtparae.cxx
@@ -1104,8 +1104,7 @@ void XMLTextParagraphExport::exportListChange(
                                 mpTextListsHelper->GetListStyleOfLastProcessedList() &&
                              // Inconsistent behavior regarding lists (#i92811#)
                              sContinueListId ==
-                                mpTextListsHelper->GetLastProcessedListId() &&
-                             !rNextInfo.IsRestart() )
+                                mpTextListsHelper->GetLastProcessedListId() )
                         {
                             GetExport().AddAttribute( XML_NAMESPACE_TEXT,
                                                       XML_CONTINUE_NUMBERING,
@@ -1120,15 +1119,15 @@ void XMLTextParagraphExport::exportListChange(
                                                           XML_CONTINUE_LIST,
                                                           sContinueListId );
                             }
+                        }
 
-                            if ( rNextInfo.IsRestart() &&
-                                 ( nListLevelsToBeOpened != 1 ||
-                                   !rNextInfo.HasStartValue() ) )
-                            {
-                                bRestartNumberingAtContinuedList = true;
-                                nRestartValueForContinuedList =
-                                            rNextInfo.GetListLevelStartValue();
-                            }
+                        if ( rNextInfo.IsRestart() &&
+                             ( nListLevelsToBeOpened != 1 ||
+                               !rNextInfo.HasStartValue() ) )
+                        {
+                            bRestartNumberingAtContinuedList = true;
+                            nRestartValueForContinuedList =
+                                        rNextInfo.GetListLevelStartValue();
                         }
 
                         mpTextListsHelper->KeepListAsProcessed( sNewListId,
@@ -1804,12 +1803,11 @@ struct XMLTextParagraphExport::DocumentListNodes
         sal_Int32 index; // see SwNode::GetIndex and SwNodeOffset
         sal_uInt64 style_id; // actually a pointer to NumRule
         OUString list_id;
-        bool isRestart;
     };
     std::vector<NodeData> docListNodes;
     DocumentListNodes(const css::uno::Reference<css::frame::XModel>& xModel)
     {
-        // Sequence of nodes, each of them represented by four-element sequence,
+        // Sequence of nodes, each of them represented by three-element sequence,
         // corresponding to NodeData members
         css::uno::Sequence<css::uno::Sequence<css::uno::Any>> nodes;
         if (auto xPropSet = xModel.query<css::beans::XPropertySet>())
@@ -1828,9 +1826,9 @@ struct XMLTextParagraphExport::DocumentListNodes
         docListNodes.reserve(nodes.getLength());
         for (const auto& node : nodes)
         {
-            assert(node.getLength() == 4);
+            assert(node.getLength() == 3);
             docListNodes.push_back({ node[0].get<sal_Int32>(), node[1].get<sal_uInt64>(),
-                                     node[2].get<OUString>(), node[3].get<bool>() });
+                                     node[2].get<OUString>() });
         }
 
         std::sort(docListNodes.begin(), docListNodes.end(),
@@ -1884,10 +1882,9 @@ struct XMLTextParagraphExport::DocumentListNodes
                 if (it->index + 1 != next->index)
                 {
                     // we have a gap before the next node with the same list and style,
-                    // with no other lists in between. There will be a continuation;
-                    // in case of restart, there will be a reference to the id;
-                    // otherwise, there will be simple 'text:continue-numbering="true"'.
-                    return !next->isRestart;
+                    // with no other lists in between. There will be a continuation with a
+                    // simple 'text:continue-numbering="true"'.
+                    return true;
                 }
                 it = next; // walk through adjacent nodes of the same list
             }