tdf#120394: DOCX list import: simplify zero width space hack

Since introducion of list format string hack with creation
of zero-width-space can be much more simple. It was being
used to indicate existing, but empty list label suffix to
avoid stripping down numbering.

Change-Id: I9a0c6047f806b2c656ef5dbab0c6b38200818bd2
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94383
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <Thorsten.Behrens@CIB.de>

6 files changed, 56 insertions, 55 deletions

diff --git a/sw/qa/extras/ooxmlexport/data/tdf120394.docx b/sw/qa/extras/ooxmlexport/data/tdf120394.docx
new file mode 100644
index 000000000000..39bd5886c0fe
--- /dev/null
+++ b/sw/qa/extras/ooxmlexport/data/tdf120394.docx
diff --git a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
index b372f2e86dcd..b95d0afe4e8f 100644
--- a/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
+++ b/sw/qa/extras/ooxmlexport/ooxmlexport14.cxx
@@ -887,6 +887,31 @@ DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf128290, "tdf128290.odt")
     assertXPath(pXml, "/w:document/w:body/w:tbl/w:tblPr/w:tblLayout", "type", "fixed");
 }
 
+DECLARE_OOXMLEXPORT_TEST(testTdf120394, "tdf120394.docx")
+{
+    CPPUNIT_ASSERT_EQUAL(1, getPages());
+    {
+        uno::Reference<beans::XPropertySet> xPara(getParagraph(1), uno::UNO_QUERY);
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(0), getProperty<sal_Int16>(xPara, "NumberingLevel"));
+        CPPUNIT_ASSERT_EQUAL(OUString("1"), getProperty<OUString>(xPara, "ListLabelString"));
+    }
+    {
+        uno::Reference<beans::XPropertySet> xPara(getParagraph(2), uno::UNO_QUERY);
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(1), getProperty<sal_Int16>(xPara, "NumberingLevel"));
+        CPPUNIT_ASSERT_EQUAL(OUString(CHAR_ZWSP), getProperty<OUString>(xPara, "ListLabelString"));
+    }
+    {
+        uno::Reference<beans::XPropertySet> xPara(getParagraph(3), uno::UNO_QUERY);
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(1), getProperty<sal_Int16>(xPara, "NumberingLevel"));
+        CPPUNIT_ASSERT_EQUAL(OUString(CHAR_ZWSP), getProperty<OUString>(xPara, "ListLabelString"));
+    }
+    {
+        uno::Reference<beans::XPropertySet> xPara(getParagraph(5), uno::UNO_QUERY);
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int16>(2), getProperty<sal_Int16>(xPara, "NumberingLevel"));
+        CPPUNIT_ASSERT_EQUAL(OUString("1.2.1"), getProperty<OUString>(xPara, "ListLabelString"));
+    }
+}
+
 DECLARE_OOXMLEXPORT_EXPORTONLY_TEST(testTdf123757, "tdf123757.docx")
 {
     xmlDocUniquePtr pXml = parseExport("word/document.xml");
diff --git a/sw/qa/extras/rtfimport/rtfimport.cxx b/sw/qa/extras/rtfimport/rtfimport.cxx
index 085a71aedc86..cf9d5dd31309 100644
--- a/sw/qa/extras/rtfimport/rtfimport.cxx
+++ b/sw/qa/extras/rtfimport/rtfimport.cxx
@@ -343,8 +343,7 @@ CPPUNIT_TEST_FIXTURE(Test, testFdo49692)
 
         if (rProp.Name == "Suffix")
         {
-            OUString aExpected(u'\x200B');
-            CPPUNIT_ASSERT_EQUAL(aExpected, rProp.Value.get<OUString>());
+            CPPUNIT_ASSERT(rProp.Value.get<OUString>().isEmpty());
         }
     }
 }
@@ -1358,7 +1357,7 @@ CPPUNIT_TEST_FIXTURE(Test, testTdf78506)
 
         if (rProp.Name == "Suffix")
             // This was '0', invalid \levelnumbers wasn't ignored.
