From 9db58cff8b7b8a050bfc9fa55bf61884939e5512 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 08:48:44 +0200 Subject: [PATCH 1/5] fix(html): put a frame where its style says, and keep its image inside A frame's image is absolutely positioned against the frame, but nothing in the frame's own markup made it a containing block - that came from `*{position:relative}` in the shipped stylesheet. Read without that sheet, every image resolved against the initial containing block and covered the page at full size. The frame now states its own position. ODF also places a frame by naming a side (`style:horizontal-pos`) rather than an offset, which was ignored, so a centred image sat hard left. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT --- CHANGELOG.md | 5 ++++ src/odr/internal/html/document_style.cpp | 36 ++++++++++++++++++++---- src/odr/internal/odf/odf_style.cpp | 24 ++++++++++++++++ src/odr/style.cpp | 1 + src/odr/style.hpp | 2 ++ 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c88a0598a..0d941a8c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- 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 + image in an odt is centred. + ## v6.5.0 - 2026-08-10 - An xml file opens as xml and reads as a foldable, highlighted source view diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index 4883ae084..b506a0dd3 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -394,16 +394,26 @@ std::string html::translate_drawing_style(const GraphicStyle &graphic_style) { } std::string html::translate_frame_properties(const Frame &frame) { + const GraphicStyle style = frame.style(); + auto text_wrap = TextWrap::run_through; - if (const GraphicStyle style = frame.style(); style.text_wrap.has_value()) { + if (style.text_wrap.has_value()) { text_wrap = *style.text_wrap; } + // a side, but only where no offset already places the frame + const std::optional horizontal_position = + frame.x().has_value() ? std::nullopt : style.horizontal_position; + + // The frame says it positions itself: read without the stylesheet's + // `*{position:relative}`, its image would fill the viewport instead. std::string result; if (const AnchorType anchor_type = frame.anchor_type(); anchor_type == AnchorType::as_char) { + result += "position:relative;"; result += "display:inline-block;"; } else if (text_wrap == TextWrap::before) { + result += "position:relative;"; result += "display:block;"; result += "float:right;clear:both;"; result += "shape-outside:content-box;"; @@ -413,12 +423,15 @@ std::string html::translate_frame_properties(const Frame &frame) { if (const std::optional y = frame.y(); y.has_value()) { result += "margin-top:" + y->to_string() + ";"; } - result += "margin-right:calc(100% - "; - result += frame.x().value_or(Measure(0, DynamicUnit("in"))).to_string(); - result += " - "; - result += frame.width()->to_string(); - result += ");"; + if (const std::optional width = frame.width(); width.has_value()) { + result += "margin-right:calc(100% - "; + result += frame.x().value_or(Measure(0, DynamicUnit("in"))).to_string(); + result += " - "; + result += width->to_string(); + result += ");"; + } } else if (text_wrap == TextWrap::after) { + result += "position:relative;"; result += "display:block;"; result += "float:left;clear:both;"; result += "shape-outside:content-box;"; @@ -429,10 +442,16 @@ std::string html::translate_frame_properties(const Frame &frame) { result += "margin-top:" + y->to_string() + ";"; } } else if (text_wrap == TextWrap::none) { + result += "position:relative;"; result += "display:block;"; if (const std::optional x = frame.x(); x.has_value()) { result += "margin-left:" + x->to_string() + ";"; } + if (horizontal_position == HorizontalAlign::center) { + result += "margin-left:auto;margin-right:auto;"; + } else if (horizontal_position == HorizontalAlign::right) { + result += "margin-left:auto;"; + } if (const std::optional y = frame.y(); y.has_value()) { result += "margin-top:" + y->to_string() + ";"; } @@ -442,6 +461,11 @@ std::string html::translate_frame_properties(const Frame &frame) { if (const std::optional x = frame.x(); x.has_value()) { result += "left:" + x->to_string() + ";"; } + if (horizontal_position == HorizontalAlign::center) { + result += "left:0;right:0;margin-left:auto;margin-right:auto;"; + } else if (horizontal_position == HorizontalAlign::right) { + result += "right:0;"; + } if (const std::optional y = frame.y(); y.has_value()) { result += "top:" + y->to_string() + ";"; } diff --git a/src/odr/internal/odf/odf_style.cpp b/src/odr/internal/odf/odf_style.cpp index 7f931d948..d99e559bb 100644 --- a/src/odr/internal/odf/odf_style.cpp +++ b/src/odr/internal/odf/odf_style.cpp @@ -147,6 +147,25 @@ std::optional read_text_wrap(const pugi::xml_attribute attribute) { return {}; } +/// `from-*` names no side - the offset places those frames. +std::optional +read_horizontal_position(const pugi::xml_attribute attribute) { + if (!attribute) { + return {}; + } + const char *value = attribute.value(); + if (std::strcmp("left", value) == 0 || std::strcmp("inside", value) == 0) { + return HorizontalAlign::left; + } + if (std::strcmp("center", value) == 0) { + return HorizontalAlign::center; + } + if (std::strcmp("right", value) == 0 || std::strcmp("outside", value) == 0) { + return HorizontalAlign::right; + } + return {}; +} + std::optional read_print_orientation(const pugi::xml_attribute attribute) { if (!attribute) { @@ -510,6 +529,11 @@ void Style::resolve_graphic_style_(const pugi::xml_node node, read_text_wrap(graphic_properties.attribute("style:wrap"))) { result.text_wrap = text_wrap; } + if (const std::optional horizontal_position = + read_horizontal_position( + graphic_properties.attribute("style:horizontal-pos"))) { + result.horizontal_position = horizontal_position; + } } StyleRegistry::StyleRegistry() = default; diff --git a/src/odr/style.cpp b/src/odr/style.cpp index e1360904f..36f4ec4aa 100644 --- a/src/odr/style.cpp +++ b/src/odr/style.cpp @@ -95,6 +95,7 @@ void GraphicStyle::override(const GraphicStyle &other) { override_if_set(fill_color, other.fill_color); override_if_set(vertical_align, other.vertical_align); override_if_set(text_wrap, other.text_wrap); + override_if_set(horizontal_position, other.horizontal_position); } } // namespace odr diff --git a/src/odr/style.hpp b/src/odr/style.hpp index 5bfa15163..764f73d07 100644 --- a/src/odr/style.hpp +++ b/src/odr/style.hpp @@ -201,6 +201,8 @@ struct GraphicStyle final { std::optional fill_color; std::optional vertical_align; std::optional text_wrap; + /// The side a frame sits on; unset where its offset decides instead. + std::optional horizontal_position; void override(const GraphicStyle &other); }; From 08cc2008d4ca593b1dbed87c9a544c7089b18089 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 08:50:59 +0200 Subject: [PATCH 2/5] test: advance the reference-output pins Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index 0fa537f5a..aca251a9c 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 "40c457831835a8e9437bfe6494cb96b4c6159fab") + REVISION "26c71049d2cbdc32ca3ea982fb814bddbc90c5fe") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "d1fdace052b45f7744f24a7c6bb84a82332d37bb") + REVISION "372fb6ed9733047835ff17d6056518e6482eea3c") From b5f329a9124bc2723f5609f1d8a355960e1e5877 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 09:05:02 +0200 Subject: [PATCH 3/5] fix(odf): let a frame's own horizontal position override the inherited one `style:horizontal-pos="from-left"` names no side, so it read as nothing and left a side inherited from the parent style standing. It now assigns, so the frame's own declaration wins. No rendered output moves - the renderer already ignores a side where an offset places the frame. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT --- src/odr/internal/odf/odf_style.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/odr/internal/odf/odf_style.cpp b/src/odr/internal/odf/odf_style.cpp index d99e559bb..7f2fda60a 100644 --- a/src/odr/internal/odf/odf_style.cpp +++ b/src/odr/internal/odf/odf_style.cpp @@ -529,10 +529,11 @@ void Style::resolve_graphic_style_(const pugi::xml_node node, read_text_wrap(graphic_properties.attribute("style:wrap"))) { result.text_wrap = text_wrap; } - if (const std::optional horizontal_position = - read_horizontal_position( - graphic_properties.attribute("style:horizontal-pos"))) { - result.horizontal_position = horizontal_position; + // assigned even when it reads as none, so a `from-*` overrides an inherited + // side rather than keeping it + if (const pugi::xml_attribute attribute = + graphic_properties.attribute("style:horizontal-pos")) { + result.horizontal_position = read_horizontal_position(attribute); } } From e93e7d98a2a60606a512e4cf41eb786133e1fd49 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 09:05:03 +0200 Subject: [PATCH 4/5] fix(bindings): expose a frame's horizontal position `GraphicStyle::horizontal_position` stopped at the C++ API; the python, java and objc bindings still ended at `text_wrap`, so no binding consumer could read it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QJ7vyWPCg1oaGoQ1dirNYT --- CHANGELOG.md | 3 ++- apple/include/OdrCoreObjC/ODRStyle.h | 2 ++ apple/src/ODRStyle.mm | 1 + apple/swift/Style+Optionals.swift | 3 +++ jni/java/app/opendocument/core/GraphicStyle.java | 9 ++++++++- jni/src/jni_style.cpp | 5 +++-- python/src/bind_style.cpp | 4 +++- 7 files changed, 22 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d941a8c0..d8c488532 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,8 @@ The release run heads these entries with the version and opens a fresh - 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 - image in an odt is centred. + image in an odt is centred. The side it names is `GraphicStyle`'s new + `horizontal_position`, carried by the python, java and objc bindings. ## v6.5.0 - 2026-08-10 diff --git a/apple/include/OdrCoreObjC/ODRStyle.h b/apple/include/OdrCoreObjC/ODRStyle.h index bf95d9c65..47baa4174 100644 --- a/apple/include/OdrCoreObjC/ODRStyle.h +++ b/apple/include/OdrCoreObjC/ODRStyle.h @@ -208,6 +208,8 @@ NS_SWIFT_NAME(GraphicStyle) @property(nonatomic, readonly, nullable) NSNumber *verticalAlign; /// `ODRTextWrap`, boxed. @property(nonatomic, readonly, nullable) NSNumber *textWrap; +/// `ODRHorizontalAlign`, boxed. +@property(nonatomic, readonly, nullable) NSNumber *horizontalPosition; - (instancetype)init NS_UNAVAILABLE; + (instancetype)new NS_UNAVAILABLE; diff --git a/apple/src/ODRStyle.mm b/apple/src/ODRStyle.mm index 28d687f34..2ac9b0c4d 100644 --- a/apple/src/ODRStyle.mm +++ b/apple/src/ODRStyle.mm @@ -232,6 +232,7 @@ + (instancetype)styleWithHandle:(const odr::GraphicStyle &)handle { result->_fillColor = box(handle.fill_color); result->_verticalAlign = box_enum(handle.vertical_align); result->_textWrap = box_enum(handle.text_wrap); + result->_horizontalPosition = box_enum(handle.horizontal_position); return result; } diff --git a/apple/swift/Style+Optionals.swift b/apple/swift/Style+Optionals.swift index 7197b0c10..3e1599c41 100644 --- a/apple/swift/Style+Optionals.swift +++ b/apple/swift/Style+Optionals.swift @@ -54,6 +54,9 @@ extension GraphicStyle { public var fill: Color? { fillColor?.asColor } public var vertical: VerticalAlign? { verticalAlign?.asEnum(VerticalAlign.self) } public var wrap: TextWrap? { textWrap?.asEnum(TextWrap.self) } + public var horizontal: HorizontalAlign? { + horizontalPosition?.asEnum(HorizontalAlign.self) + } } extension PageLayout { diff --git a/jni/java/app/opendocument/core/GraphicStyle.java b/jni/java/app/opendocument/core/GraphicStyle.java index c186a1b3a..8f9a17362 100644 --- a/jni/java/app/opendocument/core/GraphicStyle.java +++ b/jni/java/app/opendocument/core/GraphicStyle.java @@ -7,13 +7,20 @@ public final class GraphicStyle { public final Color fillColor; public final VerticalAlign verticalAlign; public final TextWrap textWrap; + public final HorizontalAlign horizontalPosition; GraphicStyle( - Measure strokeWidth, Color strokeColor, Color fillColor, int verticalAlign, int textWrap) { + Measure strokeWidth, + Color strokeColor, + Color fillColor, + int verticalAlign, + int textWrap, + int horizontalPosition) { this.strokeWidth = strokeWidth; this.strokeColor = strokeColor; this.fillColor = fillColor; this.verticalAlign = VerticalAlign.fromNative(verticalAlign); this.textWrap = TextWrap.fromNative(textWrap); + this.horizontalPosition = HorizontalAlign.fromNative(horizontalPosition); } } diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index 9208ba996..84a6c6c89 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -245,10 +245,11 @@ jobject make_graphic_style(JNIEnv *env, const odr::GraphicStyle &style) { return new_object( env, "app/opendocument/core/GraphicStyle", "(Lapp/opendocument/core/Measure;Lapp/opendocument/core/Color;" - "Lapp/opendocument/core/Color;II)V", + "Lapp/opendocument/core/Color;III)V", make_measure(env, style.stroke_width), make_color(env, style.stroke_color), make_color(env, style.fill_color), - enum_code(style.vertical_align), enum_code(style.text_wrap)); + enum_code(style.vertical_align), enum_code(style.text_wrap), + enum_code(style.horizontal_position)); } jobject make_page_layout(JNIEnv *env, const odr::PageLayout &layout) { diff --git a/python/src/bind_style.cpp b/python/src/bind_style.cpp index 78698d97a..009f5edfc 100644 --- a/python/src/bind_style.cpp +++ b/python/src/bind_style.cpp @@ -175,7 +175,9 @@ void odr_python::bind_style(py::module_ &m) { .def_readwrite("stroke_color", &odr::GraphicStyle::stroke_color) .def_readwrite("fill_color", &odr::GraphicStyle::fill_color) .def_readwrite("vertical_align", &odr::GraphicStyle::vertical_align) - .def_readwrite("text_wrap", &odr::GraphicStyle::text_wrap); + .def_readwrite("text_wrap", &odr::GraphicStyle::text_wrap) + .def_readwrite("horizontal_position", + &odr::GraphicStyle::horizontal_position); py::class_(m, "PageLayout") .def(py::init<>()) From fde76ec9a73baf35ca2d65b73b51720b7b94e8fd Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 11 Aug 2026 20:15:47 +0200 Subject: [PATCH 5/5] fix(html): read a frame's horizontal position without an empty optional gcc 14 flags the nullopt branch of the ternary as a maybe-uninitialized read of the optional's payload, which -Werror turns into a build failure. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QVkpzsGS6YASMqm3NwmWib --- src/odr/internal/html/document_style.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/odr/internal/html/document_style.cpp b/src/odr/internal/html/document_style.cpp index b506a0dd3..4fa5f7dd1 100644 --- a/src/odr/internal/html/document_style.cpp +++ b/src/odr/internal/html/document_style.cpp @@ -402,8 +402,10 @@ std::string html::translate_frame_properties(const Frame &frame) { } // a side, but only where no offset already places the frame - const std::optional horizontal_position = - frame.x().has_value() ? std::nullopt : style.horizontal_position; + auto horizontal_position = HorizontalAlign::left; + if (!frame.x().has_value() && style.horizontal_position.has_value()) { + horizontal_position = *style.horizontal_position; + } // The frame says it positions itself: read without the stylesheet's // `*{position:relative}`, its image would fill the viewport instead.