sw/qa/extras/htmlexport/data/sub_li_and_ctd.fodt |   33 ++++++++++++++++
 sw/qa/extras/htmlexport/htmlexport.cxx           |   34 +++++++++++++++++
 sw/source/filter/html/htmlatr.cxx                |    2 -
 sw/source/filter/html/htmlnum.cxx                |   46 +++++++++++++++++++++++
 sw/source/filter/html/htmlnum.hxx                |    6 +--
 sw/source/filter/html/htmlnumwriter.cxx          |   19 +++++----
 sw/source/filter/html/htmltabw.cxx               |   10 ++---
 7 files changed, 132 insertions(+), 18 deletions(-)

New commits:
commit 78eedc1a1f73f06fca1384f848f2d6b7b89787cb
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu May 18 15:56:46 2023 +0300
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Thu May 18 17:48:45 2023 +0200

    tdf#155387: fix list restart and opening/closing conditions
    
    1. List restart flag in the node does not yet mean that the restart
    would actually happen: it will be ignored, if the next node is more
    nested than the previous one.
    
    2. When writing nested list items, we should not write additional
    <li><ul> pairs for current level (previously, only level 0 was
    skipped); same for closing.
    
    Change-Id: I5f67e8fa9236e7e2e6dba2e0ec5dffbf0d7b7f8b
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151958
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>

diff --git a/sw/qa/extras/htmlexport/data/sub_li_and_ctd.fodt 
b/sw/qa/extras/htmlexport/data/sub_li_and_ctd.fodt
new file mode 100644
index 000000000000..b4a3977926c6
--- /dev/null
+++ b/sw/qa/extras/htmlexport/data/sub_li_and_ctd.fodt
@@ -0,0 +1,33 @@
+<?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:list-item>
+     <text:p>l1</text:p>
+     <text:p>l1_ctd1</text:p>
+     <text:list>
+      <text:list-item>
+       <text:p>l2</text:p>
+      </text:list-item>
+      <text:list-item>
+       <text:p>l2</text:p>
+      </text:list-item>
+     </text:list>
+     <text:p>l1_ctd2</text:p>
+     <text:list>
+      <text:list-item>
+       <text:list>
+        <text:list-item>
+         <text:p>l3</text:p>
+        </text:list-item>
+       </text:list>
+      </text:list-item>
+     </text:list>
+     <text:p>l1_ctd3</text:p>
+    </text:list-item>
+   </text:list>
+  </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/sw/qa/extras/htmlexport/htmlexport.cxx 
b/sw/qa/extras/htmlexport/htmlexport.cxx
index 18929a3626f9..7121b9f7021f 100644
--- a/sw/qa/extras/htmlexport/htmlexport.cxx
+++ b/sw/qa/extras/htmlexport/htmlexport.cxx
@@ -2571,6 +2571,40 @@ CPPUNIT_TEST_FIXTURE(SwHtmlDomExportTest, 
testReqIfTransparentTifImg)
     CPPUNIT_ASSERT_EQUAL(GraphicFileFormat::TIF, aDescriptor.GetFileFormat());
 }
 
