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 }