From 75527dd092d08a68e355b6b06da90b4a983f2d88 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 22:06:00 +0200 Subject: [PATCH] fix: honor HTML output paths and media resource locations --- CHANGELOG.md | 5 +++ python/pyodr/cli.py | 2 +- src/odr/internal/html/document.cpp | 5 ++- src/odr/internal/html/media_file.cpp | 46 ++++++++++------------ test/src/html_test.cpp | 22 +++++++++++ test/src/internal/html/media_file_test.cpp | 37 +++++++++++++++++ 6 files changed, 89 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08e16fffe..8ba7df6d9 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 +- HTML document views honor their configured output filename. Audio and video + resources are served at custom locator paths without replacing the player + or stylesheet, and named in-memory WebM files retain their playable MIME type. + The Python CLI correctly opens output paths containing URL-special characters. + - Spreadsheet previews retain ODS cells at window boundaries, clip repeated and merged cells, and trim XLSX output to populated cells inside the window. diff --git a/python/pyodr/cli.py b/python/pyodr/cli.py index e74e37739..581d330c1 100644 --- a/python/pyodr/cli.py +++ b/python/pyodr/cli.py @@ -49,7 +49,7 @@ def _translate(args, file) -> int: for page in html.pages(): print(f"{page.name}: {page.path}") if not args.no_open: - webbrowser.open(f"file://{page.path}") + webbrowser.open(Path(page.path).resolve().as_uri()) return 0 diff --git a/src/odr/internal/html/document.cpp b/src/odr/internal/html/document.cpp index b69bbf845..47faa1d63 100644 --- a/src/odr/internal/html/document.cpp +++ b/src/odr/internal/html/document.cpp @@ -461,7 +461,8 @@ class HtmlServiceImpl final : public HtmlService { : HtmlService(std::move(config), logger), m_document{std::move(document)}, m_fragments{std::move(fragments)} { m_views.emplace_back(std::make_shared( - *this, "document", 0, "document.html", m_fragments)); + *this, "document", 0, this->config().document_output_file_name, + m_fragments)); // the one fragment of a text document is the document view itself if (m_document.document_type() != DocumentType::text) { for (const auto &fragment : m_fragments) { @@ -538,7 +539,7 @@ class HtmlServiceImpl final : public HtmlService { HtmlResources write_html(const std::string &path, HtmlWriter &out) const override { - if (path == "document.html") { + if (path == config().document_output_file_name) { return write_document(out); } diff --git a/src/odr/internal/html/media_file.cpp b/src/odr/internal/html/media_file.cpp index 86479e821..09108cf5f 100644 --- a/src/odr/internal/html/media_file.cpp +++ b/src/odr/internal/html/media_file.cpp @@ -23,7 +23,7 @@ namespace odr::internal::html { namespace { /// The extension the media goes out under: the file type's canonical one, -/// unless the file came from disk named with another extension the same type +/// unless the file was named with another extension the same type /// claims. `.mkv` and `.webm` are one type here - the two are the same bytes /// down to the EBML DocType, deeper than a signature reaches - and a browser /// handed a webm under the matroska name and MIME will not play it. @@ -34,15 +34,13 @@ std::string source_extension(const DecodedFile &media_file) { return "bin"; } - if (const std::optional path = media_file.file().disk_path(); - path.has_value()) { - std::string extension = std::filesystem::path(*path).extension().string(); - if (!extension.empty()) { - extension.erase(0, 1); // the dot - extension = util::string::to_lower(extension); - if (std::ranges::find(extensions, extension) != extensions.end()) { - return extension; - } + std::string extension = + std::filesystem::path(media_file.file().name()).extension().string(); + if (!extension.empty()) { + extension.erase(0, 1); // the dot + extension = util::string::to_lower(extension); + if (std::ranges::find(extensions, extension) != extensions.end()) { + return extension; } } @@ -94,6 +92,16 @@ class HtmlServiceImpl final : public HtmlService { m_source_path{m_element + "." + m_extension}, m_mime_type{mime_type_for(m_media_file.file_type(), m_extension)}, m_resources{locate_media_resources(this->config())} { + const odr::HtmlResource resource = HtmlResource::create( + HtmlResourceType::media, m_mime_type, m_source_path, m_source_path, + m_media_file.file(), false, false, true); + HtmlResourceLocation location = + this->config().resource_locator(resource, this->config()); + if (location && (util::string::equals_ignore_case(*location, m_view_path) || + resource_location_taken(m_resources, *location))) { + location.reset(); + } + m_resources.emplace_back(resource, std::move(location)); m_views.emplace_back( std::make_shared(*this, m_element, 0, m_view_path)); } @@ -103,17 +111,13 @@ class HtmlServiceImpl final : public HtmlService { [[nodiscard]] const HtmlViews &list_views() const override { return m_views; } [[nodiscard]] bool exists(const std::string &path) const override { - return path == m_view_path || path == m_source_path || - resource_at(m_resources, path) != nullptr; + return path == m_view_path || resource_at(m_resources, path) != nullptr; } [[nodiscard]] std::string mimetype(const std::string &path) const override { if (path == m_view_path) { return "text/html"; } - if (path == m_source_path) { - return m_mime_type; - } if (const odr::HtmlResource *resource = resource_at(m_resources, path); resource != nullptr) { return resource->mime_type(); @@ -128,10 +132,6 @@ class HtmlServiceImpl final : public HtmlService { write_media(writer); return; } - if (path == m_source_path) { - m_media_file.file().pipe(out); - return; - } if (const odr::HtmlResource *resource = resource_at(m_resources, path); resource != nullptr) { resource->write_resource(out); @@ -157,11 +157,7 @@ class HtmlServiceImpl final : public HtmlService { // The media stays a resource rather than a data URI: a video is regularly // larger than everything else we emit put together, and base64 in the // markup would cost a third on top of it again. - const odr::HtmlResource resource = HtmlResource::create( - HtmlResourceType::media, m_mime_type, m_source_path, m_source_path, - m_media_file.file(), false, false, true); - const HtmlResourceLocation location = - config().resource_locator(resource, config()); + const auto &[resource, location] = m_resources.back(); resources.emplace_back(resource, location); out.write_begin(); @@ -206,7 +202,7 @@ class HtmlServiceImpl final : public HtmlService { std::string m_extension; std::string m_source_path; std::string m_mime_type; - /// The css this view links; empty of locations when the config embeds it. + /// Shipped resources followed by the media, at their located paths. HtmlResources m_resources; HtmlViews m_views; diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index b42d7b2bd..d68967fe6 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -12,6 +12,7 @@ #include +#include #include #include @@ -36,6 +37,27 @@ TEST(html, empty_handles_throw_on_access) { EXPECT_THROW(HtmlResource(nullptr), NullPointerError); } +TEST(html, document_view_uses_the_configured_output_name) { + for (const FileType type : + {FileType::opendocument_text, FileType::opendocument_spreadsheet}) { + HtmlConfig config; + config.document_output_file_name = "custom.html"; + const HtmlService service = html::translate(create_document(type), config); + const HtmlView &view = service.list_views().front(); + EXPECT_EQ(view.path(), "custom.html"); + EXPECT_TRUE(service.exists("custom.html")); + EXPECT_FALSE(service.exists("document.html")); + EXPECT_EQ(service.mimetype("custom.html"), "text/html"); + + std::ostringstream rendered; + view.write_html(rendered); + EXPECT_NE(rendered.str().find("