+CPPUNIT_TEST_FIXTURE(SwHtmlDomExportTest, testTdf155387)
+{
+    createSwDoc("sub_li_and_ctd.fodt");
+    ExportToReqif();
+
+    SvMemoryStream aStream;
+    WrapReqifFromTempFile(aStream);
+    xmlDocUniquePtr pDoc = parseXmlStream(&aStream);
+    // Without the fix in place, this would fail
+    CPPUNIT_ASSERT(pDoc);
+
+    // Single top-level list
+    assertXPath(pDoc, "/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul");
+    // Single top-level item
+    assertXPath(pDoc, 
"/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul/reqif-xhtml:li");
+    // 4 top-level paragraphs in the item
+    assertXPath(pDoc,
+                
"/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul/reqif-xhtml:li/reqif-xhtml:p",
 4);
+    // 2 sublists in the item
+    assertXPath(
+        pDoc, 
"/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul/reqif-xhtml:li/reqif-xhtml:ul",
 2);
+    // 2 items in the first sublist
+    assertXPath(pDoc,
+                
"/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul/reqif-xhtml:li/reqif-xhtml:ul[1]/"
+                "reqif-xhtml:li",
+                2);
+    // Check the last (most nested) subitem's text
+    assertXPathContent(
+        pDoc,
+        
"/reqif-xhtml:html/reqif-xhtml:div/reqif-xhtml:ul/reqif-xhtml:li/reqif-xhtml:ul[2]/"
+        "reqif-xhtml:li/reqif-xhtml:ul/reqif-xhtml:li/reqif-xhtml:p",
+        "l3");
+}
+
 CPPUNIT_PLUGIN_IMPLEMENT();
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/html/htmlatr.cxx 
b/sw/source/filter/html/htmlatr.cxx
index 84d2feb1ad5b..37b460eb0869 100644
--- a/sw/source/filter/html/htmlatr.cxx
+++ b/sw/source/filter/html/htmlatr.cxx
@@ -986,7 +986,7 @@ static void OutHTML_SwFormatOff( SwHTMLWriter& rWrt, const 
SwHTMLTextCollOutputI
             const SwHTMLNumRuleInfo& rNRInfo = rWrt.GetNumInfo();
             if( rNextInfo.GetNumRule() != rNRInfo.GetNumRule() ||
                 rNextInfo.GetDepth() != rNRInfo.GetDepth() ||
-                rNextInfo.IsNumbered() || rNextInfo.IsRestart() )
+                rNextInfo.IsNumbered() || rNextInfo.IsRestart(rNRInfo) )
                 rWrt.ChangeParaToken( HtmlTokenId::NONE );
             OutHTML_NumberBulletListEnd( rWrt, rNextInfo );
         }
diff --git a/sw/source/filter/html/htmlnum.cxx 
b/sw/source/filter/html/htmlnum.cxx
index bd8317bb5676..8fa120a630cf 100644
--- a/sw/source/filter/html/htmlnum.cxx
+++ b/sw/source/filter/html/htmlnum.cxx
@@ -44,4 +44,50 @@ void SwHTMLNumRuleInfo::Set(const SwTextNode& rTextNd)
     }
 }
 
+// Restart flag is only effective when this level is not below the previous
+bool SwHTMLNumRuleInfo::IsRestart(const SwHTMLNumRuleInfo& rPrev) const
+{
+    // calling this, when the rules are different, makes no sense
+    assert(rPrev.GetNumRule() == GetNumRule());
+
+    // An example ODF when the restart flag is set, but has no effect:
+    //   <text:list text:style-name="L1">
+    //    <text:list-item>
+    //     <text:p>l1</text:p>
+    //     <text:list>
+    //      <text:list-item>
+    //       <text:p>l2</text:p>
+    //      </text:list-item>
+    //      <text:list-item>
+    //       <text:p>l2</text:p>
+    //      </text:list-item>
+    //     </text:list>
+    //     <text:list>
+    //      <text:list-item>
+    //       <text:list>
+    //        <text:list-item>
+    //         <text:p>l3</text:p>
+    //        </text:list-item>
+    //       </text:list>
+    //      </text:list-item>
+    //     </text:list>
+    //    </text:list-item>
+    //   </text:list>
+    // In this case, "l3" is in a separate sublist than "l2", and so the "l3" 
node gets the
+    // "list restart" property. But the document rendering would be
+    //   1. l1
+    //        1.1. l2
+    //        1.2. l2
+    //             1.2.1. l3
+    // and the second-level numbering will not actually restart at the "l3" 
node.
+    //
+    // TODO/LATER: note that restarting may happen at different levels. In the 
code using this
+    // function, the level is reset to 0 whenever a restart is detected. And 
also, there is no
+    // code to actually descend to that new level (close corresponding 
li/ul/ol elements).
+
+    if (rPrev.GetDepth() < GetDepth())
+        return false; // No matter if the restart flag is set, it is not 
effective for subitems
+    return m_bRestart;
+}
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sw/source/filter/html/htmlnum.hxx 
b/sw/source/filter/html/htmlnum.hxx
index b73fd386e3f3..670f08229df2 100644
--- a/sw/source/filter/html/htmlnum.hxx
+++ b/sw/source/filter/html/htmlnum.hxx
@@ -38,8 +38,8 @@ class SwHTMLNumRuleInfo
     sal_uInt16      m_aNumStarts[MAXLEVEL];
     SwNumRule   *   m_pNumRule;       // current numbering
     sal_uInt16      m_nDeep;          // current numbering depth (1, 2, 3, ...)
