Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion python/pyodr/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 3 additions & 2 deletions src/odr/internal/html/document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<HtmlDocumentView>(
*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) {
Expand Down Expand Up @@ -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);
}

Expand Down
46 changes: 21 additions & 25 deletions src/odr/internal/html/media_file.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -34,15 +34,13 @@ std::string source_extension(const DecodedFile &media_file) {
return "bin";
}

if (const std::optional<std::string> 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;
}
}

Expand Down Expand Up @@ -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<HtmlView>(*this, m_element, 0, m_view_path));
}
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 22 additions & 0 deletions test/src/html_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

#include <odr/html.hpp>

#include <odr/document.hpp>
#include <odr/exceptions.hpp>
#include <odr/odr.hpp>

Expand All @@ -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("<html"), std::string::npos);
std::ostringstream served;
service.write("custom.html", served);
EXPECT_EQ(served.str(), rendered.str());
}
}

// A linked stylesheet is of no use to a host serving the service over http if
// the service cannot answer for the path the markup names.
TEST(html, linked_resources_are_served) {
Expand Down
37 changes: 37 additions & 0 deletions test/src/internal/html/media_file_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,43 @@ TEST(media_file, bring_offline_writes_the_media_next_to_the_page) {
EXPECT_EQ(copied.str(), mp4_signature + "payload");
}

TEST(media_file, located_media_is_served_and_does_not_replace_the_view_or_css) {
for (const std::string location :
{"assets/clip.mp4", "VIDEO.HTML", "media.css"}) {
SCOPED_TRACE(location);
HtmlConfig config;
config.embed_shipped_resources = false;
config.resource_locator = [location](const HtmlResource &resource,
const HtmlConfig &options) {
return resource.type() == HtmlResourceType::media
? HtmlResourceLocation(location)
: html::standard_resource_locator()(resource, options);
};
const HtmlService service = html::translate(open(mp4_file()), config);
const std::string page = write_path(service, "video.html");
if (location == "assets/clip.mp4") {
EXPECT_NE(page.find("src=\"assets/clip.mp4\""), std::string::npos);
EXPECT_TRUE(service.exists(location));
EXPECT_EQ(service.mimetype(location), "video/mp4");
EXPECT_EQ(write_path(service, location), mp4_signature + "payload");
} else {
EXPECT_NE(page.find("src=\"data:video/mp4;base64,"), std::string::npos);
}
EXPECT_EQ(service.mimetype("video.html"), "text/html");
EXPECT_EQ(service.mimetype("media.css"), "text/css");
EXPECT_FALSE(service.exists("video.mp4"));
}
}

TEST(media_file, named_memory_webm_keeps_its_mime_type) {
const DecodedFile file =
open(File::from_memory("\x1a\x45\xdf\xa3payload", "clip.WEBM"));
const HtmlService service = html::translate(file, HtmlConfig());
EXPECT_EQ(service.mimetype("video.webm"), "video/webm");
EXPECT_NE(write_path(service, "video.html").find("src=\"video.webm\""),
std::string::npos);
}

/// `.mkv` and `.webm` are one file type, so the canonical name would serve a
/// webm as `video/x-matroska` and no browser would play it. The name it came
/// in under wins whenever the same type claims it.
Expand Down
Loading