ODT export: omit unreferenced <text:list xml:id="...">

This attribute is needed when a numbering is built using multiple,
independent <text:list> elements. In that case the markup to connect
these are either:

	<text:list text:style-name="L1">
	</text:list>
	...
	<text:list text:continue-numbering="true" text:style-name="L1">
	</text:list>

In case there is no other list in-between, or:

	<text:list xml:id="..." text:style-name="L1">
	</text:list>
	...
	<text:list text:continue-list="..." text:style-name="L1">
	</text:list>

In case there are other lists in-between.

This means that at least in case all the text nodes of the numbering are
after each other, then the random value in xml:id="..." is never
referenced, so it can be omitted.

This helps deterministic ODF output when the input is HTML, where there
are never text:continue-list="..." attributes that would refer to these
xml:id="..." attributes.

Change-Id: Ice69422a12d4229879f89f3a4a24ed926c6d43af
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/131322
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins

8 files changed, 105 insertions, 3 deletions

diff --git a/sw/inc/numrule.hxx b/sw/inc/numrule.hxx
index 1e23a8818163..e10603a7e3dc 100644
--- a/sw/inc/numrule.hxx
+++ b/sw/inc/numrule.hxx
@@ -273,6 +273,9 @@ public:
     void dumpAsXml(xmlTextWriterPtr w) const;
     void GetGrabBagItem(css::uno::Any& rVal) const;
     void SetGrabBagItem(const css::uno::Any& rVal);
+
+    /// Is it possible that this numbering has multiple lists?
+    bool HasContinueList() const;
 };
 
 /// namespace for static functions and methods for numbering and bullets
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 93a5c149f15c..40fec8c7d8d1 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -1154,6 +1154,29 @@ void SwNumRule::SetGrabBagItem(const uno::Any& rVal)
     mpGrabBagItem->PutValue(rVal, 0);
 }
 
