Skip to content

feat(files): render markdown in the File Viewer, with Lines/Wrap toggles - #503

Open
JDProfresh wants to merge 2 commits into
Ark0N:masterfrom
JDProfresh:feat/file-viewer-markdown
Open

JDProfresh wants to merge 2 commits into
Ark0N:masterfrom
JDProfresh:feat/file-viewer-markdown

Conversation

@JDProfresh

Copy link
Copy Markdown

Summary

Clicking a .md in the Files panel opened the File Viewer as wrapped source with an Edit pencil and no way to see it rendered, although marked + DOMPurify were already on the page for the Response Viewer. This PR renders .md/.markdown through that same pipeline, with an MD pill back to source, and gives the plain-text view Lines and Wrap toggles. It also lets .avif render inline and routes printed .avif/.ico paths to the viewer instead of tailing bytes.

What it does for the user

  • A README reads as a document: headings, tables, code blocks with the same copy buttons as the chat, images relative to the file, and links to other docs that open inside the viewer.
  • MD, Lines and Wrap remember their state per device. Lines is a real gutter that never lands in a copy.
  • Edit still works from either view and re-fetches with edit=1 as before.

Design notes

  • One markdown pipeline. The viewer calls _renderMarkdown() and binds the Response Viewer's existing click delegate on the preview body (container-bound, idempotent), so there is no second parser and no second handler.
  • Relative references. The document is built inside a <template> so nothing is fetched before the rewrite; relative images are rebased onto the workspace-confined file-raw route under the document's directory, and a failed load (CSP-blocked remote image, a 404, an SVG served as a download) degrades to alt text. Relative links become a.rv-path for the delegate and lose the target marked gave them; fragment and http(s) links are untouched. No route was widened.
  • i18n. The rendered container carries data-i18n-skip, otherwise the translator rewrites the document's prose.
  • Prefs. Per-device localStorage keys (same pattern as the Files panel's show-hidden toggle), not SettingsUpdateSchema.
  • Caps. Markdown fetches the route's 10000-line ceiling (a rendered document cut at 500 lines reads as the whole document); other text keeps 500.
  • Unchanged on purpose. Printed .md paths still open the tail viewer, per the live-follow invariant in docs/architecture-invariants.md; the rendered view is reached from the Files panel. Out-of-workspace avif/ico stay unregistrable like svg/bmp.

Testing

  • npm test gate green (433 files), plus typecheck, lint, format:check, check:frontend-syntax and check:public-assets.
  • New test/file-preview-markdown.test.ts (jsdom-in-vm, same technique as response-viewer-file-links.test.ts) pins the render, the toggle re-render without refetch, the fetch caps, the image and link rebasing, the pref persistence, toggles hidden for images and during edit, and the extension set. test/routes/file-routes.test.ts gains the avif cases.
  • Ran on a real instance: desktop Chromium via Playwright and iPhone Safari at 375px, against a README with a relative image, a CSP-blocked remote image, relative, fragment and external links, a .. link back, and an Edit round-trip.

Docs

CLAUDE.md paragraph, a section in docs/architecture-invariants.md, and the Markdown row in docs/wiki/Working-With-Files.md. No version bump or changeset, per CONTRIBUTING.

Happy to split the avif/ico bit into its own PR if you would rather keep this one to the viewer alone.

Clicking a .md in the Files panel showed wrapped source with an Edit
pencil and no way to see it rendered, although marked + DOMPurify were
already on the page for the Response Viewer. The viewer now renders
.md/.markdown through that same pipeline (one parser, one click
delegate) with an MD pill back to source, and the plain-text view gains
Lines (CSS-counter gutter) and Wrap toggles. All three persist per device
in their own localStorage keys.

- Relative images are rebased onto the workspace-confined file-raw route
  under the document's directory, built inside a <template> so no fetch
  fires before the rewrite; a failed load degrades to alt text. Relative
  links become a.rv-path so the existing delegate opens them in the
  viewer; fragment and http(s) links are untouched.
- The rendered container carries data-i18n-skip so the translator does
  not rewrite the document's prose.
- Markdown fetches the route's 10000-line ceiling; other text keeps 500.
- avif renders inline (file-content image set, file-raw MIME map), and
  avif/ico printed paths open the viewer instead of tailing bytes. .md
  deliberately stays with the tail viewer for printed paths.
@Ark0N

Ark0N commented Sep 28, 2026

Copy link
Copy Markdown
Owner

Thanks for this, @JDProfresh, and welcome. This PR makes the File Viewer render .md/.markdown files through the existing Response Viewer pipeline and gives plain text Lines and Wrap toggles. The design is right: one markdown pipeline and one click delegate, no widened routes, per-device prefs, edit mode left alone. The docs and tests are thorough too.

Two things need fixing before merge, both small:

1. Encoded relative paths get encoded twice (src/web/public/panels-ui.js:4381)

marked percent-encodes link and image destinations. So ![](<my image.png>) arrives as src="my%20image.png", and ![](图片/截图.png) arrives as %E5%9B%BE.... resolveRef() keeps that string and the caller runs encodeURIComponent on it again, so file-raw looks for a file literally named my%20image.png and returns 404.

Links break the same way: [Getting Started](<Getting Started.md>) opens docs/Getting%20Started.md with "Failed to load file". A ?raw=true suffix also ends up in the path.

Please strip ?... along with #... and run decodeURIComponent on the ref before resolving segments, keeping the raw ref on a malformed escape:

let rel = ref.split('#')[0].split('?')[0];
try {
  rel = decodeURIComponent(rel);
} catch {
  /* malformed escape: keep the ref as written */
}
for (const seg of (dir + rel).split('/')) {

Then add a %20 image, a percent-encoded CJK link and a ?raw=true image to MARKDOWN_HTML in test/file-preview-markdown.test.ts.

2. A markdown file can break every inline app.* button (src/web/public/sanitize-html.js:96)

The sanitizer allowlists name, so <img name="app" src="x"> survives and makes document.app that image. Inline onclick="app.…()" handlers look up names on document first. While such a document is on screen, the MD pill, Edit, Copy and the close button all throw, and Escape does not close the viewer, so a reload is the only way out.

This hole is older than your PR (the Response Viewer has it too). Rendering repo READMEs is what makes it reachable from any cloned repository. Please drop 'name' from ALLOWED_ATTR (marked never emits it) and add a case to test/markdown-sanitizer.test.ts.

Smaller things that fit in the same pass:

  • src/web/public/panels-ui.js:4400: relative links open with this.activeSessionId (the delegate at src/web/public/app.js:2361), while images use the preview's own sessionId. A preview opened from another session's attachment card therefore resolves links against the wrong workspace. Set a.dataset.sessionId in the rebase and have the delegate prefer it.
  • src/web/public/panels-ui.js:4374: root-relative refs such as /docs/x.png are left as-is, so they 404 or open a Codeman URL in a new tab. Resolving them against the workspace root matches GitHub.
  • src/web/public/i18n.js: zh-CN entries for 'Rendered markdown', 'Line numbers' and 'Wrap lines', next to 'Edit file'.

No need to split out the avif/ico change. It is small and tested, so it stays here.

Once 1 and 2 are in, this is ready to merge.

… drop name= from the sanitizer

Review follow-up on Ark0N#503. marked percent-encodes link and image destinations, and the rebase pass encoded them a second time, so a space or a CJK character in a file name made file-raw look for a file literally named my%20image.png; refs are now decoded once (a malformed escape is kept as written) and stripped of ?query along with #fragment. Root-relative refs resolve from the workspace root as on GitHub instead of falling through as Codeman URLs. Rebased links carry the preview's own session id and the response-viewer delegate prefers it, so a document opened from another session's attachment card opens its links in that workspace rather than the active tab's.

The sanitizer no longer allows name=: marked never emits it, and <img name="app"> made document.app that image, which every inline onclick="app.…()" handler resolves before the global, so one rendered README broke every viewer button until a reload. Adds the zh-CN strings for the three toolbar titles.
@JDProfresh

Copy link
Copy Markdown
Author

Thanks for the review. All five are in 612c69d:

  • Refs are decoded once and stripped of ?query along with #fragment before the route encodes them, so ![](<my image.png>) and a CJK path resolve to the real file. A malformed escape keeps the ref as written.
  • name is out of ALLOWED_ATTR, with a clobbering case in test/markdown-sanitizer.test.ts (<img name="app"> and <a name="app"> both lose the attribute, the src and href survive).
  • Rebased links carry data-session-id and the delegate in app.js prefers it over activeSessionId.
  • Root-relative refs (/docs/x.png, /README.md) resolve from the workspace root; protocol-relative //host stays remote.
  • zh-CN strings for the three toolbar titles.

test/file-preview-markdown.test.ts gained a %20 image, a ?raw=true image, a malformed escape, a root-relative pair, a percent-encoded CJK link and the session-id assertion. The invariants doc, CLAUDE.md paragraph and the wiki line describe the decode step and the root-relative rule. Checked live against a fixture repo as well: the encoded and root-relative images load, the CJK link opens through the real delegate, and the buttons keep working with an <img name="app"> on screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants