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.

(cherry picked from commit 8f48f91009caa86d896f247059874242ed18bf39)

Conflicts:
xmloff/source/text/XMLTextNumRuleInfo.cxx

Change-Id: Ice69422a12d4229879f89f3a4a24ed926c6d43af

12 files changed, 147 insertions, 3 deletions

diff --git a/include/unotest/macros_test.hxx b/include/unotest/macros_test.hxx
index c60ea1fe97ce..7c0c5cbf6cae 100644
--- a/include/unotest/macros_test.hxx
+++ b/include/unotest/macros_test.hxx
@@ -26,6 +26,11 @@ struct TestMacroInfo
 };
 
 class BasicDLL;
+class SvStream;
+namespace utl
+{
+class TempFile;
+}
 
 namespace unotest {
 
@@ -43,6 +48,10 @@ public:
                     const OUString& rCommand,
                     const css::uno::Sequence<css::beans::PropertyValue>& rPropertyValues);
 
+    /// Opens rStreamName from rTempFile, assuming it's a ZIP storage.
+    static std::unique_ptr<SvStream> parseExportStream(const utl::TempFile& rTempFile,
+                                                       const OUString& rStreamName);
+
 protected:
     css::uno::Reference< css::frame::XDesktop2> mxDesktop;
 
diff --git a/sw/inc/numrule.hxx b/sw/inc/numrule.hxx
index 6152e6bee99f..b0bf138afb33 100644
--- a/sw/inc/numrule.hxx
+++ b/sw/inc/numrule.hxx
@@ -270,6 +270,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 1de08ae9f761..b812897a9ae1 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -1099,6 +1099,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.
+    sal_uLong 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 3fef41218cad..45fbd9c31e41 100644
--- a/sw/source/core/unocore/unoparagraph.cxx
+++ b/sw/source/core/unocore/unoparagraph.cxx
@@ -930,6 +930,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/unotest/Library_unotest.mk b/unotest/Library_unotest.mk
index ea697699ee82..3acfe2f69ed9 100644
--- a/unotest/Library_unotest.mk
+++ b/unotest/Library_unotest.mk
@@ -22,6 +22,7 @@ $(eval $(call gb_Library_use_libraries,unotest,\
 	cppuhelper \
 	sal \
 	sb \
+	utl \
 ))
 
 $(eval $(call gb_Library_use_externals,unotest,\
diff --git a/unotest/source/cpp/macros_test.cxx b/unotest/source/cpp/macros_test.cxx
index 20c56950fcaf..37b84cfab8c9 100644
--- a/unotest/source/cpp/macros_test.cxx
+++ b/unotest/source/cpp/macros_test.cxx
@@ -14,11 +14,14 @@
 #include <com/sun/star/document/MacroExecMode.hpp>
 #include <com/sun/star/uno/XComponentContext.hpp>
 #include <com/sun/star/frame/DispatchHelper.hpp>
+#include <com/sun/star/packages/zip/ZipFileAccess.hpp>
 
 #include <basic/basrdll.hxx>
 #include <cppunit/TestAssert.h>
 #include <comphelper/sequence.hxx>
 #include <comphelper/processfactory.hxx>
+#include <unotools/tempfile.hxx>
+#include <unotools/ucbstreamhelper.hxx>
 
 using namespace css;
 
@@ -76,6 +79,20 @@ void MacrosTest::dispatchCommand(const uno::Reference<lang::XComponent>& xCompon
 
     xDispatchHelper->executeDispatch(xFrame, rCommand, OUString(), 0, rPropertyValues);
 }
+
+std::unique_ptr<SvStream> MacrosTest::parseExportStream(const utl::TempFile& rTempFile,
+                                                        const OUString& rStreamName)
+{
+    const OUString aUrl = rTempFile.GetURL();
+    uno::Reference<uno::XComponentContext> xComponentContext
+        = comphelper::getProcessComponentContext();
+    uno::Reference<packages::zip::XZipFileAccess2> const xZipNames(
+        packages::zip::ZipFileAccess::createWithURL(xComponentContext, aUrl));
+    uno::Reference<io::XInputStream> const xInputStream(xZipNames->getByName(rStreamName),
+                                                        uno::UNO_QUERY);
+    std::unique_ptr<SvStream> pStream(utl::UcbStreamHelper::CreateStream(xInputStream, true));
+    return pStream;
+}
 }
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/xmloff/CppunitTest_xmloff_text.mk b/xmloff/CppunitTest_xmloff_text.mk
index e3259672605b..a61f381f11a0 100644
--- a/xmloff/CppunitTest_xmloff_text.mk
+++ b/xmloff/CppunitTest_xmloff_text.mk
@@ -13,6 +13,7 @@ $(eval $(call gb_CppunitTest_CppunitTest,xmloff_text))
 
 $(eval $(call gb_CppunitTest_use_externals,xmloff_text,\
 	boost_headers \
+	libxml2 \
 ))
 
 $(eval $(call gb_CppunitTest_add_exception_objects,xmloff_text, \
@@ -26,6 +27,7 @@ $(eval $(call gb_CppunitTest_use_libraries,xmloff_text, \
     sal \
     test \
     unotest \
+    utl \
 ))
 
 $(eval $(call gb_CppunitTest_use_sdk_api,xmloff_text))
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 2a5c65e5c2d9..9bd802e813d5 100644
--- a/xmloff/qa/unit/text.cxx
+++ b/xmloff/qa/unit/text.cxx
@@ -9,16 +9,23 @@
 
 #include <test/bootstrapfixture.hxx>
 #include <unotest/macros_test.hxx>
+#include <test/xmltesttools.hxx>
 
 #include <com/sun/star/frame/Desktop.hpp>
+#include <com/sun/star/frame/XStorable.hpp>
 #include <com/sun/star/container/XNameContainer.hpp>
 
+#include <comphelper/propertysequence.hxx>
+#include <unotools/tempfile.hxx>
+
 using namespace ::com::sun::star;
 
 char const DATA_DIRECTORY[] = "/xmloff/qa/unit/data/";
 
 /// Covers xmloff/source/text/ fixes.
-class XmloffStyleTest : public test::BootstrapFixture, public unotest::MacrosTest
+class XmloffStyleTest : public test::BootstrapFixture,
+                        public unotest::MacrosTest,
+                        public XmlTestTools
 {
 private:
     uno::Reference<lang::XComponent> mxComponent;
@@ -26,9 +33,15 @@ private:
 public:
     void setUp() override;
     void tearDown() override;
+    void registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx) override;
     uno::Reference<lang::XComponent>& getComponent() { return mxComponent; }
 };
 
+void XmloffStyleTest::registerNamespaces(xmlXPathContextPtr& pXmlXpathCtx)
+{
+    XmlTestTools::registerODFNamespaces(pXmlXpathCtx);
+}
+
 void XmloffStyleTest::setUp()
 {
     test::BootstrapFixture::setUp();
@@ -53,6 +66,30 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testMailMergeInEditeng)
     CPPUNIT_ASSERT(getComponent().is());
 }
 
+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 b982d4d6dd4b..6330822e466f 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>
@@ -40,6 +41,7 @@ XMLTextNumRuleInfo::XMLTextNumRuleInfo()
     : mxNumRules()
     , msNumRulesName()
     , msListId()
+    , mbListIdIsDefault(false)
     , mnListStartValue( -1 )
     , mnListLevel( 0 )
     , mbIsNumbered( false )
@@ -62,6 +64,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
@@ -138,6 +141,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 28555f88af26..ff0f45745dc3 100644
--- a/xmloff/source/text/XMLTextNumRuleInfo.hxx
+++ b/xmloff/source/text/XMLTextNumRuleInfo.hxx
@@ -44,6 +44,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;
@@ -84,6 +86,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 d0c067ac7ed3..c5204c9f4504 100644
--- a/xmloff/source/text/txtparae.cxx
+++ b/xmloff/source/text/txtparae.cxx
@@ -965,7 +965,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#)
@@ -984,7 +984,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#)