From d1d62acd9b9ec4cb27298aec688da86ecbb33640 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 21:50:20 +0200 Subject: [PATCH] fix(html): give a paragraph its own font, and an empty one a line break `x-p` collapsed to `font-size:0` and an empty paragraph carried an empty span to stand in for its height. A run naming no size of its own then inherited nothing and rendered at zero - one pptx in the corpus reads as eleven blank slides. The paragraph states its font instead, which sizes the line it gives an empty one, and a run without a font of its own inherits it. What paints - a background, an underline - stays on the run, where it covers the text and not the line. An empty paragraph ends in a line break rather than a break opportunity: only a break is copied, so a blank line survives being pasted elsewhere. Emptiness is read through the wrappers a file may leave behind - a bookmark, or a span or link holding nothing - which is what the removed TODO asked for. Fixes the two TODO examples: `style-missing+image-1.odt`'s first paragraph goes from 0 to 18px, and the line break missing from `encrypted-exception-3$aabbcc$.odt` was a paragraph collapsed to 0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QVkpzsGS6YASMqm3NwmWib --- CHANGELOG.md | 7 +++ src/odr/internal/html/document_element.cpp | 55 ++++++++++++++----- src/odr/internal/html/document_style.cpp | 15 +++++ src/odr/internal/html/document_style.hpp | 2 + src/odr/internal/html/frontend.cpp | 8 +-- test/data.cmake | 4 +- .../src/internal/html/document_style_test.cpp | 21 +++++++ 7 files changed, 91 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8c488532..258ba9462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,13 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- A paragraph states its own font, so a run that names none of its own is read + at that font instead of at nothing. Slides whose text was there but invisible + now show it. +- An empty paragraph keeps the height its font implies, whether or not the file + names a size for it, and copying across one yields a blank line. +- A sheet's cells read in the font the file names, falling back to the sheet's + own only where it names none. - An image stays inside its frame on a page read without the shipped stylesheet, rather than covering the whole page. - A frame that names a side instead of an offset sits on that side, so a centred diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 45528b5ef..a17ad30a7 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -300,6 +300,36 @@ void html::translate_line_break(const Element &element, state.out().write_element_end("x-s"); } +namespace { + +/// Whether a reader sees anything. A bookmark marks a place rather than filling +/// one, and a span or a link is a style around what it holds, so a paragraph +/// holding only those is still an empty line. +bool has_content(const ElementRange &children) { + for (const Element child : children) { + switch (child.type()) { + case ElementType::bookmark: + break; + case ElementType::span: + case ElementType::link: + if (has_content(child.children())) { + return true; + } + break; + case ElementType::text: + if (!child.as_text().content().empty()) { + return true; + } + break; + default: + return true; + } + } + return false; +} + +} // namespace + void html::translate_paragraph(const Element &element, const WritingState &state, const std::string &marker) { @@ -308,34 +338,29 @@ void html::translate_paragraph(const Element &element, state.out().write_element_begin( "x-p", HtmlElementOptions().set_inline(true).set_style( - "display:block;" + translate_paragraph_style(paragraph.style()))); + "display:block;" + translate_paragraph_style(paragraph.style()) + + translate_block_font_style(paragraph.text_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. + // The tab separates label from text once copied. 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) - - // TODO example `encrypted-exception-3$aabbcc$.odt` at the very bottom - // TODO has a missing line break after "As the result of the project we ..." - - // TODO example `style-missing+image-1.odt` first paragraph has no height + if (marker.empty() && !has_content(paragraph.children())) { + // A line break, not a break opportunity: only a break is copied, so a blank + // line between two paragraphs survives being pasted somewhere else. + state.out().write_element_begin( + "br", HtmlElementOptions().set_close_type(HtmlCloseType::none)); } else { + // A paragraph whose content is all out of flow has no line box of its own. state.out().write_element_begin( - "x-s", HtmlElementOptions().set_inline(true).set_style( - translate_text_style(paragraph.text_style()))); - state.out().write_element_end("x-s"); + "wbr", HtmlElementOptions().set_close_type(HtmlCloseType::none)); } - state.out().write_element_begin( - "wbr", HtmlElementOptions().set_close_type(HtmlCloseType::none)); state.out().write_element_end("x-p"); } diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index aac996bb5..86d420719 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -194,6 +194,21 @@ std::string html::translate_text_style(const TextStyle &text_style) { return result; } +std::string html::translate_block_font_style(const TextStyle &text_style) { + std::string result; + if (const std::optional font_name = text_style.font_name; + font_name.has_value()) { + result.append("font-family:") + .append(escape_attribute(std::string(*font_name))) + .append(";"); + } + if (const std::optional font_size = text_style.font_size; + font_size.has_value()) { + result.append("font-size:").append(font_size->to_string()).append(";"); + } + return result; +} + std::string html::translate_paragraph_style(const ParagraphStyle ¶graph_style) { std::string result; diff --git a/src/odr/internal/html/document_style.hpp b/src/odr/internal/html/document_style.hpp index 640e6f6a2..e2a0245bd 100644 --- a/src/odr/internal/html/document_style.hpp +++ b/src/odr/internal/html/document_style.hpp @@ -38,6 +38,8 @@ std::string translate_outer_page_style(const PageLayout &page_layout); std::string translate_outer_flowing_page_style(const PageLayout &page_layout); std::string translate_inner_page_style(const PageLayout &page_layout); std::string translate_text_style(const TextStyle &text_style); +/// The part of a text style a block may carry: what paints belongs on the run. +std::string translate_block_font_style(const TextStyle &text_style); std::string translate_paragraph_style(const ParagraphStyle ¶graph_style); std::string translate_table_style(const TableStyle &table_style); std::string diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index b11bd8416..15036ceb1 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -23,7 +23,7 @@ body{margin:0;background:#fff} /* What the formats anchor against: a page for shapes, a paragraph or a cell for frames. */ x-p,td,.odr-page-outer{position:relative} -x-p{display:block;font-size:0} +x-p{display:block} x-s{display:inline} .odr-background{padding:0;background:#525659} /* The page column, sized to the widest page so pages of differing width centre @@ -61,9 +61,9 @@ body{margin:0;background:var(--odr-sheet-canvas)} /* The sheet's own cells, not a table the document itself drew inside one. */ .odr-sheet>tbody>tr>td{vertical-align:bottom;height:inherit;padding:1px 6px} .odr-sheet>tbody>tr>td>x-p{height:inherit} -/* Sheet-wide, not cell-only: a shape's text would otherwise fall to the - `font-size:0` a page carries. */ -.odr-sheet x-p{font-family:var(--odr-sheet-font);font-size:10pt} +/* The font anything in a cell falls back to, a shape's text included, where the + file names none of its own. */ +.odr-sheet>tbody>tr>td x-p{font-family:var(--odr-sheet-font);font-size:10pt} /* Sticky cells in a collapsed border model do not repaint their borders in Chrome or WebKit, so the ruler uses inset shadows. */ .odr-sheet th{position:sticky;background:var(--odr-sheet-ruler);color:var(--odr-sheet-ruler-text);font:500 12px/1.6 var(--odr-sheet-font);text-align:center;vertical-align:middle;padding:0 4px;white-space:nowrap;user-select:none} diff --git a/test/data.cmake b/test/data.cmake index aca251a9c..f73297899 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 "26c71049d2cbdc32ca3ea982fb814bddbc90c5fe") + REVISION "1964c34c2a6bd84d8178d9586ba59a04f172b7f2") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "372fb6ed9733047835ff17d6056518e6482eea3c") + REVISION "5e5f035f8a6e6c86ad5d978887607e8b9167c019") diff --git a/test/src/internal/html/document_style_test.cpp b/test/src/internal/html/document_style_test.cpp index 25be0f6a8..e2d10f109 100644 --- a/test/src/internal/html/document_style_test.cpp +++ b/test/src/internal/html/document_style_test.cpp @@ -35,3 +35,24 @@ TEST(html_document_style, outer_flowing_page_style_without_height) { EXPECT_EQ(ihtml::translate_outer_flowing_page_style(page_layout), "width:21cm;"); } + +TEST(html_document_style, block_font_style_carries_the_font) { + TextStyle text_style; + text_style.font_name = "Arial"; + text_style.font_size = Measure("14pt"); + EXPECT_EQ(ihtml::translate_block_font_style(text_style), + "font-family:Arial;font-size:14pt;"); +} + +TEST(html_document_style, block_font_style_leaves_what_paints_to_the_run) { + TextStyle text_style; + text_style.font_size = Measure("14pt"); + text_style.font_weight = FontWeight::bold; + text_style.background_color = Color(0xff, 0xff, 0x00); + text_style.font_underline = true; + EXPECT_EQ(ihtml::translate_block_font_style(text_style), "font-size:14pt;"); +} + +TEST(html_document_style, block_font_style_of_a_style_naming_no_font) { + EXPECT_EQ(ihtml::translate_block_font_style(TextStyle()), ""); +}