-            CPPUNIT_ASSERT_EQUAL(CHAR_ZWSP, rProp.Value.get<OUString>().toChar());
+            CPPUNIT_ASSERT(rProp.Value.get<OUString>().isEmpty());
     }
 }
 
diff --git a/sw/source/core/doc/number.cxx b/sw/source/core/doc/number.cxx
index 02c2512e7872..1d2ae4f970d4 100644
--- a/sw/source/core/doc/number.cxx
+++ b/sw/source/core/doc/number.cxx
@@ -667,6 +667,13 @@ OUString SwNumRule::MakeNumString( const SwNumberTree::tNumberVector & rNumVecto
                     if (nPosition >= 0)
                         sLevelFormat = sLevelFormat.replaceAt(nPosition, sFind.getLength(), sReplacement);
                 }
+
+                // As a fallback: caller code expects nonempty string as a result.
+                // But if we have empty string (and had no errors before) this is valid result.
+                // So use classical hack with zero-width-space as a string filling.
+                if (sLevelFormat.isEmpty())
+                    sLevelFormat = OUStringChar(CHAR_ZWSP);
+
                 aStr = sLevelFormat;
             }
             else
diff --git a/sw/source/filter/ww8/wrtw8num.cxx b/sw/source/filter/ww8/wrtw8num.cxx
index e2aad33206d3..1774c37239e7 100644
--- a/sw/source/filter/ww8/wrtw8num.cxx
+++ b/sw/source/filter/ww8/wrtw8num.cxx
@@ -494,12 +494,7 @@ void MSWordExportBase::NumberingLevel(
     const vcl::Font* pBulletFont=nullptr;
     rtl_TextEncoding eChrSet=0;
     FontFamily eFamily=FAMILY_DECORATIVE;
-    if (rRule.Get(nLvl).HasListFormat())
-    {
-        // Nothing to construct: we have it already
-        sNumStr = rRule.Get(nLvl).GetListFormat();
-    }
-    else if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() ||
+    if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() ||
         SVX_NUM_BITMAP == rFormat.GetNumberingType())
     {
         // Use bullet
@@ -507,7 +502,9 @@ void MSWordExportBase::NumberingLevel(
     }
     else
     {
-        // Construct list format string from prefix, level numbers and suffix
+        // Create level string
+        // For docx it is not the best way: we can just take it from rRule.Get(nLvl).GetListFormat()
+        // But for compatibility with doc we follow same routine
         if (SVX_NUM_NUMBER_NONE != rFormat.GetNumberingType())
         {
             sal_uInt8* pLvlPos = aNumLvlPos;
@@ -518,20 +515,23 @@ void MSWordExportBase::NumberingLevel(
             // now search the nums in the string
             for (sal_uInt8 i = 0; i <= nLvl; ++i)
             {
-                OUString sSrch( OUString::number( i ));
-                sal_Int32 nFnd = sNumStr.indexOf( sSrch );
-                if( -1 != nFnd )
+                OUString sSrch(OUString::number(i));
+                sal_Int32 nFnd = sNumStr.indexOf(sSrch);
+                if (-1 != nFnd)
                 {
-                    *pLvlPos = static_cast<sal_uInt8>(nFnd + rFormat.GetPrefix().getLength() + 1 );
+                    *pLvlPos = static_cast<sal_uInt8>(nFnd + rFormat.GetPrefix().getLength() + 1);
                     ++pLvlPos;
-                    sNumStr = sNumStr.replaceAt( nFnd, 1, OUString(static_cast<char>(i)) );
+                    sNumStr = sNumStr.replaceAt(nFnd, 1, OUString(static_cast<char>(i)));
                 }
             }
         }
 
-        if (!rFormat.GetPrefix().isEmpty())
-            sNumStr = rFormat.GetPrefix() + sNumStr;
-        sNumStr += rFormat.GetSuffix();
+        if (!rRule.Get(nLvl).HasListFormat())
+        {
+            if (!rFormat.GetPrefix().isEmpty())
+                sNumStr = rFormat.GetPrefix() + sNumStr;
+            sNumStr += rFormat.GetSuffix();
+        }
     }
 
     if (SVX_NUM_CHAR_SPECIAL == rFormat.GetNumberingType() ||
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx
index 59fb4f8c2123..047d29a065ca 100644
--- a/writerfilter/source/dmapper/NumberingManager.cxx
+++ b/writerfilter/source/dmapper/NumberingManager.cxx
@@ -207,10 +207,6 @@ uno::Sequence<beans::PropertyValue> ListLevel::GetLevelProperties(bool bDefaults
     {
         if (m_xGraphicBitmap.is())
             nNumberFormat = style::NumberingType::BITMAP;
-        else if (m_sBulletChar.isEmpty() && nNumberFormat != style::NumberingType::CHAR_SPECIAL)
-            // w:lvlText is empty, that means no numbering in Word.
-            // CHAR_SPECIAL is handled separately below.
-            nNumberFormat = style::NumberingType::NUMBER_NONE;
         aNumberingProperties.push_back(lcl_makePropVal(PROP_NUMBERING_TYPE, nNumberFormat));
     }
 
@@ -548,40 +544,14 @@ void ListDef::CreateNumberingRules( DomainMapper& rDMapper,
                 if (pLevel && !pLevel->GetBulletChar().isEmpty())
                     sText = pLevel->GetBulletChar( );
 
-                if (sText.isEmpty())
-                {
-                    // Empty <w:lvlText>? Then put a Unicode "zero width space" as a suffix, so LabelFollowedBy is still shown, as in Word.
-                    // With empty suffix, Writer does not show LabelFollowedBy, either.
-                    OUString sSuffix;
-                    auto it = std::find_if(aLvlProps.begin(), aLvlProps.end(), [](const beans::PropertyValue& rValue) { return rValue.Name == "NumberingType"; });
-                    if (it != aLvlProps.end())
-                    {
-                        sal_Int16 nNumberFormat = it->Value.get<sal_Int16>();
-
-                        // No need for a zero width space without a real LabelFollowedBy.
-                        bool bLabelFollowedBy = true;
-                        it = std::find_if(aLvlProps.begin(), aLvlProps.end(), [](const beans::PropertyValue& rValue) { return rValue.Name == "LabelFollowedBy"; });
-                        if (it != aLvlProps.end())
-                        {
-                            sal_Int16 nValue;
-                            if (it->Value >>= nValue)
-                                bLabelFollowedBy = nValue != SvxNumberFormat::NOTHING;
-                        }
+                aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PREFIX), OUString("")));
+                aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_SUFFIX), OUString("")));
+                aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_LIST_FORMAT), sText));
 
-                        if (bLabelFollowedBy && nNumberFormat == style::NumberingType::NUMBER_NONE)
-                            sSuffix = OUString(u'\x200B');
-                    }
-                    aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_SUFFIX), sSuffix));
-                }
-                else
-                {
-                    aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_LIST_FORMAT), sText));
-
-                    // Total count of replacement holders is determining amount of required parent numbering to include
-                    // TODO: not sure how "%" symbol is escaped. This is not supported yet
-                    sal_Int16 nParentNum = comphelper::string::getTokenCount(sText, '%');
-                    aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PARENT_NUMBERING), nParentNum));
-                }
+                // Total count of replacement holders is determining amount of required parent numbering to include
+                // TODO: not sure how "%" symbol is escaped. This is not supported yet
+                sal_Int16 nParentNum = comphelper::string::getTokenCount(sText, '%');
+                aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_PARENT_NUMBERING), nParentNum));
 
                 aLvlProps.push_back(comphelper::makePropertyValue(getPropertyName(PROP_POSITION_AND_SPACE_MODE), sal_Int16(text::PositionAndSpaceMode::LABEL_ALIGNMENT)));