tdf#141964 writerfilter CN: numId0 has different meaning for RTF

For DOCX, numId means turn off numbering.

RTF probably doesn't have numbering as part of paragraph styles,
so it doesn't need a numId 0 to cancel existing numbering.
This numbering is applied directly to the paragraph during import.
So, for RTF, numId 0 needs to be treated just like any other numId
(except for -1 through -5 although currently
we only import positive numbers).

The statement "if pList and numId > 0" is basically redundant.
pList will not be exist for an invalid numId.
Since numId can be zero for RTF, just get rid of the clause
to avoid any misleading conclusions.

And that is all that this patch is doing. It doesn't really fix
anything, it just changes code that gives out the wrong impression.
AFAICT, RTF never uses styles, and therefore it is always
isNumberingViaRule and so is almost untouched by finishParagraph.

Change-Id: Ibc89d00ce69ad09208254ed85710f1f1aed18074
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/118910
Tested-by: Jenkins
Reviewed-by: Justin Luth <justin_luth@sil.org>

4 files changed, 55 insertions, 5 deletions

diff --git a/sw/qa/extras/rtfexport/data/tdf141964_numId0.rtf b/sw/qa/extras/rtfexport/data/tdf141964_numId0.rtf
new file mode 100644
index 000000000000..aa79b2618c19
--- /dev/null
+++ b/sw/qa/extras/rtfexport/data/tdf141964_numId0.rtf
@@ -0,0 +1,42 @@
+{\rtf1\ansi\deff0\adeflang1025

+{\fonttbl{\f0\froman\fprq2\fcharset0 Times New Roman;}{\f1\froman\fprq2\fcharset0 Times New Roman;}{\f2\fswiss\fprq2\fcharset0 Arial;}{\f3\froman\fprq2\fcharset0 Times New Roman;}{\f4\fnil\fprq0\fcharset2 StarSymbol{\*\falt Arial Unicode MS};}{\f5\fnil\fprq2\fcharset0 Arial Unicode MS;}{\f6\fnil\fprq2\fcharset0 MS Mincho;}{\f7\fnil\fprq2\fcharset0 Tahoma;}{\f8\fnil\fprq0\fcharset0 Tahoma;}}

+{\colortbl;\red0\green0\blue0;\red0\green0\blue128;\red255\green255\blue255;\red227\green227\blue227;\red128\green128\blue128;}

+{\stylesheet{\s1\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031\snext1 Normal;}

+{\s2\sa120\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031\sbasedon1\snext2 Body Text;}

+{\s3\sa120\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af8\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031\sbasedon2\snext3 List;}

+{\s4\sb120\sa120\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af8\afs20\lang255\ai\ltrch\dbch\af5\langfe255\hich\f0\fs20\lang1031\i\loch\f0\fs20\lang1031\i\sbasedon1\snext4 caption;}

+{\s5\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af8\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031\sbasedon1\snext5 Index;}

+{\s6\sb240\sa283\keepn\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\afs28\lang255\ltrch\dbch\af6\langfe255\hich\f2\fs28\lang1031\loch\f2\fs28\lang1031\sbasedon1\snext2 Heading;}

+{\s7\sb240\sa283\keepn\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\afs48\lang255\ab\ltrch\dbch\af5\langfe255\hich\f0\fs48\lang1031\b\loch\f0\fs48\lang1031\b\sbasedon6\snext2{\*\soutlvl0} heading 1;}

+{\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031\sbasedon1\snext8 Table Contents;}

+{\s9\cf0\qc{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af7\afs24\lang255\ab\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\b\loch\f0\fs24\lang1031\b\sbasedon8\snext9 Table Heading;}

+{\s10\sb240\sa283\keepn\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\afs36\lang255\ab\ltrch\dbch\af5\langfe255\hich\f0\fs36\lang1031\b\loch\f0\fs36\lang1031\b\sbasedon6\snext2{\*\soutlvl1} heading 2;}

+{\*\cs12\cf2\ul\ulc0\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031 Internet link;}

+{\*\cs13\cf0\rtlch\af4\afs18\lang255\ltrch\dbch\af4\langfe255\hich\f4\fs18\lang1031\loch\f4\fs18\lang1031 Bullet Symbols;}

+}{\*\listtable{\list\listtemplateid1

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li707}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li1414}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li2121}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li2828}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li3535}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li4242}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li4949}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li5656}