+bool SwNumRule::HasContinueList() const
+{
+    // In case all text nodes are after each other, then we won't have a later list that wants to
+    // continue us.
+    SwNodeOffset nIndex(0);
+    for (size_t i = 0; i < maTextNodeList.size(); ++i)
+    {
+        SwTextNode* pNode = maTextNodeList[i];
+        if (i > 0)
+        {
+            if (pNode->GetIndex() != nIndex + 1)
+            {
+                // May have a continue list.
+                return true;
+            }
+        }
+        nIndex = pNode->GetIndex();
+    }
+
+    // Definitely won't have a continue list.
+    return false;
+}
+
 namespace numfunc
 {
     namespace {
diff --git a/sw/source/core/unocore/unoparagraph.cxx b/sw/source/core/unocore/unoparagraph.cxx
index f39bc2cc234f..428f64a53503 100644
--- a/sw/source/core/unocore/unoparagraph.cxx
+++ b/sw/source/core/unocore/unoparagraph.cxx
@@ -921,6 +921,22 @@ static beans::PropertyState lcl_SwXParagraph_getPropertyState(
             bDone = true;
             break;
         }
+        case FN_UNO_LIST_ID:
+        {
+            if (*ppSet)
+            {
+                if ((*ppSet)->GetItemState(RES_PARATR_LIST_ID, false) == SfxItemState::SET)
+                {
+                    SwNumRule* pNumRule = rTextNode.GetNumRule();
+                    if (!pNumRule || pNumRule->HasContinueList())
+                    {
+                        eRet = beans::PropertyState_DIRECT_VALUE;
+                    }
+                }
+                bDone = true;
+            }
+            break;
+        }
         case FN_UNO_ANCHOR_TYPES:
         {
             bDone = true;
diff --git a/xmloff/qa/unit/data/list-id.fodt b/xmloff/qa/unit/data/list-id.fodt
new file mode 100644
index 000000000000..377dbcbd6473
--- /dev/null
+++ b/xmloff/qa/unit/data/list-id.fodt
@@ -0,0 +1,23 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<office:document xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" office:version="1.3" office:mimetype="application/vnd.oasis.opendocument.text">
+  <office:automatic-styles>
+    <text:list-style style:name="L1">
+      <text:list-level-style-number text:level="1" text:style-name="Numbering_20_Symbols" style:num-suffix="." style:num-format="1"/>
+    </text:list-style>
+  </office:automatic-styles>
+  <office:body>
+    <office:text>
+      <text:list text:style-name="L1">
+        <text:list-item>
+          <text:p>First</text:p>
+        </text:list-item>
+        <text:list-item>
+          <text:p>Second</text:p>
+        </text:list-item>
+        <text:list-item>
+          <text:p>Third</text:p>
+        </text:list-item>
+      </text:list>
+    </office:text>
+  </office:body>
+</office:document>
diff --git a/xmloff/qa/unit/text.cxx b/xmloff/qa/unit/text.cxx
index 1d239e2e0c09..71772edb3585 100644
--- a/xmloff/qa/unit/text.cxx
+++ b/xmloff/qa/unit/text.cxx
@@ -245,6 +245,30 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testContinueNumberingWord)
     CPPUNIT_ASSERT_EQUAL(OUString("2."), aActual);
 }
 
+CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testListId)
+{
+    // Given a document with a simple list (no continue-list="..." attribute):
+    OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + "list-id.fodt";
+    getComponent() = loadFromDesktop(aURL);
+
+    // When storing that document as ODF:
+    uno::Reference<frame::XStorable> xStorable(getComponent(), uno::UNO_QUERY);
+    uno::Sequence<beans::PropertyValue> aStoreProps = comphelper::InitPropertySequence({
+        { "FilterName", uno::makeAny(OUString("writer8")) },
+    });
+    utl::TempFile aTempFile;
+    aTempFile.EnableKillingFile();
+    xStorable->storeToURL(aTempFile.GetURL(), aStoreProps);
+
+    // Then make sure that unreferenced xml:id="..." attributes are not written:
+    std::unique_ptr<SvStream> pStream = parseExportStream(aTempFile, "content.xml");
+    xmlDocUniquePtr pXmlDoc = parseXmlStream(pStream.get());
+    // Without the accompanying fix in place, this failed with:
+    // - XPath '//text:list' unexpected 'id' attribute
+    // i.e. xml:id="..." was written unconditionally, even when no other list needed it.
+    assertXPathNoAttribute(pXmlDoc, "//text:list", "id");
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/source/text/XMLTextNumRuleInfo.cxx b/xmloff/source/text/XMLTextNumRuleInfo.cxx
index c6889abbbc9e..062b92879ee2 100644
--- a/xmloff/source/text/XMLTextNumRuleInfo.cxx
+++ b/xmloff/source/text/XMLTextNumRuleInfo.cxx
@@ -21,6 +21,7 @@
 #include <osl/diagnose.h>
 #include <sal/log.hxx>
 #include <com/sun/star/beans/XPropertySet.hpp>
+#include <com/sun/star/beans/XPropertyState.hpp>
 #include <com/sun/star/beans/PropertyValue.hpp>
 #include <com/sun/star/container/XIndexReplace.hpp>
 #include <com/sun/star/style/NumberingType.hpp>
@@ -37,7 +38,8 @@ using namespace ::com::sun::star::style;
 
 // Complete refactoring of the class and enhancement of the class for lists.
 XMLTextNumRuleInfo::XMLTextNumRuleInfo()
-    : mnListStartValue( -1 )
+    : mbListIdIsDefault(false)
+    , mnListStartValue( -1 )
     , mnListLevel( 0 )
     , mbIsNumbered( false )
     , mbIsRestart( false )
@@ -59,6 +61,7 @@ void XMLTextNumRuleInfo::Set(
     mbOutlineStyleAsNormalListStyle = bOutlineStyleAsNormalListStyle;
 
     Reference< XPropertySet > xPropSet( xTextContent, UNO_QUERY );
+    Reference<XPropertyState> xPropState(xTextContent, UNO_QUERY);
     Reference< XPropertySetInfo > xPropSetInfo = xPropSet->getPropertySetInfo();
 
     // check if this paragraph supports a numbering
@@ -135,6 +138,12 @@ void XMLTextNumRuleInfo::Set(
         if( xPropSetInfo->hasPropertyByName( "ListId" ) )
         {
             xPropSet->getPropertyValue( "ListId" ) >>= msListId;
+
+            if (xPropState.is())
+            {
+                mbListIdIsDefault
+                    = xPropState->getPropertyState("ListId") == PropertyState_DEFAULT_VALUE;
+            }
         }
 
         mbContinueingPreviousSubTree = false;
diff --git a/xmloff/source/text/XMLTextNumRuleInfo.hxx b/xmloff/source/text/XMLTextNumRuleInfo.hxx
index 53ad8e97f188..adb405411164 100644
--- a/xmloff/source/text/XMLTextNumRuleInfo.hxx
+++ b/xmloff/source/text/XMLTextNumRuleInfo.hxx
@@ -43,6 +43,8 @@ class XMLTextNumRuleInfo
 
     // paragraph's list attributes
     OUString            msListId;
+    /// msListId won't be referenced by later lists.
+    bool mbListIdIsDefault;
     sal_Int16           mnListStartValue;
     sal_Int16           mnListLevel;
     bool                mbIsNumbered;
@@ -83,6 +85,8 @@ public:
         return msListId;
     }
 
+    bool IsListIdDefault() const { return mbListIdIsDefault; }
+
     sal_Int16 GetLevel() const
     {
         return mnListLevel;
diff --git a/xmloff/source/text/txtparae.cxx b/xmloff/source/text/txtparae.cxx
index 732c119cdb58..f8e46fb71844 100644
--- a/xmloff/source/text/txtparae.cxx
+++ b/xmloff/source/text/txtparae.cxx
@@ -932,7 +932,7 @@ void XMLTextParagraphExport::exportListChange(
                     {
                         if ( bExportODF &&
                             eODFDefaultVersion >= SvtSaveOptions::ODFSVER_012 &&
-                             !sListId.isEmpty() )
+                             !sListId.isEmpty() && !rNextInfo.IsListIdDefault() )
                         {
                             /* Property text:id at element <text:list> has to be
                                replaced by property xml:id (#i92221#)
@@ -951,7 +951,7 @@ void XMLTextParagraphExport::exportListChange(
                                         mpTextListsHelper->GenerateNewListId() );
                         if ( bExportODF &&
                             eODFDefaultVersion >= SvtSaveOptions::ODFSVER_012 &&
-                             !sListId.isEmpty() )
+                             !sListId.isEmpty() && !rNextInfo.IsListIdDefault() )
                         {
                             /* Property text:id at element <text:list> has to be
                                replaced by property xml:id (#i92221#)