From 6035b9766bf263c7ee39261146af9b23639c1236 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 11:12:00 +0200 Subject: [PATCH 1/4] fix(ooxml): nest a list level under the item that opens it A run of `w:numPr` paragraphs was rebuilt into a tree by creating one fresh list per level for *every* item, so consecutive items of the same nested level each landed in a list of their own, hanging off the enclosing list rather than off the item above them. Track the open level instead: one list per level, reused while the level stays, and nested under the last item of the level above. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017ACZ1RcX9pxoMZWBaTdDRU --- .../internal/ooxml/text/ooxml_text_parser.cpp | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/odr/internal/ooxml/text/ooxml_text_parser.cpp b/src/odr/internal/ooxml/text/ooxml_text_parser.cpp index 79b4b53ec..d27fbacd9 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_parser.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_parser.cpp @@ -3,8 +3,10 @@ #include #include +#include #include #include +#include #include @@ -106,24 +108,37 @@ parse_list_element(ElementRegistry ®istry, pugi::xml_node node) { const auto &[element_id, _] = registry.create_element(ElementType::list, node); + // Word writes a flat run of paragraphs and leaves the nesting to `w:ilvl`, so + // the tree is rebuilt here: one list per open level, each nested list hanging + // off the item that opened it, and consecutive items of one level sharing it. + std::vector open_lists{element_id}; + std::vector open_items{null_element_id}; + for (; is_list_item(node); node = node.next_sibling()) { - ElementIdentifier base_id = element_id; - const std::int32_t level = list_level(node); + const auto level = static_cast(std::max(0, list_level(node))); - for (std::int32_t i = 0; i < level; ++i) { - // TODO a nested level should be a list_item wrapping the list + while (open_lists.size() > level + 1) { + open_lists.pop_back(); + open_items.pop_back(); + } + while (open_lists.size() <= level) { const auto &[nested_id, _] = registry.create_element(ElementType::list, node); - registry.append_child(base_id, nested_id); + registry.append_child(open_items.back() != null_element_id + ? open_items.back() + : open_lists.back(), + nested_id); - base_id = nested_id; + open_lists.push_back(nested_id); + open_items.push_back(null_element_id); } const auto &[item_id, _] = registry.create_element(ElementType::list_item, node); - registry.append_child(base_id, item_id); + registry.append_child(open_lists.back(), item_id); + open_items.back() = item_id; auto [child_id, unused] = parse_element_tree( registry, ElementType::paragraph, node, parse_any_element_children); From bfcbb32aac129e0fcf8be30a67c79d3042e87ecf Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 11:12:28 +0200 Subject: [PATCH 2/4] feat(document): resolve a list's type and its items' markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model knew a list only as `list` / `list_item`, with no notion of what labels them, so every list rendered as bullets and a numbered one lost its numbers outright. Both engines had the gap on their open-work list. ODF and OOXML disagree on nearly everything here — ODF nests the XML and hangs the format off a list style per level, Word writes a flat run of paragraphs that name a `w:numId` and resolve through `numbering.xml` — so the seam is the *resolved* label: each engine stamps every item at load time, in document order, and the model exposes `List::type` plus `ListItem::marker` / `::number`. `common/list_numbering` is the shared middle: a level is a format plus a label template in which `%N` names level N's counter (Word's `w:lvlText` verbatim, ODF's prefix / suffix / `text:display-levels` lowered onto it), and a `ListCounter` expands one against the running counters. Number formats — decimal, zero-padded, alphabetic, roman — live there too. ODF resolves through the list-style stack, honouring `text:start-value` and `text:continue-numbering`, and treating a `text:list-header` as unlabelled. This also fixes the style index, which had been keyed on `style:list-style` and `style:outline-style`: the elements are `text:`-prefixed, so the index it filled was always empty. OOXML resolves `w:numFmt` / `w:lvlText` / `w:start` through `w:abstractNum`, `w:numStyleLink` and `w:lvlOverride`. Counters are kept per `w:numId`, which is what lets a numbered list resume after a bullet list interrupts it. Word's symbol-font bullets arrive as private-use code points that render only in Symbol or Wingdings; they map to Unicode where the shape is recognisable and to the level's default bullet otherwise. The pptx adapter's `ListItemAdapter` went away with this: that parser never produces a list item, so the implementation was unreachable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017ACZ1RcX9pxoMZWBaTdDRU --- CMakeLists.txt | 3 + .../include/OdrCoreObjC/ODRDocumentElement.h | 16 ++ apple/src/ODRDocumentElement.mm | 34 +++ jni/CMakeLists.txt | 2 + jni/java/app/opendocument/core/Element.java | 7 + .../app/opendocument/core/ListElement.java | 15 ++ jni/java/app/opendocument/core/ListItem.java | 14 + jni/java/app/opendocument/core/ListType.java | 14 + jni/src/jni_document.cpp | 32 +++ python/src/bind_document.cpp | 11 +- python/tests/conftest.py | 17 ++ python/tests/test_document.py | 24 ++ src/odr/document_element.cpp | 19 ++ src/odr/document_element.hpp | 24 ++ src/odr/internal/abstract/document.hpp | 20 ++ src/odr/internal/common/list_numbering.cpp | 122 +++++++++ src/odr/internal/common/list_numbering.hpp | 59 +++++ src/odr/internal/odf/AGENTS.md | 4 +- src/odr/internal/odf/README.md | 7 +- src/odr/internal/odf/odf_document.cpp | 24 ++ src/odr/internal/odf/odf_element_registry.cpp | 27 ++ src/odr/internal/odf/odf_element_registry.hpp | 10 + src/odr/internal/odf/odf_list.cpp | 208 +++++++++++++++ src/odr/internal/odf/odf_list.hpp | 15 ++ src/odr/internal/odf/odf_style.cpp | 12 +- src/odr/internal/odf/odf_style.hpp | 2 + .../ooxml_presentation_document.cpp | 10 - src/odr/internal/ooxml/text/AGENTS.md | 23 +- src/odr/internal/ooxml/text/README.md | 6 +- .../ooxml/text/ooxml_text_document.cpp | 32 +++ .../ooxml/text/ooxml_text_document.hpp | 3 + .../text/ooxml_text_element_registry.cpp | 27 ++ .../text/ooxml_text_element_registry.hpp | 10 + .../internal/ooxml/text/ooxml_text_list.cpp | 243 ++++++++++++++++++ .../internal/ooxml/text/ooxml_text_list.hpp | 41 +++ test/CMakeLists.txt | 2 + test/src/document_list_test.cpp | 109 ++++++++ .../internal/common/list_numbering_test.cpp | 114 ++++++++ 38 files changed, 1338 insertions(+), 24 deletions(-) create mode 100644 jni/java/app/opendocument/core/ListElement.java create mode 100644 jni/java/app/opendocument/core/ListType.java create mode 100644 src/odr/internal/common/list_numbering.cpp create mode 100644 src/odr/internal/common/list_numbering.hpp create mode 100644 src/odr/internal/odf/odf_list.cpp create mode 100644 src/odr/internal/odf/odf_list.hpp create mode 100644 src/odr/internal/ooxml/text/ooxml_text_list.cpp create mode 100644 src/odr/internal/ooxml/text/ooxml_text_list.hpp create mode 100644 test/src/document_list_test.cpp create mode 100644 test/src/internal/common/list_numbering_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index a11aab175..464dd92fd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -119,6 +119,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/common/file.cpp" "src/odr/internal/common/filesystem.cpp" "src/odr/internal/common/image_file.cpp" + "src/odr/internal/common/list_numbering.cpp" "src/odr/internal/common/media_file.cpp" "src/odr/internal/common/path.cpp" "src/odr/internal/common/random.cpp" @@ -155,6 +156,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/odf/odf_document.cpp" "src/odr/internal/odf/odf_element_registry.cpp" "src/odr/internal/odf/odf_file.cpp" + "src/odr/internal/odf/odf_list.cpp" "src/odr/internal/odf/odf_manifest.cpp" "src/odr/internal/odf/odf_meta.cpp" "src/odr/internal/odf/odf_parser.cpp" @@ -187,6 +189,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_style.cpp" "src/odr/internal/ooxml/text/ooxml_text_document.cpp" "src/odr/internal/ooxml/text/ooxml_text_element_registry.cpp" + "src/odr/internal/ooxml/text/ooxml_text_list.cpp" "src/odr/internal/ooxml/text/ooxml_text_parser.cpp" "src/odr/internal/ooxml/text/ooxml_text_style.cpp" "src/odr/internal/ooxml/ooxml_crypto.cpp" diff --git a/apple/include/OdrCoreObjC/ODRDocumentElement.h b/apple/include/OdrCoreObjC/ODRDocumentElement.h index cff1a3cdf..97ce71903 100644 --- a/apple/include/OdrCoreObjC/ODRDocumentElement.h +++ b/apple/include/OdrCoreObjC/ODRDocumentElement.h @@ -7,6 +7,11 @@ NS_ASSUME_NONNULL_BEGIN @class ODRFile; +typedef NS_ENUM(NSInteger, ODRListType) { + ODRListTypeUnordered, + ODRListTypeOrdered, +} NS_SWIFT_NAME(ListType); + typedef NS_ENUM(NSInteger, ODRElementType) { ODRElementTypeNone = 0, @@ -195,10 +200,21 @@ NS_SWIFT_NAME(Bookmark) @property(nonatomic, readonly, copy) NSString *name; @end +/// `odr::List`. +NS_SWIFT_NAME(List) +@interface ODRList : ODRElement +/// Named apart from `ODRElement.type`, which every element answers. +@property(nonatomic, readonly) ODRListType listType; +@end + /// `odr::ListItem`. NS_SWIFT_NAME(ListItem) @interface ODRListItem : ODRElement @property(nonatomic, readonly) ODRTextStyle *style; +/// The resolved label, or empty where the list style asks for none. +@property(nonatomic, readonly, copy) NSString *marker; +/// The counter behind `marker`, `nil` for an unordered item. +@property(nonatomic, readonly, nullable) NSNumber *number; @end /// `odr::Table`. diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm index 6894885f3..fe0921f0f 100644 --- a/apple/src/ODRDocumentElement.mm +++ b/apple/src/ODRDocumentElement.mm @@ -30,6 +30,9 @@ ODR_SAME_ENUM(ODRElementTypeSpan, odr::ElementType::span); ODR_SAME_ENUM(ODRElementTypeLink, odr::ElementType::link); ODR_SAME_ENUM(ODRElementTypeBookmark, odr::ElementType::bookmark); +ODR_SAME_ENUM(ODRListTypeUnordered, odr::ListType::unordered); +ODR_SAME_ENUM(ODRListTypeOrdered, odr::ListType::ordered); + ODR_SAME_ENUM(ODRElementTypeList, odr::ElementType::list); ODR_SAME_ENUM(ODRElementTypeListItem, odr::ElementType::list_item); ODR_SAME_ENUM(ODRElementTypeTable, odr::ElementType::table); @@ -127,6 +130,9 @@ + (nullable ODRElement *)elementWithHandle:(odr::Element)handle case odr::ElementType::bookmark: klass = [ODRBookmark class]; break; + case odr::ElementType::list: + klass = [ODRList class]; + break; case odr::ElementType::list_item: klass = [ODRListItem class]; break; @@ -540,6 +546,16 @@ - (NSString *)name { @end +@implementation ODRList + +- (ODRListType)listType { + return guarded_value( + [&] { return static_cast(self.handle.as_list().type()); }, + ODRListTypeUnordered); +} + +@end + @implementation ODRListItem - (ODRTextStyle *)style { @@ -551,6 +567,24 @@ - (ODRTextStyle *)style { nil); } +- (NSString *)marker { + return guarded_value( + [&]() -> NSString * { + return to_nsstring(self.handle.as_list_item().marker()); + }, + @""); +} + +- (nullable NSNumber *)number { + return guarded_value( + [&]() -> NSNumber * { + const std::optional number = + self.handle.as_list_item().number(); + return number.has_value() ? @(*number) : nil; + }, + nil); +} + @end @implementation ODRTable diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt index ad572e9a9..e0f36089a 100644 --- a/jni/CMakeLists.txt +++ b/jni/CMakeLists.txt @@ -119,7 +119,9 @@ add_jar(odr_java "java/app/opendocument/core/Line.java" "java/app/opendocument/core/LineBreak.java" "java/app/opendocument/core/Link.java" + "java/app/opendocument/core/ListElement.java" "java/app/opendocument/core/ListItem.java" + "java/app/opendocument/core/ListType.java" "java/app/opendocument/core/MasterPage.java" "java/app/opendocument/core/Measure.java" "java/app/opendocument/core/NativeLibrary.java" diff --git a/jni/java/app/opendocument/core/Element.java b/jni/java/app/opendocument/core/Element.java index f07e44a85..da368af6f 100644 --- a/jni/java/app/opendocument/core/Element.java +++ b/jni/java/app/opendocument/core/Element.java @@ -136,6 +136,11 @@ public Bookmark asBookmark() { return h == 0 ? null : new Bookmark(h, owner()); } + public ListElement asList() { + long h = asListNative(handle()); + return h == 0 ? null : new ListElement(h, owner()); + } + public ListItem asListItem() { long h = asListItemNative(handle()); return h == 0 ? null : new ListItem(h, owner()); @@ -251,6 +256,8 @@ final List wrapAll(long[] handles) { private native long asBookmarkNative(long handle); + private native long asListNative(long handle); + private native long asListItemNative(long handle); private native long asTableNative(long handle); diff --git a/jni/java/app/opendocument/core/ListElement.java b/jni/java/app/opendocument/core/ListElement.java new file mode 100644 index 000000000..49444442e --- /dev/null +++ b/jni/java/app/opendocument/core/ListElement.java @@ -0,0 +1,15 @@ +package app.opendocument.core; + +/** List element. Mirrors {@code odr::List}; named apart from {@code java.util.List}. */ +public final class ListElement extends Element { + ListElement(long handle, Object owner) { + super(handle, owner); + } + + /** Named apart from {@link Element#type}, which every element answers. */ + public ListType listType() { + return ListType.fromNative(listTypeNative(handle())); + } + + private native int listTypeNative(long handle); +} diff --git a/jni/java/app/opendocument/core/ListItem.java b/jni/java/app/opendocument/core/ListItem.java index 5e8c0805c..a69d76b9d 100644 --- a/jni/java/app/opendocument/core/ListItem.java +++ b/jni/java/app/opendocument/core/ListItem.java @@ -10,5 +10,19 @@ public TextStyle style() { return styleNative(handle()); } + /** The resolved label, or empty where the list style asks for none. */ + public String marker() { + return markerNative(handle()); + } + + /** The counter behind {@link #marker}, {@code null} for an unordered item. */ + public Integer number() { + return numberNative(handle()); + } + private native TextStyle styleNative(long handle); + + private native String markerNative(long handle); + + private native Integer numberNative(long handle); } diff --git a/jni/java/app/opendocument/core/ListType.java b/jni/java/app/opendocument/core/ListType.java new file mode 100644 index 000000000..34ee87965 --- /dev/null +++ b/jni/java/app/opendocument/core/ListType.java @@ -0,0 +1,14 @@ +package app.opendocument.core; + +/** Mirrors {@code odr::ListType}; constant order must match the C++ declaration. */ +public enum ListType { + UNORDERED, ORDERED; + + static ListType fromNative(int code) { + return code < 0 ? null : values()[code]; + } + + int toNative() { + return ordinal(); + } +} diff --git a/jni/src/jni_document.cpp b/jni/src/jni_document.cpp index 6a51344c9..58243642e 100644 --- a/jni/src/jni_document.cpp +++ b/jni/src/jni_document.cpp @@ -304,6 +304,7 @@ ODR_JNI_ELEMENT_AS(Span, as_span) ODR_JNI_ELEMENT_AS(Text, as_text) ODR_JNI_ELEMENT_AS(Link, as_link) ODR_JNI_ELEMENT_AS(Bookmark, as_bookmark) +ODR_JNI_ELEMENT_AS(List, as_list) ODR_JNI_ELEMENT_AS(ListItem, as_list_item) ODR_JNI_ELEMENT_AS(Table, as_table) ODR_JNI_ELEMENT_AS(TableColumn, as_table_column) @@ -608,6 +609,15 @@ Java_app_opendocument_core_Bookmark_nameNative(JNIEnv *env, jobject, }); } +// app.opendocument.core.ListElement + +extern "C" JNIEXPORT jint JNICALL +Java_app_opendocument_core_ListElement_listTypeNative(JNIEnv *env, jobject, + jlong handle) { + return guarded( + env, [&] { return static_cast(element(handle).as_list().type()); }); +} + // app.opendocument.core.ListItem extern "C" JNIEXPORT jobject JNICALL @@ -619,6 +629,28 @@ Java_app_opendocument_core_ListItem_styleNative(JNIEnv *env, jobject, }); } +extern "C" JNIEXPORT jstring JNICALL +Java_app_opendocument_core_ListItem_markerNative(JNIEnv *env, jobject, + jlong handle) { + return guarded(env, [&] { + return to_jstring(env, element(handle).as_list_item().marker()); + }); +} + +extern "C" JNIEXPORT jobject JNICALL +Java_app_opendocument_core_ListItem_numberNative(JNIEnv *env, jobject, + jlong handle) { + return guarded(env, [&] { + std::optional number; + if (const std::optional value = + element(handle).as_list_item().number(); + value.has_value()) { + number = static_cast(*value); + } + return odr_jni::make_integer_opt(env, number); + }); +} + // app.opendocument.core.Table extern "C" JNIEXPORT jlong JNICALL diff --git a/python/src/bind_document.cpp b/python/src/bind_document.cpp index d596228ae..b03932c88 100644 --- a/python/src/bind_document.cpp +++ b/python/src/bind_document.cpp @@ -79,6 +79,10 @@ void odr_python::bind_document(py::module_ &m) { .value("at_page", odr::AnchorType::at_page) .value("at_paragraph", odr::AnchorType::at_paragraph); + py::enum_(m, "ListType") + .value("unordered", odr::ListType::unordered) + .value("ordered", odr::ListType::ordered); + py::enum_(m, "ValueType") .value("unknown", odr::ValueType::unknown) .value("string", odr::ValueType::string) @@ -160,6 +164,7 @@ void odr_python::bind_document(py::module_ &m) { .def("as_span", &odr::Element::as_span, keep_self_alive) .def("as_text", &odr::Element::as_text, keep_self_alive) .def("as_link", &odr::Element::as_link, keep_self_alive) + .def("as_list", &odr::Element::as_list, keep_self_alive) .def("as_bookmark", &odr::Element::as_bookmark, keep_self_alive) .def("as_list_item", &odr::Element::as_list_item, keep_self_alive) .def("as_table", &odr::Element::as_table, keep_self_alive) @@ -234,8 +239,12 @@ void odr_python::bind_document(py::module_ &m) { bind_element(m, "Bookmark").def("name", &odr::Bookmark::name); + bind_element(m, "List").def("list_type", &odr::List::type); + bind_element(m, "ListItem") - .def("style", &odr::ListItem::style, keep_self_alive); + .def("style", &odr::ListItem::style, keep_self_alive) + .def("marker", &odr::ListItem::marker) + .def("number", &odr::ListItem::number); bind_element(m, "Table") .def("first_row", &odr::Table::first_row, keep_self_alive) diff --git a/python/tests/conftest.py b/python/tests/conftest.py index 9fdc0bf3b..0adf48e6c 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -5,12 +5,29 @@ ODT_CONTENT_XML = """ + + + + + + + + Hello from pyodr! Second paragraph + + Bulleted + + + First + Second + diff --git a/python/tests/test_document.py b/python/tests/test_document.py index f847d513f..15c7de379 100644 --- a/python/tests/test_document.py +++ b/python/tests/test_document.py @@ -93,3 +93,27 @@ def test_document_filesystem(odt_path): document = pyodr.open(str(odt_path)).as_document_file().document() filesystem = document.as_filesystem() assert filesystem.is_file("/content.xml") + + +def test_list_markers(odt_path): + document = pyodr.open(str(odt_path)).as_document_file().document() + + lists = [ + child + for child in document.root_element().children() + if child.type() == pyodr.ElementType.list + ] + assert len(lists) == 2 + + bullets, numbers = (element.as_list() for element in lists) + assert bullets.list_type() == pyodr.ListType.unordered + assert numbers.list_type() == pyodr.ListType.ordered + + def items(element): + return [child.as_list_item() for child in element.children()] + + assert [item.marker() for item in items(lists[0])] == ["•"] + assert [item.number() for item in items(lists[0])] == [None] + + assert [item.marker() for item in items(lists[1])] == ["1.", "2."] + assert [item.number() for item in items(lists[1])] == [1, 2] diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp index 44149cfb6..d9c03e9eb 100644 --- a/src/odr/document_element.cpp +++ b/src/odr/document_element.cpp @@ -158,6 +158,13 @@ Bookmark Element::as_bookmark() const { return {m_adapter, m_identifier, m_adapter->bookmark_adapter(m_identifier)}; } +List Element::as_list() const { + if (!exists_()) { + return {}; + } + return {m_adapter, m_identifier, m_adapter->list_adapter(m_identifier)}; +} + ListItem Element::as_list_item() const { if (!exists_()) { return {}; @@ -456,10 +463,22 @@ std::string Bookmark::name() const { return exists_() ? m_adapter2->bookmark_name(m_identifier) : ""; } +ListType List::type() const { + return exists_() ? m_adapter2->list_type(m_identifier) : ListType::unordered; +} + TextStyle ListItem::style() const { return exists_() ? m_adapter2->list_item_style(m_identifier) : TextStyle(); } +std::string ListItem::marker() const { + return exists_() ? m_adapter2->list_item_marker(m_identifier) : ""; +} + +std::optional ListItem::number() const { + return exists_() ? m_adapter2->list_item_number(m_identifier) : std::nullopt; +} + TableRow Table::first_row() const { if (!exists_()) { return {}; diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index f8a108ccd..5a38d684b 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -41,6 +41,7 @@ class SpanAdapter; class TextAdapter; class LinkAdapter; class BookmarkAdapter; +class ListAdapter; class ListItemAdapter; class TableAdapter; class TableColumnAdapter; @@ -71,6 +72,7 @@ class Span; class Text; class Link; class Bookmark; +class List; class ListItem; class Table; class TableColumn; @@ -138,6 +140,12 @@ enum class ValueType { float_number, }; +/// @brief Collection of list types. +enum class ListType { + unordered, + ordered, +}; + /// @brief Represents an element in a document. class Element { public: @@ -174,6 +182,7 @@ class Element { [[nodiscard]] Text as_text() const; [[nodiscard]] Link as_link() const; [[nodiscard]] Bookmark as_bookmark() const; + [[nodiscard]] List as_list() const; [[nodiscard]] ListItem as_list_item() const; [[nodiscard]] Table as_table() const; [[nodiscard]] TableColumn as_table_column() const; @@ -404,12 +413,27 @@ class Bookmark final : public ElementBase { [[nodiscard]] std::string name() const; }; +/// @brief Represents a list element in a document. +class List final : public ElementBase { +public: + using ElementBase::ElementBase; + + [[nodiscard]] ListType type() const; +}; + /// @brief Represents a list item element in a document. class ListItem final : public ElementBase { public: using ElementBase::ElementBase; [[nodiscard]] TextStyle style() const; + + /// The label this item is marked with, already resolved against the list + /// style and the running counters — a bullet, "1.", "I.a)", or empty where + /// the list style asks for no label. + [[nodiscard]] std::string marker() const; + /// The counter behind `marker`, absent for an unordered item. + [[nodiscard]] std::optional number() const; }; /// @brief Represents a table element in a document. diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index 40686990e..0712d8bba 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -16,6 +16,7 @@ enum class ElementType; class DocumentPath; enum class ValueType; enum class AnchorType; +enum class ListType; struct PageLayout; struct TableDimensions; struct TablePosition; @@ -47,6 +48,7 @@ class SpanAdapter; class TextAdapter; class LinkAdapter; class BookmarkAdapter; +class ListAdapter; class ListItemAdapter; class TableAdapter; class TableColumnAdapter; @@ -160,6 +162,10 @@ class ElementAdapter { bookmark_adapter([[maybe_unused]] const ElementIdentifier element_id) const { return nullptr; } + [[nodiscard]] virtual const ListAdapter * + list_adapter([[maybe_unused]] const ElementIdentifier element_id) const { + return nullptr; + } [[nodiscard]] virtual const ListItemAdapter * list_item_adapter([[maybe_unused]] const ElementIdentifier element_id) const { return nullptr; @@ -354,12 +360,26 @@ class BookmarkAdapter { bookmark_name(ElementIdentifier element_id) const = 0; }; +class ListAdapter { +public: + virtual ~ListAdapter() = default; + + [[nodiscard]] virtual ListType + list_type(ElementIdentifier element_id) const = 0; +}; + class ListItemAdapter { public: virtual ~ListItemAdapter() = default; [[nodiscard]] virtual TextStyle list_item_style(ElementIdentifier element_id) const = 0; + + [[nodiscard]] virtual std::string + list_item_marker(ElementIdentifier element_id) const = 0; + + [[nodiscard]] virtual std::optional + list_item_number(ElementIdentifier element_id) const = 0; }; class TableAdapter { diff --git a/src/odr/internal/common/list_numbering.cpp b/src/odr/internal/common/list_numbering.cpp new file mode 100644 index 000000000..bc94096c6 --- /dev/null +++ b/src/odr/internal/common/list_numbering.cpp @@ -0,0 +1,122 @@ +#include + +#include +#include + +namespace odr::internal { + +namespace { + +std::string format_decimal(const std::uint32_t number) { + return std::to_string(number); +} + +std::string format_letter(std::uint32_t number, const char base) { + std::string result; + for (; number > 0; number = (number - 1) / 26) { + result.push_back(static_cast(base + (number - 1) % 26)); + } + std::ranges::reverse(result); + return result; +} + +std::string format_roman(std::uint32_t number, const bool upper) { + static constexpr std::array values{ + 1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1}; + static constexpr std::array upper_symbols{ + "M", "CM", "D", "CD", "C", "XC", "L", "XL", "X", "IX", "V", "IV", "I"}; + static constexpr std::array lower_symbols{ + "m", "cm", "d", "cd", "c", "xc", "l", "xl", "x", "ix", "v", "iv", "i"}; + + // Roman has no zero and the additive notation runs out of thousands marks. + if (number == 0 || number > 3999) { + return format_decimal(number); + } + + const auto &symbols = upper ? upper_symbols : lower_symbols; + + std::string result; + for (std::size_t i = 0; i < values.size(); ++i) { + for (; number >= values[i]; number -= values[i]) { + result.append(symbols[i]); + } + } + return result; +} + +} // namespace + +std::string format_list_number(const ListNumberFormat format, + const std::uint32_t number) { + switch (format) { + case ListNumberFormat::decimal: + return format_decimal(number); + case ListNumberFormat::decimal_zero: + return number < 10 ? "0" + format_decimal(number) : format_decimal(number); + case ListNumberFormat::letter_lower: + return format_letter(number, 'a'); + case ListNumberFormat::letter_upper: + return format_letter(number, 'A'); + case ListNumberFormat::roman_lower: + return format_roman(number, false); + case ListNumberFormat::roman_upper: + return format_roman(number, true); + case ListNumberFormat::none: + case ListNumberFormat::bullet: + default: + return ""; + } +} + +std::string ListCounter::advance(const std::uint32_t level, + const ListLevel &list_level) { + grow_(level); + + // 0 marks a level that has not run yet, so it takes its own start value. + std::uint32_t &number = m_numbers[level]; + number = + number == 0 ? std::max(list_level.start, 1) : number + 1; + m_formats[level] = list_level.format; + + std::fill(std::next(std::begin(m_numbers), level + 1), std::end(m_numbers), + 0); + + std::string result; + const std::string &label = list_level.label; + for (std::size_t i = 0; i < label.size();) { + const char c = label[i]; + if (c != '%' || i + 1 >= label.size() || label[i + 1] < '1' || + label[i + 1] > '9') { + result.push_back(c); + ++i; + continue; + } + const auto placeholder = static_cast(label[i + 1] - '1'); + if (placeholder < m_numbers.size()) { + result.append(format_list_number( + m_formats[placeholder], + std::max(m_numbers[placeholder], 1))); + } + i += 2; + } + return result; +} + +std::uint32_t ListCounter::number(const std::uint32_t level) const { + return level < m_numbers.size() ? m_numbers[level] : 0; +} + +void ListCounter::restart(const std::uint32_t level, + const std::uint32_t number) { + grow_(level); + m_numbers[level] = number; +} + +void ListCounter::grow_(const std::uint32_t level) { + if (level >= m_numbers.size()) { + m_numbers.resize(level + 1, 0); + m_formats.resize(level + 1, ListNumberFormat::decimal); + } +} + +} // namespace odr::internal diff --git a/src/odr/internal/common/list_numbering.hpp b/src/odr/internal/common/list_numbering.hpp new file mode 100644 index 000000000..b22628552 --- /dev/null +++ b/src/odr/internal/common/list_numbering.hpp @@ -0,0 +1,59 @@ +#pragma once + +#include +#include +#include +#include + +namespace odr::internal { + +enum class ListNumberFormat { + none, + bullet, + decimal, + decimal_zero, + letter_lower, + letter_upper, + roman_lower, + roman_upper, +}; + +/// @brief One level of a list style, in the shape ODF and OOXML both lower to. +/// +/// `label` is OOXML's `w:lvlText`: literal text in which `%N` stands for the +/// counter of level N, counted from 1. ODF has no such template — its +/// prefix / suffix / `text:display-levels` compose into one. +struct ListLevel final { + ListNumberFormat format{ListNumberFormat::none}; + std::string label; + std::uint32_t start{1}; +}; + +/// @brief What one list item is labelled with, once resolved. +struct ListMarker final { + std::string text; + std::optional number; +}; + +std::string format_list_number(ListNumberFormat format, std::uint32_t number); + +/// @brief The counters of one list, indexed by level. +class ListCounter final { +public: + /// Advances `level` (counted from 0), resets the levels below it, and expands + /// that level's label against the resulting counters. + std::string advance(std::uint32_t level, const ListLevel &list_level); + + /// The counter `advance` last produced for `level`. + [[nodiscard]] std::uint32_t number(std::uint32_t level) const; + + void restart(std::uint32_t level, std::uint32_t number); + +private: + std::vector m_numbers; + std::vector m_formats; + + void grow_(std::uint32_t level); +}; + +} // namespace odr::internal diff --git a/src/odr/internal/odf/AGENTS.md b/src/odr/internal/odf/AGENTS.md index f18df0c6c..2addb109e 100644 --- a/src/odr/internal/odf/AGENTS.md +++ b/src/odr/internal/odf/AGENTS.md @@ -116,7 +116,7 @@ The structural/foundational gaps, roughly by value: empty-row/cell detection is approximate. 6. **Style gaps beyond missing properties**: percent margins dropped; `transparent`/alpha colours → `nullopt`; the Style-vs-element cascade layering - is provisional (`// TODO use override?`). List/outline numbering is indexed but - not rendered as numbers. + is provisional (`// TODO use override?`). Outline numbering is indexed but not + applied to headings. 7. **StarOffice/template mimetypes** are aliased onto the four base types; they may deserve distinct `FileType`s (`odf_meta.cpp`). diff --git a/src/odr/internal/odf/README.md b/src/odr/internal/odf/README.md index 6bafad9ac..b46f54fed 100644 --- a/src/odr/internal/odf/README.md +++ b/src/odr/internal/odf/README.md @@ -80,8 +80,11 @@ Roughly ordered by importance. - [x] soft page breaks - [x] listings - [x] bullets - - [ ] numbering (`style:list-style` / `style:outline-style` are indexed but - not yet rendered as numbers) + - [x] numbering (`text:list-style` levels resolved to markers: format, + prefix / suffix, `text:display-levels`, `text:start-value`, + `text:continue-numbering`) + - [ ] `text:list-level-style-image` (falls back to a bullet) + - [ ] `text:outline-style` (indexed, not applied to headings) ### Spreadsheet documents (`.ods`) diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 898e5343d..bd70201a5 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -42,6 +43,8 @@ Document::Document(const FileType file_type, const DocumentType document_type, m_style_registry = StyleRegistry(*this, m_content_xml.document_element(), m_styles_xml.document_element()); + resolve_list_numbering(m_element_registry, m_style_registry, m_root_element); + m_element_adapter = create_element_adapter(*this, m_element_registry); } @@ -157,6 +160,7 @@ class ElementAdapter final : public abstract::ElementAdapter, public abstract::TextAdapter, public abstract::LinkAdapter, public abstract::BookmarkAdapter, + public abstract::ListAdapter, public abstract::ListItemAdapter, public abstract::TableAdapter, public abstract::TableColumnAdapter, @@ -278,6 +282,11 @@ class ElementAdapter final : public abstract::ElementAdapter, bookmark_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::bookmark ? this : nullptr; } + [[nodiscard]] const ListAdapter * + list_adapter(const ElementIdentifier element_id) const override { + return element_type(element_id) == ElementType::list ? this : nullptr; + } + [[nodiscard]] const ListItemAdapter * list_item_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::list_item ? this : nullptr; @@ -639,11 +648,26 @@ class ElementAdapter final : public abstract::ElementAdapter, return get_node(element_id).attribute("text:name").value(); } + [[nodiscard]] ListType + list_type(const ElementIdentifier element_id) const override { + return m_registry->list_type(element_id); + } + [[nodiscard]] TextStyle list_item_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; } + [[nodiscard]] std::string + list_item_marker(const ElementIdentifier element_id) const override { + return m_registry->list_marker(element_id).text; + } + + [[nodiscard]] std::optional + list_item_number(const ElementIdentifier element_id) const override { + return m_registry->list_marker(element_id).number; + } + [[nodiscard]] TableDimensions table_dimensions(const ElementIdentifier element_id) const override { const pugi::xml_node node = get_node(element_id); diff --git a/src/odr/internal/odf/odf_element_registry.cpp b/src/odr/internal/odf/odf_element_registry.cpp index 524db1cd0..1ea0dbcf2 100644 --- a/src/odr/internal/odf/odf_element_registry.cpp +++ b/src/odr/internal/odf/odf_element_registry.cpp @@ -12,6 +12,8 @@ void ElementRegistry::clear() noexcept { m_tables.clear(); m_sheets.clear(); m_sheet_cells.clear(); + m_list_types.clear(); + m_list_markers.clear(); } [[nodiscard]] std::size_t ElementRegistry::size() const noexcept { @@ -321,4 +323,29 @@ ElementRegistry::Sheet::cell_node(const std::uint32_t column, return {}; } +void ElementRegistry::set_list_type(const ElementIdentifier id, + const ListType type) { + check_element_id(id); + m_list_types[id] = type; +} + +void ElementRegistry::set_list_marker(const ElementIdentifier id, + ListMarker marker) { + check_element_id(id); + m_list_markers[id] = std::move(marker); +} + +[[nodiscard]] ListType +ElementRegistry::list_type(const ElementIdentifier id) const { + const auto it = m_list_types.find(id); + return it != std::end(m_list_types) ? it->second : ListType::unordered; +} + +[[nodiscard]] const ListMarker & +ElementRegistry::list_marker(const ElementIdentifier id) const { + static const ListMarker none; + const auto it = m_list_markers.find(id); + return it != std::end(m_list_markers) ? it->second : none; +} + } // namespace odr::internal::odf diff --git a/src/odr/internal/odf/odf_element_registry.hpp b/src/odr/internal/odf/odf_element_registry.hpp index 97d85197d..0c7515828 100644 --- a/src/odr/internal/odf/odf_element_registry.hpp +++ b/src/odr/internal/odf/odf_element_registry.hpp @@ -2,6 +2,8 @@ #include #include + +#include #include #include @@ -112,6 +114,12 @@ class ElementRegistry final { [[nodiscard]] const SheetCell *sheet_cell_element(ElementIdentifier id) const; + void set_list_type(ElementIdentifier id, ListType type); + void set_list_marker(ElementIdentifier id, ListMarker marker); + + [[nodiscard]] ListType list_type(ElementIdentifier id) const; + [[nodiscard]] const ListMarker &list_marker(ElementIdentifier id) const; + void append_child(ElementIdentifier parent_id, ElementIdentifier child_id); void append_column(ElementIdentifier table_id, ElementIdentifier column_id); void append_shape(ElementIdentifier sheet_id, ElementIdentifier shape_id); @@ -123,6 +131,8 @@ class ElementRegistry final { std::unordered_map m_tables; std::unordered_map m_sheets; std::unordered_map m_sheet_cells; + std::unordered_map m_list_types; + std::unordered_map m_list_markers; /// Links `child_id` as the last child of the chain `first_id`/`last_id`. void link_child(ElementIdentifier parent_id, ElementIdentifier child_id, diff --git a/src/odr/internal/odf/odf_list.cpp b/src/odr/internal/odf/odf_list.cpp new file mode 100644 index 000000000..46070c9b3 --- /dev/null +++ b/src/odr/internal/odf/odf_list.cpp @@ -0,0 +1,208 @@ +#include + +#include + +#include +#include +#include + +#include +#include +#include + +#include + +namespace odr::internal::odf { + +namespace { + +ListNumberFormat parse_number_format(const char *format) { + const std::string value = format != nullptr ? format : ""; + + if (value.empty()) { + return ListNumberFormat::none; + } + if (value == "a") { + return ListNumberFormat::letter_lower; + } + if (value == "A") { + return ListNumberFormat::letter_upper; + } + if (value == "i") { + return ListNumberFormat::roman_lower; + } + if (value == "I") { + return ListNumberFormat::roman_upper; + } + // Anything else is decimal; a leading zero asks for the padded variant. + return value.front() == '0' ? ListNumberFormat::decimal_zero + : ListNumberFormat::decimal; +} + +/// The level style for `level` (counted from 0), or the deepest one defined. +pugi::xml_node level_style_node(const pugi::xml_node list_style, + const std::uint32_t level) { + pugi::xml_node result; + for (const pugi::xml_node child : list_style.children()) { + const auto child_level = child.attribute("text:level").as_uint(1); + if (child_level == level + 1) { + return child; + } + if (!result || child_level > result.attribute("text:level").as_uint(1)) { + result = child; + } + } + return result; +} + +ListLevel read_level(const pugi::xml_node level_style, + const std::uint32_t level) { + ListLevel result; + + const std::string name = level_style.name(); + + if (name == "text:list-level-style-number") { + result.format = + parse_number_format(level_style.attribute("style:num-format").value()); + result.start = level_style.attribute("text:start-value").as_uint(1); + + // ODF spells the label out as prefix, the numbers of the last + // `text:display-levels` levels joined by a period, then suffix. + const auto display_levels = + level_style.attribute("text:display-levels").as_uint(1); + const std::uint32_t first = + display_levels > level ? 0 : level - display_levels + 1; + + result.label = level_style.attribute("style:num-prefix").value(); + for (std::uint32_t i = first; i <= level; ++i) { + if (i > first) { + result.label += "."; + } + result.label += "%" + std::to_string(i + 1); + } + result.label += level_style.attribute("style:num-suffix").value(); + + return result; + } + + result.format = ListNumberFormat::bullet; + if (name == "text:list-level-style-bullet") { + result.label = level_style.attribute("text:bullet-char").value(); + } + if (result.label.empty()) { + // A bullet without a character, and the image variant, still want a mark. + result.label = "•"; + } + return result; +} + +bool is_ordered(const ListNumberFormat format) { + return format != ListNumberFormat::bullet && format != ListNumberFormat::none; +} + +class Resolver final { +public: + Resolver(ElementRegistry ®istry, const StyleRegistry &styles) + : m_registry{®istry}, m_styles{&styles} {} + + void walk(const ElementIdentifier id) { + for (ElementIdentifier child_id = id; child_id != null_element_id; + child_id = m_registry->element_at(child_id).next_sibling_id) { + const ElementRegistry::Element &element = + m_registry->element_at(child_id); + + switch (element.type) { + case ElementType::list: + walk_list_(child_id, element); + break; + case ElementType::list_item: + walk_list_item_(child_id, element); + break; + default: + walk(element.first_child_id); + break; + } + } + } + +private: + ElementRegistry *m_registry{nullptr}; + const StyleRegistry *m_styles{nullptr}; + + std::unordered_map m_counters; + std::vector m_open_lists; + + [[nodiscard]] ListLevel level_at_(const std::string &style_name, + const std::uint32_t level) const { + return read_level( + level_style_node(m_styles->list_style_node(style_name), level), level); + } + + void walk_list_(const ElementIdentifier id, + const ElementRegistry::Element &element) { + const auto level = static_cast(m_open_lists.size()); + + // A nested list usually carries no style of its own and stays on the one + // its ancestor opened. + std::string style_name = element.node.attribute("text:style-name").value(); + if (style_name.empty() && !m_open_lists.empty()) { + style_name = m_open_lists.back(); + } + + // Only an outermost list restarts; a nested one continues under the + // counters its parent item just reset. + if (level == 0 && + !element.node.attribute("text:continue-numbering").as_bool()) { + m_counters.erase(style_name); + } + + m_registry->set_list_type(id, + is_ordered(level_at_(style_name, level).format) + ? ListType::ordered + : ListType::unordered); + + m_open_lists.push_back(std::move(style_name)); + walk(element.first_child_id); + m_open_lists.pop_back(); + } + + void walk_list_item_(const ElementIdentifier id, + const ElementRegistry::Element &element) { + if (!m_open_lists.empty() && + std::string(element.node.name()) != "text:list-header") { + const auto level = static_cast(m_open_lists.size() - 1); + const std::string &style_name = m_open_lists.back(); + + ListLevel list_level = level_at_(style_name, level); + + ListCounter &counter = m_counters[style_name]; + if (const pugi::xml_attribute start = + element.node.attribute("text:start-value")) { + list_level.start = start.as_uint(1); + counter.restart(level, 0); + } + + ListMarker marker; + marker.text = counter.advance(level, list_level); + if (is_ordered(list_level.format)) { + marker.number = counter.number(level); + } + m_registry->set_list_marker(id, std::move(marker)); + } + + walk(element.first_child_id); + } +}; + +} // namespace + +void resolve_list_numbering(ElementRegistry ®istry, + const StyleRegistry &styles, + const ElementIdentifier root_id) { + if (root_id == null_element_id) { + return; + } + Resolver{registry, styles}.walk(registry.element_at(root_id).first_child_id); +} + +} // namespace odr::internal::odf diff --git a/src/odr/internal/odf/odf_list.hpp b/src/odr/internal/odf/odf_list.hpp new file mode 100644 index 000000000..918d70365 --- /dev/null +++ b/src/odr/internal/odf/odf_list.hpp @@ -0,0 +1,15 @@ +#pragma once + +#include + +namespace odr::internal::odf { +class ElementRegistry; +class StyleRegistry; + +/// Stamps every list and item, in document order — which is what the counters +/// need. +void resolve_list_numbering(ElementRegistry ®istry, + const StyleRegistry &styles, + ElementIdentifier root_id); + +} // namespace odr::internal::odf diff --git a/src/odr/internal/odf/odf_style.cpp b/src/odr/internal/odf/odf_style.cpp index 9b297035e..7f931d948 100644 --- a/src/odr/internal/odf/odf_style.cpp +++ b/src/odr/internal/odf/odf_style.cpp @@ -542,9 +542,9 @@ void StyleRegistry::generate_indices_(const pugi::xml_node node) { m_index_default_style[e.attribute("style:family").value()] = e; } else if (name == "style:style") { m_index_style[e.attribute("style:name").value()] = e; - } else if (name == "style:list-style") { + } else if (name == "text:list-style") { m_index_list_style[e.attribute("style:name").value()] = e; - } else if (name == "style:outline-style") { + } else if (name == "text:outline-style") { m_index_outline_style[e.attribute("style:name").value()] = e; } else if (name == "style:page-layout") { m_index_page_layout[e.attribute("style:name").value()] = e; @@ -643,6 +643,14 @@ pugi::xml_node StyleRegistry::font_face_node(const std::string &name) const { return {}; } +pugi::xml_node StyleRegistry::list_style_node(const std::string &name) const { + if (const auto list_style_it = m_index_list_style.find(name); + list_style_it != std::end(m_index_list_style)) { + return list_style_it->second; + } + return {}; +} + ElementIdentifier StyleRegistry::master_page(const std::string &name) const { if (const auto master_page_elements_it = m_master_page_elements.find(name); master_page_elements_it != std::end(m_master_page_elements)) { diff --git a/src/odr/internal/odf/odf_style.hpp b/src/odr/internal/odf/odf_style.hpp index 227615377..3f6620eab 100644 --- a/src/odr/internal/odf/odf_style.hpp +++ b/src/odr/internal/odf/odf_style.hpp @@ -74,6 +74,8 @@ class StyleRegistry final { [[nodiscard]] pugi::xml_node font_face_node(const std::string &name) const; + [[nodiscard]] pugi::xml_node list_style_node(const std::string &name) const; + [[nodiscard]] ElementIdentifier master_page(const std::string &name) const; [[nodiscard]] ElementIdentifier first_master_page() const; diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp index bac0fc5e5..9b6c7d26e 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp @@ -152,7 +152,6 @@ class ElementAdapter final : public abstract::ElementAdapter, public abstract::TextAdapter, public abstract::LinkAdapter, public abstract::BookmarkAdapter, - public abstract::ListItemAdapter, public abstract::TableAdapter, public abstract::TableColumnAdapter, public abstract::TableRowAdapter, @@ -241,10 +240,6 @@ class ElementAdapter final : public abstract::ElementAdapter, bookmark_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::bookmark ? this : nullptr; } - [[nodiscard]] const ListItemAdapter * - list_item_adapter(const ElementIdentifier element_id) const override { - return element_type(element_id) == ElementType::list_item ? this : nullptr; - } [[nodiscard]] const TableAdapter * table_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::table ? this : nullptr; @@ -398,11 +393,6 @@ class ElementAdapter final : public abstract::ElementAdapter, return get_node(element_id).attribute("text:name").value(); } - [[nodiscard]] TextStyle - list_item_style(const ElementIdentifier element_id) const override { - return get_intermediate_style(element_id).text_style; - } - [[nodiscard]] TableDimensions table_dimensions(const ElementIdentifier element_id) const override { const pugi::xml_node node = get_node(element_id); diff --git a/src/odr/internal/ooxml/text/AGENTS.md b/src/odr/internal/ooxml/text/AGENTS.md index e56c7cd1b..64ceea203 100644 --- a/src/odr/internal/ooxml/text/AGENTS.md +++ b/src/odr/internal/ooxml/text/AGENTS.md @@ -25,7 +25,18 @@ siblings for a continuation at the same grid column (grid column = sum of preceding cells' `gridSpan`s). **Lists are detected structurally**, before the tag table: a paragraph with -`w:pPr/w:numPr` is a list item, nesting synthesised from the `w:ilvl` level. +`w:pPr/w:numPr` is a list item, nesting synthesised from the `w:ilvl` level — +one list per open level, each nested list hanging off the item that opened it. +A `w:numPr` inherited from `w:pStyle` is *not* seen, so such a paragraph is not +recognised as a list item. + +**Numbering resolves at load, not at render.** `NumberingRegistry` +(`ooxml_text_list.*`) indexes `word/numbering.xml`; a post-parse pass walks the +tree in document order and stamps each item with its label. Counters live per +`w:numId` — not per element — which is what makes Word's numbering survive an +interleaved list. `w:lvlText` is the template, `%N` naming a level's counter; +the shared expansion and the number formats are in `common/list_numbering.*`, +where ODF lowers to the same shape. **Style resolution mixes a static hierarchy with a runtime cascade.** `StyleRegistry` indexes `w:style` by `w:styleId` and pre-flattens the @@ -53,15 +64,17 @@ throws (no re-encryption). | `ooxml_text_parser.{hpp,cpp}` | `parse_tree`: tag dispatch, list/text/table special parsers | | `ooxml_text_element_registry.{hpp,cpp}` | Flat element store + Table/Text side maps | | `ooxml_text_style.{hpp,cpp}` | `StyleRegistry`/`Style`: `w:styleId` index, `w:basedOn` flatten, docDefaults, partial-style readers | +| `ooxml_text_list.{hpp,cpp}` | `NumberingRegistry`: `word/numbering.xml` index; the post-parse pass that stamps every item's marker | ## Status & open work Style/element coverage is in [`README.md`](README.md). Foundational gaps: -1. **Numbering.** `w:numPr` levels drive list *nesting*, but `numbering.xml` is - never parsed, so list formats are not resolved to actual numbers (bullets - only). The nested-level construction in the parser is partly stubbed - (`/* TODO fix lists */`). +1. **Numbering gaps.** A `w:numPr` reached through `w:pStyle` is not detected; + `w:lvlOverride` handles `w:startOverride` and a replacement `w:lvl` but + nothing else; symbol-font bullets are mapped to Unicode by a small table and + otherwise fall back to the level's default shape, since the private-use code + points Word writes render only in Symbol / Wingdings. 2. **No structural editing**; save doesn't stream (buffers document.xml, re-zips the whole package); no re-encryption on save. 3. **Theme fonts unhandled.** `w:rFonts w:asciiTheme="minorHAnsi"` (etc.) is diff --git a/src/odr/internal/ooxml/text/README.md b/src/odr/internal/ooxml/text/README.md index a19dffea4..f4fb4cb8b 100644 --- a/src/odr/internal/ooxml/text/README.md +++ b/src/odr/internal/ooxml/text/README.md @@ -35,8 +35,10 @@ Roughly ordered by importance. - [x] structured document tags (rendered as generic groups) - [x] listings - [x] bullets (incl. nesting by level) - - [ ] numbering (`w:numPr` levels are honored, but `numbering.xml` formats are - not resolved to actual numbers) + - [x] numbering (`numbering.xml` resolved to markers: `w:numFmt`, `w:lvlText` + templates, `w:start`, `w:lvlOverride`, `w:numStyleLink`) + - [ ] a `w:numPr` inherited through `w:pStyle` + - [ ] picture bullets (`w:lvlPicBulletId`) - [ ] annotations / comments ### Styles diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index 912a3c2ff..5e55601c4 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -31,6 +31,12 @@ Document::Document(std::shared_ptr files) m_document_xml = util::xml::parse(*m_files, AbsPath("/word/document.xml")); m_styles_xml = util::xml::parse(*m_files, AbsPath("/word/styles.xml")); + // Optional: a document without a single list carries no numbering part. + if (m_files->exists(AbsPath("/word/numbering.xml"))) { + m_numbering_xml = + util::xml::parse(*m_files, AbsPath("/word/numbering.xml")); + } + m_document_relations = parse_relationships(*m_files, AbsPath("/word/document.xml")); @@ -38,6 +44,11 @@ Document::Document(std::shared_ptr files) m_element_registry, m_document_xml.document_element().child("w:body")); m_style_registry = StyleRegistry(m_styles_xml.document_element()); + m_numbering_registry = NumberingRegistry(m_numbering_xml.document_element(), + m_styles_xml.document_element()); + + resolve_list_numbering(m_element_registry, m_numbering_registry, + m_root_element); m_element_adapter = create_element_adapter(*this, m_element_registry); } @@ -105,6 +116,7 @@ class ElementAdapter final : public abstract::ElementAdapter, public abstract::TextAdapter, public abstract::LinkAdapter, public abstract::BookmarkAdapter, + public abstract::ListAdapter, public abstract::ListItemAdapter, public abstract::TableAdapter, public abstract::TableColumnAdapter, @@ -193,6 +205,11 @@ class ElementAdapter final : public abstract::ElementAdapter, bookmark_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::bookmark ? this : nullptr; } + [[nodiscard]] const ListAdapter * + list_adapter(const ElementIdentifier element_id) const override { + return element_type(element_id) == ElementType::list ? this : nullptr; + } + [[nodiscard]] const ListItemAdapter * list_item_adapter(const ElementIdentifier element_id) const override { return element_type(element_id) == ElementType::list_item ? this : nullptr; @@ -362,6 +379,21 @@ class ElementAdapter final : public abstract::ElementAdapter, return get_intermediate_style(element_id).text_style; } + [[nodiscard]] ListType + list_type(const ElementIdentifier element_id) const override { + return m_registry->list_type(element_id); + } + + [[nodiscard]] std::string + list_item_marker(const ElementIdentifier element_id) const override { + return m_registry->list_marker(element_id).text; + } + + [[nodiscard]] std::optional + list_item_number(const ElementIdentifier element_id) const override { + return m_registry->list_marker(element_id).number; + } + [[nodiscard]] TableDimensions table_dimensions(const ElementIdentifier element_id) const override { const pugi::xml_node node = get_node(element_id); diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.hpp b/src/odr/internal/ooxml/text/ooxml_text_document.hpp index 734311cec..e002ffd33 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.hpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -32,11 +33,13 @@ class Document final : public internal::Document { private: pugi::xml_document m_document_xml; pugi::xml_document m_styles_xml; + pugi::xml_document m_numbering_xml; Relations m_document_relations; ElementRegistry m_element_registry; StyleRegistry m_style_registry; + NumberingRegistry m_numbering_registry; }; } // namespace odr::internal::ooxml::text diff --git a/src/odr/internal/ooxml/text/ooxml_text_element_registry.cpp b/src/odr/internal/ooxml/text/ooxml_text_element_registry.cpp index 0e241cc4f..1b8870bc1 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_element_registry.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_element_registry.cpp @@ -8,6 +8,8 @@ void ElementRegistry::clear() noexcept { m_elements.clear(); m_tables.clear(); m_texts.clear(); + m_list_types.clear(); + m_list_markers.clear(); } [[nodiscard]] std::size_t ElementRegistry::size() const noexcept { @@ -141,4 +143,29 @@ void ElementRegistry::append_column(const ElementIdentifier table_id, table_element_at(table_id).last_column_id = column_id; } +void ElementRegistry::set_list_type(const ElementIdentifier id, + const ListType type) { + check_element_id(id); + m_list_types[id] = type; +} + +void ElementRegistry::set_list_marker(const ElementIdentifier id, + ListMarker marker) { + check_element_id(id); + m_list_markers[id] = std::move(marker); +} + +[[nodiscard]] ListType +ElementRegistry::list_type(const ElementIdentifier id) const { + const auto it = m_list_types.find(id); + return it != std::end(m_list_types) ? it->second : ListType::unordered; +} + +[[nodiscard]] const ListMarker & +ElementRegistry::list_marker(const ElementIdentifier id) const { + static const ListMarker none; + const auto it = m_list_markers.find(id); + return it != std::end(m_list_markers) ? it->second : none; +} + } // namespace odr::internal::ooxml::text diff --git a/src/odr/internal/ooxml/text/ooxml_text_element_registry.hpp b/src/odr/internal/ooxml/text/ooxml_text_element_registry.hpp index 0faf355d8..e3c9eb9e9 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_element_registry.hpp +++ b/src/odr/internal/ooxml/text/ooxml_text_element_registry.hpp @@ -3,6 +3,8 @@ #include #include +#include + #include #include #include @@ -54,10 +56,18 @@ class ElementRegistry final { void append_child(ElementIdentifier parent_id, ElementIdentifier child_id); void append_column(ElementIdentifier table_id, ElementIdentifier column_id); + void set_list_type(ElementIdentifier id, ListType type); + void set_list_marker(ElementIdentifier id, ListMarker marker); + + [[nodiscard]] ListType list_type(ElementIdentifier id) const; + [[nodiscard]] const ListMarker &list_marker(ElementIdentifier id) const; + private: std::vector m_elements; std::unordered_map m_tables; std::unordered_map m_texts; + std::unordered_map m_list_types; + std::unordered_map m_list_markers; void check_element_id(ElementIdentifier id) const; void check_table_id(ElementIdentifier id) const; diff --git a/src/odr/internal/ooxml/text/ooxml_text_list.cpp b/src/odr/internal/ooxml/text/ooxml_text_list.cpp new file mode 100644 index 000000000..e2735882a --- /dev/null +++ b/src/odr/internal/ooxml/text/ooxml_text_list.cpp @@ -0,0 +1,243 @@ +#include + +#include + +#include + +#include + +#include + +namespace odr::internal::ooxml::text { + +namespace { + +/// How deep `w:numStyleLink` is followed before giving up on a cycle. +constexpr std::uint32_t max_numbering_indirection = 4; + +ListNumberFormat parse_number_format(const std::string &format) { + if (format == "bullet") { + return ListNumberFormat::bullet; + } + if (format == "none") { + return ListNumberFormat::none; + } + if (format == "upperRoman") { + return ListNumberFormat::roman_upper; + } + if (format == "lowerRoman") { + return ListNumberFormat::roman_lower; + } + if (format == "upperLetter") { + return ListNumberFormat::letter_upper; + } + if (format == "lowerLetter") { + return ListNumberFormat::letter_lower; + } + if (format == "decimalZero") { + return ListNumberFormat::decimal_zero; + } + return ListNumberFormat::decimal; +} + +/// Word writes symbol-font bullets as private-use code points that only render +/// in Symbol or Wingdings, so they arrive as tofu in any other font. Map the +/// ones that carry meaning and fall back to the level's default shape. +std::string resolve_bullet(const std::string &text, const std::uint32_t level) { + static constexpr std::array defaults{"•", "◦", "▪"}; + + if (text.empty()) { + return defaults[level % defaults.size()]; + } + + auto it = std::begin(text); + const char32_t first = utf8::unchecked::next(it); + if (first < 0xE000 || first > 0xF8FF) { + return text; + } + + switch (first) { + case 0xF06E: + return "■"; + case 0xF075: + return "❖"; + case 0xF0A7: + return "▪"; + case 0xF0A8: + return "◆"; + case 0xF0D8: + return "➢"; + case 0xF0FC: + return "✔"; + default: + return defaults[level % defaults.size()]; + } +} + +pugi::xml_node level_node(const pugi::xml_node abstract_numbering, + const std::uint32_t level) { + pugi::xml_node result; + for (const pugi::xml_node child : abstract_numbering.children("w:lvl")) { + const auto child_level = child.attribute("w:ilvl").as_uint(0); + if (child_level == level) { + return child; + } + if (!result || child_level > result.attribute("w:ilvl").as_uint(0)) { + result = child; + } + } + return result; +} + +ListLevel read_level(const pugi::xml_node node, const std::uint32_t level) { + ListLevel result; + result.format = + parse_number_format(node.child("w:numFmt").attribute("w:val").value()); + result.start = node.child("w:start").attribute("w:val").as_uint(1); + result.label = node.child("w:lvlText").attribute("w:val").value(); + + if (result.format == ListNumberFormat::bullet) { + result.label = resolve_bullet(result.label, level); + } + return result; +} + +} // namespace + +NumberingRegistry::NumberingRegistry(const pugi::xml_node numbering_root, + const pugi::xml_node styles_root) { + for (const pugi::xml_node node : numbering_root.children("w:abstractNum")) { + m_abstract_numbering[node.attribute("w:abstractNumId").value()] = node; + } + for (const pugi::xml_node node : numbering_root.children("w:num")) { + m_numbering[node.attribute("w:numId").value()] = node; + } + for (const pugi::xml_node node : styles_root.children("w:style")) { + if (const pugi::xml_attribute num_id = + node.child("w:pPr").child("w:numPr").child("w:numId").attribute( + "w:val")) { + m_style_numbering[node.attribute("w:styleId").value()] = num_id.value(); + } + } +} + +pugi::xml_node +NumberingRegistry::abstract_numbering_(const std::string &num_id, + const std::uint32_t depth) const { + if (depth > max_numbering_indirection) { + return {}; + } + + const auto numbering_it = m_numbering.find(num_id); + if (numbering_it == std::end(m_numbering)) { + return {}; + } + + const auto abstract_it = m_abstract_numbering.find( + numbering_it->second.child("w:abstractNumId").attribute("w:val").value()); + if (abstract_it == std::end(m_abstract_numbering)) { + return {}; + } + + // An abstract numbering may only name the style that holds the real one. + if (const pugi::xml_node link = abstract_it->second.child("w:numStyleLink")) { + const auto style_it = + m_style_numbering.find(link.attribute("w:val").value()); + if (style_it != std::end(m_style_numbering) && style_it->second != num_id) { + if (const pugi::xml_node linked = + abstract_numbering_(style_it->second, depth + 1)) { + return linked; + } + } + } + + return abstract_it->second; +} + +ListLevel NumberingRegistry::level(const std::string &num_id, + const std::uint32_t level) const { + ListLevel result; + result.format = ListNumberFormat::bullet; + result.label = resolve_bullet("", level); + + const auto numbering_it = m_numbering.find(num_id); + if (numbering_it == std::end(m_numbering)) { + return result; + } + + pugi::xml_node override_node; + for (const pugi::xml_node node : + numbering_it->second.children("w:lvlOverride")) { + if (node.attribute("w:ilvl").as_uint(0) == level) { + override_node = node; + break; + } + } + + if (const pugi::xml_node overridden = override_node.child("w:lvl")) { + result = read_level(overridden, level); + } else if (const pugi::xml_node node = + level_node(abstract_numbering_(num_id, 0), level)) { + result = read_level(node, level); + } + + if (const pugi::xml_attribute start = + override_node.child("w:startOverride").attribute("w:val")) { + result.start = start.as_uint(1); + } + + return result; +} + +void resolve_list_numbering(ElementRegistry ®istry, + const NumberingRegistry &numbering, + const ElementIdentifier root_id) { + if (root_id == null_element_id) { + return; + } + + // Word keeps one set of counters per `w:numId`, independent of how the + // paragraphs nest, so no stack is needed — only document order. + std::unordered_map counters; + + const auto numbering_properties = [](const pugi::xml_node node) { + return node.child("w:pPr").child("w:numPr"); + }; + + const auto walk = [&](auto &self, const ElementIdentifier id) -> void { + for (ElementIdentifier child_id = id; child_id != null_element_id; + child_id = registry.element_at(child_id).next_sibling_id) { + const ElementRegistry::Element &element = registry.element_at(child_id); + + if (element.type == ElementType::list || + element.type == ElementType::list_item) { + const pugi::xml_node properties = numbering_properties(element.node); + const std::string num_id = + properties.child("w:numId").attribute("w:val").value(); + const auto level = + properties.child("w:ilvl").attribute("w:val").as_uint(0); + const ListLevel list_level = numbering.level(num_id, level); + const bool ordered = list_level.format != ListNumberFormat::bullet && + list_level.format != ListNumberFormat::none; + + if (element.type == ElementType::list) { + registry.set_list_type(child_id, ordered ? ListType::ordered + : ListType::unordered); + } else { + ListMarker marker; + marker.text = counters[num_id].advance(level, list_level); + if (ordered) { + marker.number = counters[num_id].number(level); + } + registry.set_list_marker(child_id, std::move(marker)); + } + } + + self(self, element.first_child_id); + } + }; + + walk(walk, registry.element_at(root_id).first_child_id); +} + +} // namespace odr::internal::ooxml::text diff --git a/src/odr/internal/ooxml/text/ooxml_text_list.hpp b/src/odr/internal/ooxml/text/ooxml_text_list.hpp new file mode 100644 index 000000000..1bc293ddf --- /dev/null +++ b/src/odr/internal/ooxml/text/ooxml_text_list.hpp @@ -0,0 +1,41 @@ +#pragma once + +#include + +#include + +#include +#include + +#include + +namespace odr::internal::ooxml::text { +class ElementRegistry; + +/// @brief `word/numbering.xml`: the list level definitions, indexed. +class NumberingRegistry final { +public: + NumberingRegistry() = default; + NumberingRegistry(pugi::xml_node numbering_root, pugi::xml_node styles_root); + + /// The definition `num_id` gives `level` (counted from 0), already resolved + /// through the abstract numbering and any override. + [[nodiscard]] ListLevel level(const std::string &num_id, + std::uint32_t level) const; + +private: + std::unordered_map m_abstract_numbering; + std::unordered_map m_numbering; + std::unordered_map m_style_numbering; + + [[nodiscard]] pugi::xml_node abstract_numbering_(const std::string &num_id, + std::uint32_t depth) const; +}; + +/// Stamps every list and item, in document order — which is what the counters +/// need. +void resolve_list_numbering(ElementRegistry ®istry, + const NumberingRegistry &numbering, + ElementIdentifier root_id); + +} // namespace odr::internal::ooxml::text diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 66be0c470..d07c04513 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -26,6 +26,7 @@ add_executable(odr_test "src/test_util.cpp" "${CMAKE_CURRENT_BINARY_DIR}/src/test_info.cpp" + "src/document_list_test.cpp" "src/document_path_test.cpp" "src/document_test.cpp" "src/file_test.cpp" @@ -45,6 +46,7 @@ add_executable(odr_test "src/internal/cfb/cfb_archive_test.cpp" + "src/internal/common/list_numbering_test.cpp" "src/internal/common/path_test.cpp" "src/internal/common/table_cursor_test.cpp" "src/internal/common/table_range_test.cpp" diff --git a/test/src/document_list_test.cpp b/test/src/document_list_test.cpp new file mode 100644 index 000000000..938a02c24 --- /dev/null +++ b/test/src/document_list_test.cpp @@ -0,0 +1,109 @@ +#include +#include +#include +#include + +#include + +#include + +#include +#include +#include +#include +#include + +using namespace odr; +using namespace odr::test; + +namespace { + +struct Marker final { + std::string text; + std::optional number; + ListType type{ListType::unordered}; +}; + +void collect_markers(const Element element, const ListType type, + std::vector &result) { + for (const Element child : element.children()) { + if (child.type() == ElementType::list) { + collect_markers(child, child.as_list().type(), result); + continue; + } + if (child.type() == ElementType::list_item) { + const ListItem list_item = child.as_list_item(); + result.push_back({list_item.marker(), list_item.number(), type}); + } + collect_markers(child, type, result); + } +} + +std::vector markers_of(const std::string &short_path) { + const Logger logger = Logger::create_stdio("odr-test", LogLevel::warning); + const DocumentFile document_file(TestData::test_file_path(short_path), + logger); + + std::vector result; + collect_markers(document_file.document().root_element(), ListType::unordered, + result); + return result; +} + +std::vector texts_of(const std::vector &markers) { + std::vector result; + result.reserve(markers.size()); + for (const Marker &marker : markers) { + result.push_back(marker.text); + } + return result; +} + +bool contains(const std::vector &markers, + const std::string &text) { + return std::ranges::find(markers, text) != std::end(markers); +} + +} // namespace + +TEST(DocumentList, odt_resolves_bullets_and_numbers) { + const std::vector markers = + markers_of("odr-public/odt/style-various-1.odt"); + + EXPECT_EQ((std::vector{"•", "•", "◦", "1.", "2.", "1."}), + texts_of(markers)); + + EXPECT_EQ(ListType::unordered, markers[0].type); + EXPECT_FALSE(markers[0].number.has_value()); + EXPECT_EQ(ListType::ordered, markers[3].type); + EXPECT_EQ(1, markers[3].number); +} + +TEST(DocumentList, docx_resolves_the_same_document_the_same_way) { + EXPECT_EQ((std::vector{"•", "•", "◦", "1.", "2.", "1."}), + texts_of(markers_of("odr-public/docx/style-various-1.docx"))); +} + +TEST(DocumentList, docx_resolves_multi_level_labels) { + const std::vector markers = + texts_of(markers_of("odr-public/docx/sample1.docx")); + + // A `w:lvlText` of "%1.%2.%3." spells the whole path out at the deep level. + EXPECT_TRUE(contains(markers, "1.1.1.")); + // Roman numerals, and a list that resumes after an interruption. + EXPECT_TRUE(contains(markers, "iii.")); +} + +TEST(DocumentList, docx_keeps_counting_across_an_interleaved_list) { + // `sample3.docx` breaks a numbered list with bullets and then goes on: Word + // counts per `w:numId`, so the numbering does not restart at 1. + const std::vector markers = + markers_of("odr-public/docx/sample3.docx"); + + EXPECT_EQ( + (std::vector{"1.", "2.", "3.", "4.", "5.", "•", "•", "6."}), + texts_of(markers)); + + EXPECT_EQ(6, markers.back().number); + EXPECT_FALSE(markers[5].number.has_value()); +} diff --git a/test/src/internal/common/list_numbering_test.cpp b/test/src/internal/common/list_numbering_test.cpp new file mode 100644 index 000000000..690f94c82 --- /dev/null +++ b/test/src/internal/common/list_numbering_test.cpp @@ -0,0 +1,114 @@ +#include + +#include + +using namespace odr::internal; + +namespace { + +ListLevel decimal_level(std::string label, const std::uint32_t start = 1) { + return {ListNumberFormat::decimal, std::move(label), start}; +} + +} // namespace + +TEST(FormatListNumber, decimal) { + EXPECT_EQ("1", format_list_number(ListNumberFormat::decimal, 1)); + EXPECT_EQ("42", format_list_number(ListNumberFormat::decimal, 42)); + EXPECT_EQ("09", format_list_number(ListNumberFormat::decimal_zero, 9)); + EXPECT_EQ("10", format_list_number(ListNumberFormat::decimal_zero, 10)); +} + +TEST(FormatListNumber, letter) { + EXPECT_EQ("a", format_list_number(ListNumberFormat::letter_lower, 1)); + EXPECT_EQ("z", format_list_number(ListNumberFormat::letter_lower, 26)); + EXPECT_EQ("aa", format_list_number(ListNumberFormat::letter_lower, 27)); + EXPECT_EQ("ab", format_list_number(ListNumberFormat::letter_lower, 28)); + EXPECT_EQ("AA", format_list_number(ListNumberFormat::letter_upper, 27)); +} + +TEST(FormatListNumber, roman) { + EXPECT_EQ("i", format_list_number(ListNumberFormat::roman_lower, 1)); + EXPECT_EQ("iv", format_list_number(ListNumberFormat::roman_lower, 4)); + EXPECT_EQ("XIV", format_list_number(ListNumberFormat::roman_upper, 14)); + EXPECT_EQ("MCMXCIV", format_list_number(ListNumberFormat::roman_upper, 1994)); + EXPECT_EQ("4000", format_list_number(ListNumberFormat::roman_upper, 4000)); +} + +TEST(FormatListNumber, without_a_number) { + EXPECT_EQ("", format_list_number(ListNumberFormat::none, 1)); + EXPECT_EQ("", format_list_number(ListNumberFormat::bullet, 1)); +} + +TEST(ListCounter, counts_up_within_a_level) { + ListCounter counter; + const ListLevel level = decimal_level("%1."); + + EXPECT_EQ("1.", counter.advance(0, level)); + EXPECT_EQ("2.", counter.advance(0, level)); + EXPECT_EQ("3.", counter.advance(0, level)); +} + +TEST(ListCounter, starts_at_the_level_start_value) { + ListCounter counter; + const ListLevel level = decimal_level("%1.", 5); + + EXPECT_EQ("5.", counter.advance(0, level)); + EXPECT_EQ("6.", counter.advance(0, level)); +} + +TEST(ListCounter, resets_deeper_levels) { + ListCounter counter; + const ListLevel outer = decimal_level("%1."); + const ListLevel inner = decimal_level("%1.%2."); + + EXPECT_EQ("1.", counter.advance(0, outer)); + EXPECT_EQ("1.1.", counter.advance(1, inner)); + EXPECT_EQ("1.2.", counter.advance(1, inner)); + EXPECT_EQ("2.", counter.advance(0, outer)); + EXPECT_EQ("2.1.", counter.advance(1, inner)); +} + +TEST(ListCounter, expands_each_level_in_its_own_format) { + ListCounter counter; + const ListLevel outer{ListNumberFormat::roman_upper, "%1.", 1}; + const ListLevel inner{ListNumberFormat::letter_lower, "%1.%2)", 1}; + + EXPECT_EQ("I.", counter.advance(0, outer)); + EXPECT_EQ("I.a)", counter.advance(1, inner)); + EXPECT_EQ("I.b)", counter.advance(1, inner)); +} + +TEST(ListCounter, treats_a_bullet_label_as_literal) { + ListCounter counter; + const ListLevel level{ListNumberFormat::bullet, "•", 1}; + + EXPECT_EQ("•", counter.advance(0, level)); + EXPECT_EQ("•", counter.advance(0, level)); +} + +TEST(ListCounter, restarts_where_told_to) { + ListCounter counter; + const ListLevel level = decimal_level("%1."); + + EXPECT_EQ("1.", counter.advance(0, level)); + EXPECT_EQ("2.", counter.advance(0, level)); + counter.restart(0, 0); + EXPECT_EQ("1.", counter.advance(0, level)); + counter.restart(0, 9); + EXPECT_EQ("10.", counter.advance(0, level)); +} + +TEST(ListCounter, keeps_a_stray_percent_literal) { + ListCounter counter; + const ListLevel level = decimal_level("%1 of 100%"); + + EXPECT_EQ("1 of 100%", counter.advance(0, level)); +} + +TEST(ListCounter, starts_deep_without_its_ancestors) { + ListCounter counter; + const ListLevel level = decimal_level("%1.%2.%3."); + + EXPECT_EQ("1.1.1.", counter.advance(2, level)); +} From 78310eaddcb403fe84274033069886756d15e426 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 11:12:41 +0200 Subject: [PATCH 3/4] feat(html): write list markers as text so a copy keeps them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bullets were drawn by `list-style`, which makes them a `::marker` pseudo-element — outside the DOM text stream, and so absent from every copied selection. Users reported bullets and numbers going missing on paste; numbers were worse off still, since every list was a `
    `. Write the label as real text instead, as an `x-s` carrying the resolved marker. It goes *inside* the item's first paragraph, not beside it — as a sibling of that block the serializer would break the line between label and text, giving "•\nOne" instead of "•\tOne". The list itself becomes `div role="list"` / `div role="listitem"` rather than `ul`/`li`. The label is document-defined text — "1.2.3.", "a)", a symbol — that no HTML list marker reproduces, so an application that draws its own marker next to ours shows both: the macOS rich-text importer behind TextEdit, Mail and Notes does exactly that, ignoring `list-style:none` however it is spelled, and turns "1. One" into "1. 1. One" on paste. Divs give it nothing to generate, and the ARIA roles keep the semantics a screen reader needs. It also matches the rest of this renderer, which already prefers `x-p`/`x-s` over semantic tags. The marker hangs into the item's padding, so a wrapped line still aligns under the text, and it grows past its 2em box rather than colliding with the text when the label is long ("1.1.1."). It carries its own font size because `x-p` collapses to `font-size:0`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017ACZ1RcX9pxoMZWBaTdDRU --- src/odr/internal/html/document_element.cpp | 46 ++++++++++++++++++---- src/odr/internal/html/document_element.hpp | 6 ++- src/odr/internal/html/frontend.cpp | 4 ++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 5a041cbc6..45528b5ef 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -301,13 +301,25 @@ void html::translate_line_break(const Element &element, } void html::translate_paragraph(const Element &element, - const WritingState &state) { + const WritingState &state, + const std::string &marker) { const Paragraph paragraph = element.as_paragraph(); state.out().write_element_begin( "x-p", HtmlElementOptions().set_inline(true).set_style( "display:block;" + translate_paragraph_style(paragraph.style()))); + if (!marker.empty()) { + state.out().write_element_begin( + "x-s", HtmlElementOptions() + .set_inline(true) + .set_class("odr-list-marker") + .set_style(translate_text_style(paragraph.text_style()))); + // The tab separates label from text once copied; `x-p` collapses to + // `font-size:0`, so the marker has to carry the item's text style itself. + state.out().out() << escape_text(marker) << " "; + state.out().write_element_end("x-s"); + } translate_children(paragraph.children(), state); if (paragraph.first_child()) { // TODO if element is content (e.g. bookmark does not count) @@ -359,9 +371,15 @@ void html::translate_bookmark(const Element &element, } void html::translate_list(const Element &element, const WritingState &state) { - state.out().write_element_begin("ul"); + // `div`s, not `ul`/`li`: an importer that draws its own marker over the one + // we write shows both, and the macOS rich-text one does exactly that whatever + // `list-style` says. The roles keep what a screen reader needs. + state.out().write_element_begin( + "div", HtmlElementOptions() + .set_class("odr-list") + .set_attributes(HtmlAttributesVector{{"role", "list"}})); translate_children(element.children(), state); - state.out().write_element_end("ul"); + state.out().write_element_end("div"); } void html::translate_list_item(const Element &element, @@ -369,10 +387,24 @@ void html::translate_list_item(const Element &element, const ListItem list_item = element.as_list_item(); state.out().write_element_begin( - "li", - HtmlElementOptions().set_style(translate_text_style(list_item.style()))); - translate_children(list_item.children(), state); - state.out().write_element_end("li"); + "div", HtmlElementOptions() + .set_class("odr-list-item") + .set_attributes(HtmlAttributesVector{{"role", "listitem"}}) + .set_style(translate_text_style(list_item.style()))); + + // Inside the first paragraph, not beside it: a sibling of that block copies + // onto a line of its own. + std::string marker = list_item.marker(); + for (const Element child : list_item.children()) { + if (!marker.empty() && child.type() == ElementType::paragraph) { + translate_paragraph(child, state, marker); + marker.clear(); + continue; + } + translate_element(child, state); + } + + state.out().write_element_end("div"); } void html::translate_table(const Element &element, const WritingState &state) { diff --git a/src/odr/internal/html/document_element.hpp b/src/odr/internal/html/document_element.hpp index f49e251b9..9b15f8714 100644 --- a/src/odr/internal/html/document_element.hpp +++ b/src/odr/internal/html/document_element.hpp @@ -1,5 +1,7 @@ #pragma once +#include + namespace odr { class Element; class ElementRange; @@ -25,7 +27,9 @@ void translate_master_page(const MasterPage &masterPage, void translate_text(const Element &element, const WritingState &state); void translate_line_break(const Element &element, const WritingState &state); -void translate_paragraph(const Element &element, const WritingState &state); +/// `marker`, when set, is written inside the paragraph ahead of its content. +void translate_paragraph(const Element &element, const WritingState &state, + const std::string &marker = ""); void translate_span(const Element &element, const WritingState &state); void translate_link(const Element &element, const WritingState &state); void translate_bookmark(const Element &element, const WritingState &state); diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index b31bc834d..3de7a2c48 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -29,6 +29,10 @@ x-s{display:inline} .odr-page-outer{display:flex;margin:0 16px;background:#fff;box-shadow:0 1px 4px rgba(0,0,0,.5);z-index:-1000} mark{background:#ff0} mark.current{background:orange} +/* The label is text rather than a `::marker`, which no selection would copy. + It hangs into the item's padding so wrapped lines align under the text. */ +.odr-list-item{padding-left:2em} +.odr-list-marker{display:inline-block;min-width:2em;margin-left:-2em;white-space:pre} )css"; /// No `text-overflow`: it sat on `td`, whose overflow is visible, and making it From e42fdaf3507fc2527816ee345a713bd17f66f95b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 9 Aug 2026 15:35:03 +0200 Subject: [PATCH 4/4] test(data): pin the reference output list markers emit Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Nk1S12YmjBsmksSJ4tVB3X --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index 47c0e8bb5..104b3fd8d 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "9e2f4c30b31d38aa4cf4bfae0b6920fd05f8563b") + REVISION "0a1114a9a50417db31cb91193b356cf56b745b8e") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "bfe777c6e3236b892dc125d61997036224208fbc") + REVISION "20a86c3be30a879ff6c4be21a246c4a17f95ff2e")