From 3bda64f3f0fb088d5499a71a470a376368e61941 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 02:45:26 +0200 Subject: [PATCH 1/2] fix: reserve viewer resources before document images --- CHANGELOG.md | 3 ++ src/odr/internal/html/document.cpp | 3 +- src/odr/internal/html/document_element.cpp | 10 ++++++ src/odr/internal/html/filesystem.cpp | 5 +-- src/odr/internal/html/frontend.cpp | 25 +++++++++++++ src/odr/internal/html/frontend.hpp | 2 ++ src/odr/internal/html/html_service.cpp | 11 ++++++ src/odr/internal/html/html_service.hpp | 6 ++++ test/src/html_test.cpp | 42 ++++++++++++++++++++++ 9 files changed, 102 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9028f624..9075f913d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Document images cannot replace viewer scripts or stylesheets at shared output + paths; colliding images are embedded instead. + - Document HTML escapes locale metadata and keeps individual views for sheets, slides and pages named `document`. diff --git a/src/odr/internal/html/document.cpp b/src/odr/internal/html/document.cpp index c4961e52c..b69bbf845 100644 --- a/src/odr/internal/html/document.cpp +++ b/src/odr/internal/html/document.cpp @@ -292,7 +292,8 @@ HtmlResources render(const Document &document, const HtmlConfig &config, const Logger &logger, HtmlWriter &out, const std::string &name, const std::optional content_pixels, Write &&write) { - HtmlResources resources; + HtmlResources resources = locate_document_resources( + config, document.document_type() == DocumentType::spreadsheet); const auto body = [&](const WritingState &state) { write_body_begin(document, state); diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 31a0f9914..ce749da2a 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -1163,6 +1163,16 @@ void html::translate_image(const Element &element, const WritingState &state) { path, image.file(), false, false, true); resource_location = state.config().resource_locator(resource, state.config()); + if (resource_location) { + const odr::HtmlResource *existing = + resource_at(state.resources(), *resource_location); + if (uri_kind(*resource_location) == UriKind::refused || + (resource_location_taken(state.resources(), *resource_location) && + (existing == nullptr || existing->is_shipped() || + existing->path() != resource.path()))) { + resource_location.reset(); + } + } } else if (loads_external_source(image.href(), state.config())) { resource = HtmlResource::create(HtmlResourceType::image, "image/jpg", "image", diff --git a/src/odr/internal/html/filesystem.cpp b/src/odr/internal/html/filesystem.cpp index ab9507a60..7d5cfc59f 100644 --- a/src/odr/internal/html/filesystem.cpp +++ b/src/odr/internal/html/filesystem.cpp @@ -168,10 +168,7 @@ class HtmlServiceImpl final : public HtmlService { if (location.has_value() && (uri_kind(*location) == UriKind::refused || util::string::equals_ignore_case(*location, listing_path) || - std::ranges::any_of(state.resources(), [&](const auto &entry) { - return entry.second && - util::string::equals_ignore_case(*entry.second, *location); - }))) { + resource_location_taken(state.resources(), *location))) { return std::nullopt; } diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index 05dda9a33..f40b5a9ee 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -367,6 +367,31 @@ void html::write_host_bridge_script(const WritingState &state) { write_script(host_bridge_js_asset, state); } +HtmlResources html::locate_document_resources(const HtmlConfig &config, + const bool spreadsheet) { + static constexpr std::array assets{document_css_asset, search_css_asset, + search_js_asset, editing_js_asset, + viewport_js_asset}; + static constexpr std::array dark{document_dark_css_asset, + search_dark_css_asset}; + HtmlResources resources = locate_all(assets, dark, config); + if (config.editable) { + locate(document_js_asset, config, resources); + } + if (spreadsheet) { + locate(spreadsheet_css_asset, config, resources); + if (writes_dark_style(config)) { + locate(spreadsheet_dark_css_asset, config, resources); + } + locate(spreadsheet_js_asset, config, resources); + if (config.editable) { + locate(sheet_editing_js_asset, config, resources); + } + } + locate_host_bridge(config, resources); + return resources; +} + HtmlResources html::locate_filesystem_resources(const HtmlConfig &config) { static constexpr std::array assets{filesystem_css_asset, search_css_asset, search_js_asset, viewport_js_asset}; diff --git a/src/odr/internal/html/frontend.hpp b/src/odr/internal/html/frontend.hpp index b30ebc1c0..c1bc2fd93 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -68,6 +68,8 @@ void write_host_bridge_script(const WritingState &state); /// What the corresponding `write_*` calls would link, without writing anything: /// a service has to answer for these paths as well as for its views. Every /// entry is located `nullopt` when the config embeds them. +HtmlResources locate_document_resources(const HtmlConfig &config, + bool spreadsheet); HtmlResources locate_filesystem_resources(const HtmlConfig &config); HtmlResources locate_text_resources(const HtmlConfig &config); HtmlResources locate_xml_resources(const HtmlConfig &config); diff --git a/src/odr/internal/html/html_service.cpp b/src/odr/internal/html/html_service.cpp index f0152ca03..ae8d4b46c 100644 --- a/src/odr/internal/html/html_service.cpp +++ b/src/odr/internal/html/html_service.cpp @@ -1,6 +1,9 @@ #include #include +#include + +#include #include @@ -89,6 +92,14 @@ void HtmlResource::write_resource(std::ostream &os) const { namespace odr::internal { +bool html::resource_location_taken(const HtmlResources &resources, + const std::string_view location) { + return std::ranges::any_of(resources, [&](const auto &entry) { + return entry.second && + util::string::equals_ignore_case(*entry.second, location); + }); +} + const odr::HtmlResource *html::resource_at(const HtmlResources &resources, const std::string &path) { for (const auto &[resource, location] : resources) { diff --git a/src/odr/internal/html/html_service.hpp b/src/odr/internal/html/html_service.hpp index 71484aa04..ad059a071 100644 --- a/src/odr/internal/html/html_service.hpp +++ b/src/odr/internal/html/html_service.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include @@ -48,6 +50,10 @@ class HtmlView : public abstract::HtmlView { const odr::HtmlResource *resource_at(const HtmlResources &resources, const std::string &path); +/// Whether a location is already used, including case-insensitive filesystems. +bool resource_location_taken(const HtmlResources &resources, + std::string_view location); + class HtmlResource : public abstract::HtmlResource { public: static odr::HtmlResource create(HtmlResourceType type, std::string mime_type, diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index 490ae5c85..bb21c0672 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -121,6 +121,48 @@ TEST(html, linked_images_are_served) { check("odr-public/xlsx/sample.xlsx"); } +TEST(html, document_images_yield_to_shipped_resources) { + for (const auto &[path, target] : std::array{ + std::pair{"odr-public/odt/image-text-wrap.odt", "search.js"}, + std::pair{"odr-public/odt/image-text-wrap.odt", "Search.JS"}, + std::pair{"odr-public/docx/file-sample_100kB.docx", + "host-bridge.js"}, + std::pair{"odr-public/xlsx/sample.xlsx", "spreadsheet.css"}}) { + SCOPED_TRACE(path); + SCOPED_TRACE(target); + HtmlConfig config; + config.embed_images = false; + config.embed_shipped_resources = false; + config.host_message_handler = "host"; + config.resource_locator = [target](const HtmlResource &resource, + const HtmlConfig &options) { + return resource.type() == HtmlResourceType::image + ? HtmlResourceLocation(target) + : html::standard_resource_locator()(resource, options); + }; + const HtmlService service = + html::translate(open(TestData::test_file_path(path)), config); + std::ostringstream page; + const HtmlResources resources = service.list_views().at(0).write_html(page); + std::size_t images = 0; + for (const auto &[resource, location] : resources) { + if (resource.type() == HtmlResourceType::image) { + ++images; + EXPECT_FALSE(location.has_value()); + } else if (resource.is_shipped() && location) { + std::ostringstream expected; + resource.write_resource(expected); + std::ostringstream served; + service.write(*location, served); + EXPECT_EQ(served.str(), expected.str()) << *location; + EXPECT_EQ(service.mimetype(*location), resource.mime_type()); + } + } + EXPECT_GT(images, 0u); + EXPECT_NE(page.str().find(R"(src="data:image/)"), std::string::npos); + } +} + TEST(html, archive_entry_yields_to_each_shipped_resource) { const DecodedFile file = open(TestData::test_file_path("odr-public/odt/about.odt"), From c6d4da99d85b26f932010f0a9c2029e873e62eaf Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 21:50:38 +0200 Subject: [PATCH 2/2] refactor(html): name the same-image case of an image location A location that this same image already holds stays shared; any other taken location embeds the image. Put the standard includes of the service after the odr includes. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- src/odr/internal/html/document_element.cpp | 8 +++++--- src/odr/internal/html/html_service.cpp | 4 ++-- src/odr/internal/html/html_service.hpp | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index ce749da2a..9d2f46029 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -1164,12 +1164,14 @@ void html::translate_image(const Element &element, const WritingState &state) { resource_location = state.config().resource_locator(resource, state.config()); if (resource_location) { + // a location taken by anything but this same image embeds instead const odr::HtmlResource *existing = resource_at(state.resources(), *resource_location); + const bool same_image = existing != nullptr && !existing->is_shipped() && + existing->path() == resource.path(); if (uri_kind(*resource_location) == UriKind::refused || - (resource_location_taken(state.resources(), *resource_location) && - (existing == nullptr || existing->is_shipped() || - existing->path() != resource.path()))) { + (!same_image && + resource_location_taken(state.resources(), *resource_location))) { resource_location.reset(); } } diff --git a/src/odr/internal/html/html_service.cpp b/src/odr/internal/html/html_service.cpp index ae8d4b46c..60d686277 100644 --- a/src/odr/internal/html/html_service.cpp +++ b/src/odr/internal/html/html_service.cpp @@ -3,10 +3,10 @@ #include #include -#include - #include +#include + namespace odr::internal::html { HtmlService::HtmlService(HtmlConfig config, const Logger &logger) diff --git a/src/odr/internal/html/html_service.hpp b/src/odr/internal/html/html_service.hpp index ad059a071..672d343d7 100644 --- a/src/odr/internal/html/html_service.hpp +++ b/src/odr/internal/html/html_service.hpp @@ -1,12 +1,12 @@ #pragma once -#include - #include #include #include +#include + namespace odr::internal::html { class HtmlService : public abstract::HtmlService {