From 5ba797e7a844dd376d2d77bb3fe990010fa08be7 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 2 Sep 2026 20:06:07 +0200 Subject: [PATCH] feat(html): warn on the element types the renderer drops, instead of dropping them silently `translate_element`'s `default:` carried a `// TODO log` and threw the element and its whole subtree away without a word. It now names the type through `state.logger()`, which `WritingState` has carried all along. The corpus test was handing the renderer the null logger, so nothing it reported has ever been read; it now passes one, at warning level so the pdf and svm per-operator debug output does not bury the run. Across the whole corpus that measures exactly one gap: 312 dropped `page_break` elements in 30 files, 240 in odt and 72 in doc. No other element type reaches the default case. Towards #150. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018cMYRXLJdiCkH65Jm2W9B5 --- CHANGELOG.md | 4 ++ src/odr/internal/html/document_element.cpp | 73 +++++++++++++++++++++- test/src/html_output_test.cpp | 8 ++- 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81e8fc2e7..5c6d91719 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- The HTML renderer warns, rather than silently dropping, when it reaches an + element type it has no `translate_*` for. Across the test corpus that is + `page_break` and nothing else. Towards #150. + - An ODF `draw:object` chart is drawn from the chart part's own markup, not from the replacement image beside it: bar, line, area, scatter, pie and ring, with their titles, legends, axes and series colours. An object holding no diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 7c58f0241..97359674f 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -19,6 +20,72 @@ namespace odr::internal { +namespace { + +/// Names every enumerator, with no `default`, so a type added to +/// @ref ElementType has to be named here too. +const char *element_type_name(const ElementType type) { + switch (type) { + case ElementType::none: + return "none"; + case ElementType::root: + return "root"; + case ElementType::slide: + return "slide"; + case ElementType::sheet: + return "sheet"; + case ElementType::page: + return "page"; + case ElementType::master_page: + return "master_page"; + case ElementType::sheet_cell: + return "sheet_cell"; + case ElementType::text: + return "text"; + case ElementType::line_break: + return "line_break"; + case ElementType::page_break: + return "page_break"; + case ElementType::paragraph: + return "paragraph"; + case ElementType::span: + return "span"; + case ElementType::link: + return "link"; + case ElementType::bookmark: + return "bookmark"; + case ElementType::list: + return "list"; + case ElementType::list_item: + return "list_item"; + case ElementType::table: + return "table"; + case ElementType::table_column: + return "table_column"; + case ElementType::table_row: + return "table_row"; + case ElementType::table_cell: + return "table_cell"; + case ElementType::frame: + return "frame"; + case ElementType::image: + return "image"; + case ElementType::rect: + return "rect"; + case ElementType::line: + return "line"; + case ElementType::circle: + return "circle"; + case ElementType::custom_shape: + return "custom_shape"; + case ElementType::group: + return "group"; + } + return "?"; +} + +} // namespace + void html::translate_children(const ElementRange &range, const WritingState &state) { for (const Element child : range) { @@ -78,7 +145,11 @@ void html::translate_element(const Element &element, translate_children(element.children(), state); break; default: - // TODO log + // The element and its whole subtree are dropped. Logged rather than + // thrown: a renderer shows what it can, and the corpus run is what ranks + // the gaps by how often they actually occur. + ODR_WARNING(state.logger(), "html: dropped unhandled element " + << element_type_name(element.type())); break; } } diff --git a/test/src/html_output_test.cpp b/test/src/html_output_test.cpp index ab6c203d3..82c45515e 100644 --- a/test/src/html_output_test.cpp +++ b/test/src/html_output_test.cpp @@ -194,7 +194,13 @@ TEST_P(HtmlOutputTests, html_meta) { const std::string output_path_tmp = output_path + "/tmp"; fs::create_directories(output_path_tmp); - HtmlService service = html::translate(file, output_path_tmp, config); + // The renderer used to get the null logger, so nothing it reported was ever + // read. Warnings only: its debug output is per pdf operator and per svm + // action, and would bury the corpus log. + const Logger render_logger = + Logger::create_stdio("odr-test", LogLevel::warning); + HtmlService service = + html::translate(file, output_path_tmp, config, render_logger); Html html = service.bring_offline(output_path); fs::remove_all(output_path_tmp);