sw/source/uibase/uno/unotxdoc.cxx | 7 +-
xmloff/qa/unit/data/listRestartAfterBreak.fodt | 25 ++++++++++
xmloff/qa/unit/text.cxx | 61 +++++++++++++++++++++++++
xmloff/source/text/txtparae.cxx | 33 ++++++-------
4 files changed, 104 insertions(+), 22 deletions(-)
New commits:
commit 7db19db341608ba2059543a3c8d61cd458470602
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Thu Jun 15 10:34:41 2023 +0300
Commit: Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Thu Jun 15 15:47:01 2023 +0200
ODF export: simplify restart handling to skip list id where possible
This continues to minimize cases where random ids are written, helping
to make the output more deterministic; it builds upon commits
8f48f91009caa86d896f247059874242ed18bf39 (ODT export: omit unreferenced
<text:list xml:id="...">, 2022-03-10), and
82bbf63582bdf28e7918e58ebf6657a9144bc9f3 (tdf#155823: Improve the check
if the list id is not required, 2023-06-14).
The previous code used to write 'text:continue-list' when the list is
restarted. It is unnecessary when there is no other condition requiring
such a reference (like style change, or interleaving lists); so relax
the conditions allowing to put simple 'text:continue-numbering="true"'.
This also allows to simplify a bit the code around 'ShouldSkipListId'.
Change-Id: Idf8be455953d08fd578266bda22f3a55d7b9ee23
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/153104
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
diff --git a/sw/source/uibase/uno/unotxdoc.cxx
b/sw/source/uibase/uno/unotxdoc.cxx
index 51c777ee47da..e68d468e3fff 100644
--- a/sw/source/uibase/uno/unotxdoc.cxx
+++ b/sw/source/uibase/uno/unotxdoc.cxx
@@ -1974,8 +1974,8 @@ Any SwXTextDocument::getPropertyValue(const OUString&
rPropertyName)
// A hack to avoid writing random list ids to ODF when they are not
referred later
// see XMLTextParagraphExport::DocumentListNodes ctor
- // Sequence of nodes, each of them represented by four-element
sequence:
- // [ index, styleIntPtr, list_id, isRestart ]
+ // Sequence of nodes, each of them represented by three-element
sequence:
+ // [ index, styleIntPtr, list_id ]
std::vector<css::uno::Sequence<css::uno::Any>> nodes;
const SwDoc& rDoc = *m_pDocShell->GetDoc();
@@ -1989,9 +1989,8 @@ Any SwXTextDocument::getPropertyValue(const OUString&
rPropertyName)
{
css::uno::Any index(pTextNode->GetIndex().get());
css::uno::Any list_id(pTextNode->GetListId());
- css::uno::Any isRestart(pTextNode->IsListRestart());
- nodes.push_back({ index, styleIntPtr, list_id, isRestart });
+ nodes.push_back({ index, styleIntPtr, list_id });
}
}
return css::uno::Any(comphelper::containerToSequence(nodes));
diff --git a/xmloff/qa/unit/data/listRestartAfterBreak.fodt
b/xmloff/qa/unit/data/listRestartAfterBreak.fodt
new file mode 100644
index 000000000000..7ae5c84d7060
--- /dev/null
+++ b/xmloff/qa/unit/data/listRestartAfterBreak.fodt
@@ -0,0 +1,25 @@
+<?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:style-name="Numbering 123">
+ <text:list-item>
+ <text:p>Item1</text:p>
+ </text:list-item>
+ </text:list>
+ <text:p/>
+ <text:list text:continue-numbering="true" text:style-name="Numbering 123">
+ <text:list-item>
+ <text:p>Item2</text:p>
+ </text:list-item>
+ </text:list>
+ <text:p/>
+ <text:list text:continue-numbering="true" text:style-name="Numbering 123">
+ <text:list-item text:start-value="1">
+ <text:p>Item3 (restarted)</text:p>
+ </text:list-item>
+ </text:list>
+ </office:text>
+ </office:body>
+</office:document>
\ No newline at end of file
diff --git a/xmloff/qa/unit/text.cxx b/xmloff/qa/unit/text.cxx
index bff3f8b14821..abd4a0e07dea 100644
--- a/xmloff/qa/unit/text.cxx
+++ b/xmloff/qa/unit/text.cxx
@@ -347,6 +347,67 @@ CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testListIdState)
CPPUNIT_ASSERT(!id.isEmpty());
}
+CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testListIdOnRestart)
+{
+ // Test that a restart of a continued list, by itself, does not introduce
a unneeded xml:id
+ // and text:continue-list, but uses text:continue-numbering, and is
imported correctly.
+
+ // Given a document with a list with a restart after break:
+ loadFromURL(u"listRestartAfterBreak.fodt");
+
+ auto xTextDocument(mxComponent.queryThrow<css::text::XTextDocument>());
+ auto
xParaEnumAccess(xTextDocument->getText().queryThrow<css::container::XEnumerationAccess>());
+ auto xParaEnum(xParaEnumAccess->createEnumeration());
+
+ auto xPara(xParaEnum->nextElement().queryThrow<beans::XPropertySet>());
+ auto aActual(xPara->getPropertyValue("ListLabelString").get<OUString>());
+ CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+ OUString list_id = xPara->getPropertyValue("ListId").get<OUString>();
+ xParaEnum->nextElement(); // Skip empty intermediate paragraph
+ xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+ aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+ CPPUNIT_ASSERT_EQUAL(OUString("2."), aActual);
+ CPPUNIT_ASSERT_EQUAL(list_id,
xPara->getPropertyValue("ListId").get<OUString>());
+ xParaEnum->nextElement(); // Skip empty intermediate paragraph
+ xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY);
+ aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+ // Check that restart was applied correctly, with simple
'text:continue-numbering="true"'
+ CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+ CPPUNIT_ASSERT_EQUAL(list_id,
xPara->getPropertyValue("ListId").get<OUString>());
+
+ // When storing that document as ODF:
+ saveAndReload("writer8");
+
+ xTextDocument.set(mxComponent, uno::UNO_QUERY_THROW);
+ xParaEnumAccess.set(xTextDocument->getText(), uno::UNO_QUERY_THROW);
+ xParaEnum.set(xParaEnumAccess->createEnumeration());
+
+ xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+ aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+ CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+ list_id = xPara->getPropertyValue("ListId").get<OUString>();
+ xParaEnum->nextElement(); // Skip empty intermediate paragraph
+ xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+ aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+ CPPUNIT_ASSERT_EQUAL(OUString("2."), aActual);
+ CPPUNIT_ASSERT_EQUAL(list_id,
xPara->getPropertyValue("ListId").get<OUString>());
+ xParaEnum->nextElement(); // Skip empty intermediate paragraph
+ xPara.set(xParaEnum->nextElement(), uno::UNO_QUERY_THROW);
+ aActual = xPara->getPropertyValue("ListLabelString").get<OUString>();
+ CPPUNIT_ASSERT_EQUAL(OUString("1."), aActual);
+ CPPUNIT_ASSERT_EQUAL(list_id,
xPara->getPropertyValue("ListId").get<OUString>());
+
+ // Then make sure that no xml:id="..." attribute is written, even in
restarted case:
+ xmlDocUniquePtr pXmlDoc = parseExport("content.xml");
+ CPPUNIT_ASSERT(pXmlDoc);
+ assertXPath(pXmlDoc, "//text:list", 3);
+ assertXPathNoAttribute(pXmlDoc, "//text:list[1]", "id");
+ assertXPathNoAttribute(pXmlDoc, "//text:list[2]", "id");
+ assertXPathNoAttribute(pXmlDoc, "//text:list[3]", "id");
+ assertXPathNoAttribute(pXmlDoc, "//text:list[3]", "continue-list");
+ assertXPath(pXmlDoc, "//text:list[3]", "continue-numbering", "true");
+}
+
CPPUNIT_TEST_FIXTURE(XmloffStyleTest, testClearingBreakExport)
{
// Given a document with a clearing break:
diff --git a/xmloff/source/text/txtparae.cxx b/xmloff/source/text/txtparae.cxx
index c78c504bfa50..34e12aec02b2 100644
--- a/xmloff/source/text/txtparae.cxx
+++ b/xmloff/source/text/txtparae.cxx
@@ -1104,8 +1104,7 @@ void XMLTextParagraphExport::exportListChange(
mpTextListsHelper->GetListStyleOfLastProcessedList() &&
// Inconsistent behavior regarding lists
(#i92811#)
sContinueListId ==
- mpTextListsHelper->GetLastProcessedListId() &&
- !rNextInfo.IsRestart() )
+ mpTextListsHelper->GetLastProcessedListId() )
{
GetExport().AddAttribute( XML_NAMESPACE_TEXT,
XML_CONTINUE_NUMBERING,
@@ -1120,15 +1119,15 @@ void XMLTextParagraphExport::exportListChange(
XML_CONTINUE_LIST,
sContinueListId );
}
+ }
- if ( rNextInfo.IsRestart() &&
- ( nListLevelsToBeOpened != 1 ||
- !rNextInfo.HasStartValue() ) )
- {
- bRestartNumberingAtContinuedList = true;
- nRestartValueForContinuedList =
- rNextInfo.GetListLevelStartValue();
- }
+ if ( rNextInfo.IsRestart() &&
+ ( nListLevelsToBeOpened != 1 ||
+ !rNextInfo.HasStartValue() ) )
+ {
+ bRestartNumberingAtContinuedList = true;
+ nRestartValueForContinuedList =
+ rNextInfo.GetListLevelStartValue();
}
mpTextListsHelper->KeepListAsProcessed( sNewListId,
@@ -1804,12 +1803,11 @@ struct XMLTextParagraphExport::DocumentListNodes
sal_Int32 index; // see SwNode::GetIndex and SwNodeOffset
sal_uInt64 style_id; // actually a pointer to NumRule
OUString list_id;
- bool isRestart;
};
std::vector<NodeData> docListNodes;
DocumentListNodes(const css::uno::Reference<css::frame::XModel>& xModel)
{
- // Sequence of nodes, each of them represented by four-element
sequence,
+ // Sequence of nodes, each of them represented by three-element
sequence,
// corresponding to NodeData members
css::uno::Sequence<css::uno::Sequence<css::uno::Any>> nodes;
if (auto xPropSet = xModel.query<css::beans::XPropertySet>())
@@ -1828,9 +1826,9 @@ struct XMLTextParagraphExport::DocumentListNodes
docListNodes.reserve(nodes.getLength());
for (const auto& node : nodes)
{
- assert(node.getLength() == 4);
+ assert(node.getLength() == 3);
docListNodes.push_back({ node[0].get<sal_Int32>(),
node[1].get<sal_uInt64>(),
- node[2].get<OUString>(),
node[3].get<bool>() });
+ node[2].get<OUString>() });
}
std::sort(docListNodes.begin(), docListNodes.end(),
@@ -1884,10 +1882,9 @@ struct XMLTextParagraphExport::DocumentListNodes
if (it->index + 1 != next->index)
{
// we have a gap before the next node with the same list
and style,
- // with no other lists in between. There will be a
continuation;
- // in case of restart, there will be a reference to the id;
- // otherwise, there will be simple
'text:continue-numbering="true"'.
- return !next->isRestart;
+ // with no other lists in between. There will be a
continuation with a
+ // simple 'text:continue-numbering="true"'.
+ return true;
}
it = next; // walk through adjacent nodes of the same list
}