-    bool        m_bRestart : 1;   // Export: restart numbering
-    bool        m_bNumbered : 1;  // Export: paragraph is numbered
+    bool        m_bRestart;   // Export: restart numbering
+    bool        m_bNumbered;  // Export: paragraph is numbered
 
 public:
 
@@ -75,7 +75,7 @@ public:
     void DecDepth() { if (m_nDeep!=0) --m_nDeep; }
     inline sal_uInt8 GetLevel() const;
 
-    bool IsRestart() const { return m_bRestart; }
+    bool IsRestart(const SwHTMLNumRuleInfo& rPrev) const;
 
     bool IsNumbered() const { return m_bNumbered; }
 
diff --git a/sw/source/filter/html/htmlnumwriter.cxx 
b/sw/source/filter/html/htmlnumwriter.cxx
index 1be982e77ed7..8ca8c08de5ee 100644
--- a/sw/source/filter/html/htmlnumwriter.cxx
+++ b/sw/source/filter/html/htmlnumwriter.cxx
@@ -53,7 +53,7 @@ void SwHTMLWriter::FillNextNumInfo()
             // numbering level during import.
             if( bTable &&
                 m_pNextNumRuleInfo->GetNumRule()==GetNumInfo().GetNumRule() &&
-                !m_pNextNumRuleInfo->IsRestart() )
+                !m_pNextNumRuleInfo->IsRestart(GetNumInfo()) )
             {
                 m_pNextNumRuleInfo->SetDepth( GetNumInfo().GetDepth() );
             }
@@ -90,7 +90,7 @@ SwHTMLWriter& OutHTML_NumberBulletListStart( SwHTMLWriter& 
rWrt,
     SwHTMLNumRuleInfo& rPrevInfo = rWrt.GetNumInfo();
     bool bSameRule = rPrevInfo.GetNumRule() == rInfo.GetNumRule();
     if( bSameRule && rPrevInfo.GetDepth() >= rInfo.GetDepth() &&
-        !rInfo.IsRestart() )
+        !rInfo.IsRestart(rPrevInfo) )
     {
         return rWrt;
     }
