Skip to content

fix: serve documentation assets inline so SVG images render #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
barredterra merged 1 commit into from
Aug 16, 2026
Merged

fix: serve documentation assets inline so SVG images render #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions compendium/docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,13 @@ def get_asset(page_path: str, asset_path: str, locale: str | None = None):
with open(asset_file, "rb") as f:
content = f.read()

frappe.response["type"] = "binary"
frappe.response["filename"] = os.path.basename(asset_file)
filename = os.path.basename(asset_file)
# as_raw (type "download") sets the MIME type from the filename. type "binary"
# is always application/octet-stream, which browsers refuse to render as SVG.
frappe.response["type"] = "download"
frappe.response["filename"] = filename
frappe.response["filecontent"] = content
frappe.response["display_content_as"] = "inline"


def get_documented_locale_infos(raw_pages=None):
Expand Down
15 changes: 15 additions & 0 deletions compendium/tests/test_docs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from compendium.docs import (
build_navigation_tree,
discover_pages,
get_asset,
get_first_page,
get_locales,
get_page,
Expand Down Expand Up @@ -173,6 +174,20 @@ def test_protected_assets_require_page_access(self):
asset_file = resolve_asset_path(page, "logo.png")
self.assertTrue(asset_file.endswith("logo.png"))

def test_get_asset_serves_svg_inline(self):
svg = b'<svg xmlns="http://www.w3.org/2000/svg"></svg>'
with self.docs_environment(
{
"en/page.md": "---\ntitle: Page\n---\n![Diagram](diagram.svg)",
"en/diagram.svg": svg,
}
):
get_asset("page", "diagram.svg", locale="en")
self.assertEqual(frappe.response["type"], "download")
self.assertEqual(frappe.response["display_content_as"], "inline")
self.assertEqual(frappe.response["filename"], "diagram.svg")
self.assertEqual(frappe.response["filecontent"], svg)
Comment on lines +184 to +189

@greptile-apps greptile-apps Bot Aug 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test skips HTTP response conversion

This regression test inspects only the intermediate frappe.response dictionary, so it will continue to pass when Frappe fails to translate that metadata into the MIME type and inline Content-Disposition required for SVG rendering. Exercise the response conversion and assert the resulting HTTP headers to cover the behavior this change fixes.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: compendium/tests/test_docs.py
Line: 184-189

Comment:
**Test skips HTTP response conversion**

This regression test inspects only the intermediate `frappe.response` dictionary, so it will continue to pass when Frappe fails to translate that metadata into the MIME type and inline Content-Disposition required for SVG rendering. Exercise the response conversion and assert the resulting HTTP headers to cover the behavior this change fixes.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Cursor


def test_get_first_page_and_tree_ordering(self):
with self.docs_environment(
{
Expand Down
Loading