diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ce82238..0cc792f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ The release run heads these entries with the version and opens a fresh - `wasm/README.md` says what Content-Security-Policy the rendered output needs, and what each directive is for. The failures are quiet: a pdf whose `data:` fonts are blocked renders as tofu rather than falling back. +- An odf table keeps the rows and columns a grouping element holds, such as the + `` LibreOffice writes for a repeating header row. + They were dropped from the output entirely, in text documents and + spreadsheets alike. ## v6.9.0 - 2026-08-18 diff --git a/CMakeLists.txt b/CMakeLists.txt index 249c2fae..04feb219 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -163,6 +163,7 @@ set(ODR_SOURCE_FILES "src/odr/internal/odf/odf_meta.cpp" "src/odr/internal/odf/odf_parser.cpp" "src/odr/internal/odf/odf_style.cpp" + "src/odr/internal/odf/odf_table.cpp" "src/odr/internal/oldms/text/doc_document.cpp" "src/odr/internal/oldms/text/doc_element_registry.cpp" diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index 14f76fec..1660bd74 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -448,7 +449,7 @@ class ElementAdapter final : public abstract::ElementAdapter, TableDimensions result; TableCursor cursor; - for (auto row : node.children("table:table-row")) { + for (const pugi::xml_node row : table_rows(node)) { const auto rows_repeated = row.attribute("table:number-rows-repeated").as_uint(1); cursor.add_row(rows_repeated); @@ -710,7 +711,7 @@ class ElementAdapter final : public abstract::ElementAdapter, TableDimensions result; TableCursor cursor; - for (auto column : node.children("table:table-column")) { + for (const pugi::xml_node column : table_columns(node)) { const auto columns_repeated = column.attribute("table:number-columns-repeated").as_uint(1); cursor.add_column(columns_repeated); @@ -719,7 +720,7 @@ class ElementAdapter final : public abstract::ElementAdapter, result.columns = cursor.column(); cursor = {}; - for (auto row : node.children("table:table-row")) { + for (const pugi::xml_node row : table_rows(node)) { const auto rows_repeated = row.attribute("table:number-rows-repeated").as_uint(1); cursor.add_row(rows_repeated); diff --git a/src/odr/internal/odf/odf_parser.cpp b/src/odr/internal/odf/odf_parser.cpp index 02d62a74..57f9c7ac 100644 --- a/src/odr/internal/odf/odf_parser.cpp +++ b/src/odr/internal/odf/odf_parser.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -117,7 +118,7 @@ parse_table(ElementRegistry ®istry, const pugi::xml_node node) { // TODO inflate table first? - for (const pugi::xml_node column_node : node.children("table:table-column")) { + for (const pugi::xml_node column_node : table_columns(node)) { const std::uint32_t repeat = column_node.attribute("table:number-columns-repeated").as_uint(1); for (std::uint32_t i = 0; i < repeat; ++i) { @@ -127,7 +128,7 @@ parse_table(ElementRegistry ®istry, const pugi::xml_node node) { } } - for (const pugi::xml_node row_node : node.children("table:table-row")) { + for (const pugi::xml_node row_node : table_rows(node)) { // TODO log warning if repeated auto [row_id, _] = parse_any_element_tree(registry, row_node); registry.append_child(element_id, row_id); @@ -153,7 +154,7 @@ parse_sheet(ElementRegistry ®istry, const pugi::xml_node node) { TableCursor cursor; - for (const pugi::xml_node column_node : node.children("table:table-column")) { + for (const pugi::xml_node column_node : table_columns(node)) { const std::uint32_t columns_repeated = column_node.attribute("table:number-columns-repeated").as_uint(1); @@ -165,7 +166,7 @@ parse_sheet(ElementRegistry ®istry, const pugi::xml_node node) { sheet.dimensions.columns = cursor.column(); cursor = {}; - for (const pugi::xml_node row_node : node.children("table:table-row")) { + for (const pugi::xml_node row_node : table_rows(node)) { const std::uint32_t rows_repeated = row_node.attribute("table:number-rows-repeated").as_uint(1); diff --git a/src/odr/internal/odf/odf_table.cpp b/src/odr/internal/odf/odf_table.cpp new file mode 100644 index 00000000..f08d1272 --- /dev/null +++ b/src/odr/internal/odf/odf_table.cpp @@ -0,0 +1,65 @@ +#include + +#include + +#include +#include +#include +#include + +namespace odr::internal::odf { + +namespace { + +constexpr std::array row_group_names{ + std::string_view("table:table-header-rows"), + std::string_view("table:table-rows"), + std::string_view("table:table-row-group"), +}; + +constexpr std::array column_group_names{ + std::string_view("table:table-header-columns"), + std::string_view("table:table-columns"), + std::string_view("table:table-column-group"), +}; + +/// A group carries the visibility of what it holds ([ODF 1.2] 19.766). +bool is_displayed(const pugi::xml_node group) { + const pugi::xml_attribute display = group.attribute("table:display"); + return !display || display.as_bool(true); +} + +void collect(const pugi::xml_node parent, const std::string_view name, + const std::span group_names, + std::vector &out) { + for (const pugi::xml_node child : parent.children()) { + const std::string_view child_name = child.name(); + if (child_name == name) { + out.push_back(child); + } else if (std::ranges::find(group_names, child_name) != + std::end(group_names) && + is_displayed(child)) { + collect(child, name, group_names, out); + } + } +} + +} // namespace + +} // namespace odr::internal::odf + +namespace odr::internal { + +std::vector odf::table_rows(const pugi::xml_node table) { + std::vector result; + collect(table, "table:table-row", row_group_names, result); + return result; +} + +std::vector odf::table_columns(const pugi::xml_node table) { + std::vector result; + collect(table, "table:table-column", column_group_names, result); + return result; +} + +} // namespace odr::internal diff --git a/src/odr/internal/odf/odf_table.hpp b/src/odr/internal/odf/odf_table.hpp new file mode 100644 index 00000000..18c10da2 --- /dev/null +++ b/src/odr/internal/odf/odf_table.hpp @@ -0,0 +1,18 @@ +#pragma once + +#include + +namespace pugi { +class xml_node; +} // namespace pugi + +namespace odr::internal::odf { + +/// A table's rows in document order, including those a grouping element holds +/// - the `table:table-*-rows` family, which nests ([ODF 1.2] 9.1.7). +[[nodiscard]] std::vector table_rows(pugi::xml_node table); + +/// The column counterpart of `table_rows()` ([ODF 1.2] 9.1.6). +[[nodiscard]] std::vector table_columns(pugi::xml_node table); + +} // namespace odr::internal::odf diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 09157ede..8f9ac286 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -59,6 +59,8 @@ add_executable(odr_test "src/internal/svg/svg_file_test.cpp" "src/internal/xml/xml_file_test.cpp" + "src/internal/odf/odf_table_test.cpp" + "src/internal/oldms/doc_test.cpp" "src/internal/oldms/ppt_test.cpp" "src/internal/oldms/xls_test.cpp" diff --git a/test/data.cmake b/test/data.cmake index 2196cd61..05d914f3 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -22,4 +22,4 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "08b933720c39706541d2ba74f62e248d3d2a8b21") + REVISION "21f9c3f49de727acee20768d6177980577423764") diff --git a/test/src/internal/odf/odf_table_test.cpp b/test/src/internal/odf/odf_table_test.cpp new file mode 100644 index 00000000..cf3869b5 --- /dev/null +++ b/test/src/internal/odf/odf_table_test.cpp @@ -0,0 +1,219 @@ +#include + +#include +#include +#include +#include +#include + +#include +#include +#include + +#include + +#include + +#include +#include +#include +#include +#include + +using namespace odr; +using namespace odr::internal; +using namespace odr::internal::odf; + +namespace { + +std::vector names_of(const std::vector &nodes) { + std::vector result; + for (const pugi::xml_node node : nodes) { + result.emplace_back(node.attribute("id").value()); + } + return result; +} + +pugi::xml_node parse_table(pugi::xml_document &document, + const std::string &xml) { + EXPECT_TRUE(document.load_string(xml.c_str())); + return document.child("table:table"); +} + +/// An odf package is a zip with a mimetype and a `content.xml`, which keeps +/// the input to these tests a string. +std::string write_odf(const std::string &name, const std::string &mimetype, + const std::string &body) { + const std::string content = + R"()" + R"()" + R"()" + + body + R"()"; + + const std::string path = (std::filesystem::current_path() / name).string(); + + zip::ZipArchive zip; + zip.insert_file(std::end(zip), RelPath("mimetype"), + std::make_shared(mimetype)); + zip.insert_file(std::end(zip), RelPath("content.xml"), + std::make_shared(content)); + std::ofstream out(path); + zip.save(out); + + return path; +} + +void collect_text(const Element element, std::vector &out) { + if (!element) { + return; + } + if (const Text text = element.as_text()) { + out.push_back(text.content()); + } + for (const Element child : element.children()) { + collect_text(child, out); + } +} + +/// An `Element` points into the document, which has to outlive the walk. +std::vector text_of(const Document &document) { + std::vector result; + collect_text(document.root_element(), result); + return result; +} + +constexpr auto header_row_table = R"( + + + + HEADER_A + HEADER_B + + + + BODY_A + BODY_B + +)"; + +} // namespace + +TEST(OdfTable, rows_directly_under_the_table) { + pugi::xml_document document; + const pugi::xml_node table = parse_table(document, R"( + + + + )"); + + EXPECT_EQ(names_of(table_rows(table)), (std::vector{"a", "b"})); +} + +TEST(OdfTable, rows_inside_a_grouping_element) { + pugi::xml_document document; + const pugi::xml_node table = parse_table(document, R"( + + + + + + + + )"); + + EXPECT_EQ(names_of(table_rows(table)), + (std::vector{"header", "body"})); +} + +TEST(OdfTable, row_groups_nest) { + pugi::xml_document document; + const pugi::xml_node table = parse_table(document, R"( + + + + + + + + + + + )"); + + EXPECT_EQ(names_of(table_rows(table)), + (std::vector{"a", "b", "c"})); +} + +TEST(OdfTable, a_collapsed_group_is_not_shown) { + pugi::xml_document document; + const pugi::xml_node table = parse_table(document, R"( + + + + + + + + + + + )"); + + EXPECT_EQ(names_of(table_rows(table)), (std::vector{"shown"})); +} + +TEST(OdfTable, columns_inside_a_grouping_element) { + pugi::xml_document document; + const pugi::xml_node table = parse_table(document, R"( + + + + + + + + + )"); + + EXPECT_EQ(names_of(table_columns(table)), + (std::vector{"a", "b", "c"})); +} + +TEST(OdfTable, a_row_in_a_group_is_not_dropped_from_a_text_document) { + const std::string path = write_odf( + "odf_table_header_rows.odt", "application/vnd.oasis.opendocument.text", + std::string("") + header_row_table + ""); + + const Document document = odr::open(path).as_document_file().document(); + const std::vector text = text_of(document); + + EXPECT_NE(std::ranges::find(text, "HEADER_A"), std::end(text)); + EXPECT_NE(std::ranges::find(text, "BODY_A"), std::end(text)); +} + +TEST(OdfTable, a_row_in_a_group_is_not_dropped_from_a_spreadsheet) { + const std::string path = + write_odf("odf_table_header_rows.ods", + "application/vnd.oasis.opendocument.spreadsheet", + std::string("") + header_row_table + + ""); + + // a sheet's cells are off-tree, so they are read by position rather than + // walked + const Document document = odr::open(path).as_document_file().document(); + const Sheet sheet = document.root_element().first_child().as_sheet(); + ASSERT_TRUE(sheet); + + EXPECT_EQ(sheet.dimensions().rows, 2); + + std::vector header; + collect_text(sheet.cell(0, 0), header); + EXPECT_EQ(header, (std::vector{"HEADER_A"})); + + std::vector body; + collect_text(sheet.cell(0, 1), body); + EXPECT_EQ(body, (std::vector{"BODY_A"})); +}