+{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li6363}

+{\*\soutlvl{\listlevel\levelnfc23\leveljc0\levelstartat1\levelfollow0{\leveltext \'01\u8226 ?;}{\levelnumbers;}\f4\fs18\f4\fs18\f4\fs18\f4\fi-283\li7070}}\listid1}

+}{\listoverridetable{\listoverride\listid1\listoverridecount0\ls0}}

+

+{\info{\author Rainer Bielefeld}{\creatim\yr2005\mo12\dy1\hr8\min13}{\revtim\yr1601\mo1\dy1\hr0\min0}{\printim\yr1601\mo1\dy1\hr0\min0}{\comment StarWriter}{\vern6800}}\deftab720

+{\*\pgdsctbl

+{\pgdsc0\pgdscuse195\pgwsxn11905\pghsxn16837\marglsxn1134\margrsxn1134\margtsxn1134\margbsxn1134\pgdscnxt0 Standard;}}

+\paperh16837\paperw11905\margl1134\margr1134\margt1134\margb1134

+

+\pard\pard\plain \sb240\sa283\keepn\fs36\b\fs36\b\fs36\b \ltrpar\s10\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\sb240\sa283\keepn\rtlch\afs36\lang255\ab\ltrch\dbch\af5\langfe255\hich\f0\fs36\lang1031\b\loch\f0\fs36\lang1031\b {\rtlch \ltrch\loch\f0\fs36\lang1031\i0\b Inhaltsverzeichnis}

+\par \pard\plain \ltrpar\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031 {\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 [{\field{\*\fldinst HYPERLINK "javascript:toggleToc()" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031 {\*\bkmkstart togglelink}{\*\bkmkend togglelink}Verbergen}}]}

+\par \ls0\li707\ri0\lin707\rin0\fi-283\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\field{\*\fldinst HYPERLINK "http://de.wikipedia.org/wiki/Hohenpolding" \\l "eschichte" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\loch{\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 Geschichte}}}

+\par \pard\plain {\listtext\pard\plain \li707\ri0\lin707\rin0\fi-283\f4\fs18\f4\fs18\f4\fs18 \u8226\'2d\tab}\ilvl0 \ltrpar\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\ls0\li707\ri0\lin707\rin0\fi-283\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\field{\*\fldinst HYPERLINK "http://de.wikipedia.org/wiki/Hohenpolding" \\l "appen_und_Fahne" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 Wappen und Fahne}} }

+\par \pard\plain {\listtext\pard\plain \li707\ri0\lin707\rin0\fi-283\f4\fs18\f4\fs18\f4\fs18 \u8226\'2d\tab}\ilvl0 \ltrpar\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\ls0\li707\ri0\lin707\rin0\fi-283\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\field{\*\fldinst HYPERLINK "http://de.wikipedia.org/wiki/Hohenpolding" \\l "ehensw.C3.BCrdigkeiten" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 Sehensw\'fcrdigkeiten}} }

+\par \pard\plain {\listtext\pard\plain \li707\ri0\lin707\rin0\fi-283\f4\fs18\f4\fs18\f4\fs18 \u8226\'2d\tab}\ilvl0 \ltrpar\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\ls0\li707\ri0\lin707\rin0\fi-283\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\field{\*\fldinst HYPERLINK "http://de.wikipedia.org/wiki/Hohenpolding" \\l "hrenb.C3.BCrger" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 Ehrenb\'fcrger}} }

+\par \pard\plain {\listtext\pard\plain \li707\ri0\lin707\rin0\fi-283\sa283\f4\fs18\f4\fs18\f4\fs18 \u8226\'2d\tab}\ilvl0 \ltrpar\s8\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\ls0\li707\ri0\lin707\rin0\fi-283\sa283\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\field{\*\fldinst HYPERLINK "http://de.wikipedia.org/wiki/Hohenpolding" \\l "eblinks" }{\fldrslt \*\cs12\cf2\ul\ulc0\rtlch\ltrch\dbch\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031{\rtlch \ltrch\loch\f0\fs24\lang1031\i0\b0 Weblinks}} }

+\pard\plain \ltrpar\s2\cf0{\*\hyphen2\hyphlead2\hyphtrail2\hyphmax0}\li68\ri0\lin68\rin0\fi0\sa120\rtlch\af7\afs24\lang255\ltrch\dbch\af5\langfe255\hich\f0\fs24\lang1031\loch\f0\fs24\lang1031 

+\par}

diff --git a/sw/qa/extras/rtfexport/rtfexport3.cxx b/sw/qa/extras/rtfexport/rtfexport3.cxx
index efbb7c806477..90982b870c83 100644
--- a/sw/qa/extras/rtfexport/rtfexport3.cxx
+++ b/sw/qa/extras/rtfexport/rtfexport3.cxx
@@ -63,6 +63,14 @@ DECLARE_RTFEXPORT_TEST(testTdf108949, "tdf108949_footnoteCharFormat.odt")
                                    getRun(getParagraphOfText(1, xFootnoteText), 1), "CharColor"));
 }
 
+DECLARE_RTFEXPORT_TEST(testTdf141964_numId0, "tdf141964_numId0.rtf")
+{
+    // Unit test added where numId is zero - which is only possible in RTF.
+    uno::Reference<beans::XPropertySet> xPara(getParagraph(3, "Geschichte"), uno::UNO_QUERY);
+    // Pre-emptive test: ensure that paragraph 3 remains numbered and numId0 doesn't mean no numbering.
+    CPPUNIT_ASSERT(!getProperty<OUString>(xPara, "NumberingStyleName").isEmpty());
+}
+
 DECLARE_RTFEXPORT_TEST(testTdf108949_footnote, "tdf108949_footnote.rtf")
 {
     CPPUNIT_ASSERT_EQUAL_MESSAGE("Paragraph Numbering style", OUString(),
diff --git a/writerfilter/source/dmapper/DomainMapper_Impl.cxx b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
index ae10911d5eaa..78f7dbc16b24 100644
--- a/writerfilter/source/dmapper/DomainMapper_Impl.cxx
+++ b/writerfilter/source/dmapper/DomainMapper_Impl.cxx
@@ -1584,16 +1584,16 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
         //does not specify the numbering
         sal_Int16 nListLevel = GetListLevel(pEntry, pParaContext);
         // Undefined listLevel with a valid numId is treated as a first level numbering.
-        if (nListLevel == -1 && nListId > 0)
+        if (nListLevel == -1 && nListId > (IsOOXMLImport() ? 0 : -1))
             nListLevel = 0;
 
         if (!bNoNumbering && nListLevel >= 0 && nListLevel < 9)
             pParaContext->Insert( PROP_NUMBERING_LEVEL, uno::makeAny(nListLevel), false );
 
         auto const pList(GetListTable()->GetList(nListId));
-        if (pList && nListId > 0 && !pParaContext->isSet(PROP_NUMBERING_STYLE_NAME))
+        if (pList && !pParaContext->isSet(PROP_NUMBERING_STYLE_NAME))
         {
-            // ListLevel 9 means Body Level/no numbering.  numId 0 means no numbering.
+            // ListLevel 9 means Body Level/no numbering.
             if (bNoNumbering || nListLevel == 9)
                 pParaContext->Insert(PROP_NUMBERING_STYLE_NAME, uno::makeAny(OUString()), true);
             else if ( !isNumberingViaRule )
@@ -1672,7 +1672,7 @@ void DomainMapper_Impl::finishParagraph( const PropertyMapPtr& pPropertyMap, con
 
         if (nListId == 0 && !pList)
         {
-            // Seems situation with listid=0 and missing list definition is used by MS Word
+            // Seems situation with listid=0 and missing list definition is used by DOCX
             // to remove numbering defined previously. But some default numbering attributes
             // are still applied. This is first line indent, probably something more?
             if (!pParaContext->isSet(PROP_PARA_FIRST_LINE_INDENT))
diff --git a/writerfilter/source/dmapper/NumberingManager.cxx b/writerfilter/source/dmapper/NumberingManager.cxx
index f312d7080dcb..1d4c70f613a1 100644
--- a/writerfilter/source/dmapper/NumberingManager.cxx
+++ b/writerfilter/source/dmapper/NumberingManager.cxx
@@ -1177,7 +1177,7 @@ ListDef::Pointer ListsManager::GetList( sal_Int32 nId )
     }
 
     // nId 0 is only valid for abstractNum, not numId (which has an abstract definition)
-    assert(nId || !pList || !pList->GetAbstractDefinition());
+    assert(!pList || nId || !pList->GetAbstractDefinition() || m_rDMapper.IsRTFImport());
 
     return pList;
 }