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 ebf8b0b0699fe34c86badb3087869d902a4e40a8
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu May 18 15:56:46 2023 +0300
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Mon May 22 11:28:06 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>
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/152010
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 710a45c10f3e..b42eef6d83f5 100644
--- a/sw/qa/extras/htmlexport/htmlexport.cxx
+++ b/sw/qa/extras/htmlexport/htmlexport.cxx
@@ -2459,6 +2459,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 822718763e4d..95db5a76525a 100644
--- a/sw/source/filter/html/htmlatr.cxx
+++ b/sw/source/filter/html/htmlatr.cxx
@@ -982,7 +982,7 @@ static void OutHTML_SwFormatOff( Writer& rWrt, const
SwHTMLTextCollOutputInfo& r
const SwHTMLNumRuleInfo& rNRInfo = rHWrt.GetNumInfo();
if( rNextInfo.GetNumRule() != rNRInfo.GetNumRule() ||
rNextInfo.GetDepth() != rNRInfo.GetDepth() ||
- rNextInfo.IsNumbered() || rNextInfo.IsRestart() )
+ rNextInfo.IsNumbered() || rNextInfo.IsRestart(rNRInfo) )
rHWrt.ChangeParaToken( HtmlTokenId::NONE );
OutHTML_NumberBulletListEnd( rHWrt, 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 19483e518b9e..49ea346d17e2 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 @@ Writer& 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 @@ Writer& 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 @@ Writer& OutHTML_NumberBulletListStart( SwHTMLWriter& rWrt,
rWrt.m_aBulletGrfs[i].clear();
OString sOut = "<" + rWrt.GetNamespace();
- if (rWrt.mbXHTML && (nPrevDepth != 0 || 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 );
@@ -321,7 +322,8 @@ Writer& 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 (rWrt.mbXHTML && !rInfo.IsNumbered())
@@ -360,7 +362,7 @@ Writer& 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),
@@ -385,7 +387,7 @@ Writer& 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-- )
@@ -402,8 +404,9 @@ Writer& 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 ab611724474d..538a509db53c 100644
--- a/sw/source/filter/html/htmltabw.cxx
+++ b/sw/source/filter/html/htmltabw.cxx
@@ -1036,9 +1036,8 @@ Writer& OutHTML_SwTableNode( Writer& rWrt, SwTableNode &
rNode,
if( aLRItem.GetLeft() > 0 && rHTMLWrt.m_nDefListMargin > 0 &&
( !rHTMLWrt.GetNumInfo().GetNumRule() ||
( rHTMLWrt.GetNextNumInfo() &&
- (rHTMLWrt.GetNextNumInfo()->IsRestart() ||
- rHTMLWrt.GetNumInfo().GetNumRule() !=
- rHTMLWrt.GetNextNumInfo()->GetNumRule()) ) ) )
+ (rHTMLWrt.GetNumInfo().GetNumRule() !=
rHTMLWrt.GetNextNumInfo()->GetNumRule() ||
+ rHTMLWrt.GetNextNumInfo()->IsRestart(rHTMLWrt.GetNumInfo()))
) ) )
{
// If the paragraph before the table is not numbered or the
// paragraph after the table starts with a new numbering or with
@@ -1186,9 +1185,8 @@ Writer& OutHTML_SwTableNode( Writer& rWrt, SwTableNode &
rNode,
rHTMLWrt.m_bOutTable = false;
if( rHTMLWrt.GetNextNumInfo() &&
- !rHTMLWrt.GetNextNumInfo()->IsRestart() &&
- rHTMLWrt.GetNextNumInfo()->GetNumRule() ==
- rHTMLWrt.GetNumInfo().GetNumRule() )
+ rHTMLWrt.GetNextNumInfo()->GetNumRule() ==
rHTMLWrt.GetNumInfo().GetNumRule() &&
+ !rHTMLWrt.GetNextNumInfo()->IsRestart(rHTMLWrt.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