@@ -205,7 +205,7 @@ SwHTMLWriter& OutHTML_NumberBulletListStart( SwHTMLWriter& 
rWrt,
     OSL_ENSURE( rWrt.m_nLastParaToken == HtmlTokenId::NONE,
                 "<PRE> was not closed before <OL>." );
     sal_uInt16 nPrevDepth =
-        (bSameRule && !rInfo.IsRestart()) ? rPrevInfo.GetDepth() : 0;
+        (bSameRule && !rInfo.IsRestart(rPrevInfo)) ? rPrevInfo.GetDepth() : 0;
 
     for( sal_uInt16 i=nPrevDepth; i<rInfo.GetDepth(); i++ )
     {
@@ -213,8 +213,9 @@ SwHTMLWriter& OutHTML_NumberBulletListStart( SwHTMLWriter& 
rWrt,
 
         rWrt.m_aBulletGrfs[i].clear();
         OString sOut = "<" + rWrt.GetNamespace();
-        if (rWrt.mbXHTML && i != 0)
+        if (rWrt.mbXHTML && i != nPrevDepth)
         {
+            // for all skipped sublevels, add a li
             sOut += OOO_STRING_SVTOOLS_HTML_li "><" + rWrt.GetNamespace();
         }
         const SwNumFormat& rNumFormat = rInfo.GetNumRule()->Get( i );
@@ -318,7 +319,8 @@ SwHTMLWriter& OutHTML_NumberBulletListEnd( SwHTMLWriter& 
rWrt,
 {
     SwHTMLNumRuleInfo& rInfo = rWrt.GetNumInfo();
     bool bSameRule = rNextInfo.GetNumRule() == rInfo.GetNumRule();
-    bool bListEnd = !bSameRule || rNextInfo.GetDepth() < rInfo.GetDepth() || 
rNextInfo.IsRestart();
+    bool bListEnd = !bSameRule || rNextInfo.GetDepth() < rInfo.GetDepth() || 
rNextInfo.IsRestart(rInfo);
+    bool bNextIsSubitem = !bListEnd && rNextInfo.GetDepth() > rInfo.GetDepth();
 
     std::optional<bool> oAtLeastOneNumbered;
     if (!rInfo.IsNumbered())
@@ -355,7 +357,7 @@ SwHTMLWriter& OutHTML_NumberBulletListEnd( SwHTMLWriter& 
rWrt,
     // node is numbered.
     bool bPrevIsNumbered = rInfo.IsNumbered() || *oAtLeastOneNumbered;
     // XHTML </li> for the list item content, if there is an open <li>.
-    if ((bListEnd && bPrevIsNumbered) || (!bListEnd && rNextInfo.IsNumbered()))
+    if ((bListEnd && bPrevIsNumbered) || (!bListEnd && !bNextIsSubitem && 
rNextInfo.IsNumbered()))
     {
         HTMLOutFuncs::Out_AsciiTag(
             rWrt.Strm(), Concat2View(rWrt.GetNamespace() + 
OOO_STRING_SVTOOLS_HTML_li),
@@ -379,7 +381,7 @@ SwHTMLWriter& OutHTML_NumberBulletListEnd( SwHTMLWriter& 
rWrt,
     OSL_ENSURE( rWrt.m_nLastParaToken == HtmlTokenId::NONE,
                 "<PRE> was not closed before </OL>." );
     sal_uInt16 nNextDepth =
-        (bSameRule && !rNextInfo.IsRestart()) ? rNextInfo.GetDepth() : 0;
+        (bSameRule && !rNextInfo.IsRestart(rInfo)) ? rNextInfo.GetDepth() : 0;
 
     // MIB 23.7.97: We must loop backwards, to get the right order of 
</OL>/</UL>
     for( sal_uInt16 i=rInfo.GetDepth(); i>nNextDepth; i-- )
@@ -396,8 +398,9 @@ SwHTMLWriter& OutHTML_NumberBulletListEnd( SwHTMLWriter& 
rWrt,
         else
             aTag = OOO_STRING_SVTOOLS_HTML_orderlist;
         HTMLOutFuncs::Out_AsciiTag( rWrt.Strm(), 
Concat2View(rWrt.GetNamespace() + aTag), false );
-        if (rWrt.mbXHTML && (nNextDepth != 0 || i != 1))
+        if (rWrt.mbXHTML && i != nNextDepth + 1)
         {
+            // for all skipped sublevels, close a li
             HTMLOutFuncs::Out_AsciiTag(
                 rWrt.Strm(), Concat2View(rWrt.GetNamespace() + 
OOO_STRING_SVTOOLS_HTML_li),
                 /*bOn=*/false);
diff --git a/sw/source/filter/html/htmltabw.cxx 
b/sw/source/filter/html/htmltabw.cxx
index b636e0f6b14a..db18995ebb25 100644
--- a/sw/source/filter/html/htmltabw.cxx
+++ b/sw/source/filter/html/htmltabw.cxx
@@ -1034,9 +1034,8 @@ SwHTMLWriter& OutHTML_SwTableNode( SwHTMLWriter& rWrt, 
SwTableNode & rNode,
         if( aLRItem.GetLeft() > 0 && rWrt.m_nDefListMargin > 0 &&
             ( !rWrt.GetNumInfo().GetNumRule() ||
               ( rWrt.GetNextNumInfo() &&
-                (rWrt.GetNextNumInfo()->IsRestart() ||
-                 rWrt.GetNumInfo().GetNumRule() !=
-                    rWrt.GetNextNumInfo()->GetNumRule()) ) ) )
+                (rWrt.GetNumInfo().GetNumRule() != 
rWrt.GetNextNumInfo()->GetNumRule() ||
+                 rWrt.GetNextNumInfo()->IsRestart(rWrt.GetNumInfo())) ) ) )
         {
             // If the paragraph before the table is not numbered or the
             // paragraph after the table starts with a new numbering or with
@@ -1184,9 +1183,8 @@ SwHTMLWriter& OutHTML_SwTableNode( SwHTMLWriter& rWrt, 
SwTableNode & rNode,
     rWrt.m_bOutTable = false;
 
     if( rWrt.GetNextNumInfo() &&
-        !rWrt.GetNextNumInfo()->IsRestart() &&
-        rWrt.GetNextNumInfo()->GetNumRule() ==
-            rWrt.GetNumInfo().GetNumRule() )
+        rWrt.GetNextNumInfo()->GetNumRule() == rWrt.GetNumInfo().GetNumRule() 
&&
+        !rWrt.GetNextNumInfo()->IsRestart(rWrt.GetNumInfo()) )
     {
         // If the paragraph after the table is numbered with the same rule as 
the
         // one before, then the NumInfo of the next paragraph holds the level 
of

Reply via email to