Rendered markdown: relative links/images broken + prose CSS pass #182

Closed
opened 2026-09-06 22:35:23 +00:00 by crueber · 3 comments
Owner

Rendered markdown: relative links/images broken + prose CSS pass

On a rendered README (e.g. /crueber/walhub tree view): the three ![...] screenshots show as broken-image icons, and relative links don't navigate anywhere sensible.

Root cause (to verify in code)

The renderer emits src/href verbatim. Relative references (shots/a.png, docs/b.md) resolve against the SPA route URL → 404/HTML. Absolute URLs are fine.

Fix

Resolve relative references at render time against the file's own location (repo + ref + dir):

  • Images → the repo's raw-bytes endpoint at the same ref (click-to-full-size keeps working; absolute URLs untouched).
  • Links to .md/markdown files → the in-app blob view for that path at the same ref (consistent with the tabs feature).
  • Other relative links → raw URL at the same ref (or the blob view — pick one, document).
  • Anchors (#frag), absolute http(s):, mailto: → untouched.
  • Fragment links to headings within the same file (#quick-start) should scroll within the rendered view if cheap; otherwise leave as-is and note it.

Prose CSS pass

Style the rendered markdown to take advantage of the new renderer: heading scale/spacing, code blocks + inline code (both themes), tables (borders/padding/header), blockquotes, lists (incl. nested + task-list checkboxes), hr, images (max-width:100%, block margin), link colors. Both themes, no new deps.

Acceptance criteria

  • Screenshots/images in READMEs render; relative links navigate (md → blob view, others → raw).
  • Prose looks designed, not default, in dark + light.
  • node --test green (URL-resolution unit tests + existing suite); browser check of a doc-heavy README both themes, zero console errors, zero broken images.
# Rendered markdown: relative links/images broken + prose CSS pass On a rendered README (e.g. `/crueber/walhub` tree view): the three `![...]` screenshots show as broken-image icons, and relative links don't navigate anywhere sensible. ## Root cause (to verify in code) The renderer emits `src`/`href` verbatim. Relative references (`shots/a.png`, `docs/b.md`) resolve against the SPA route URL → 404/HTML. Absolute URLs are fine. ## Fix Resolve relative references at render time against the file's own location (repo + ref + dir): - **Images** → the repo's raw-bytes endpoint at the same ref (click-to-full-size keeps working; absolute URLs untouched). - **Links to `.md`/markdown files** → the in-app blob view for that path at the same ref (consistent with the tabs feature). - **Other relative links** → raw URL at the same ref (or the blob view — pick one, document). - **Anchors (`#frag`), absolute `http(s):`, `mailto:`** → untouched. - Fragment links to headings within the same file (`#quick-start`) should scroll within the rendered view if cheap; otherwise leave as-is and note it. ## Prose CSS pass Style the rendered markdown to take advantage of the new renderer: heading scale/spacing, code blocks + inline code (both themes), tables (borders/padding/header), blockquotes, lists (incl. nested + task-list checkboxes), `hr`, images (`max-width:100%`, block margin), link colors. Both themes, no new deps. ## Acceptance criteria - [ ] Screenshots/images in READMEs render; relative links navigate (md → blob view, others → raw). - [ ] Prose looks designed, not default, in dark + light. - [ ] `node --test` green (URL-resolution unit tests + existing suite); browser check of a doc-heavy README both themes, zero console errors, zero broken images.
Author
Owner

Fixed by #183 (branch fix/issue-182, against main — not merged). Relative src/href now resolve at render time against {owner, repo, ref, dir} (images/other → ?raw endpoint, .md → blob view; anchors/schemes untouched; threads pass no ctx — documented), plus the prose CSS pass both themes. Proof: node --test 378/378 green; real-Chromium pass on a live server (README both themes, md→blob nav, sub-doc dir base, release tag base, zero console errors, zero broken images). One notable find: Solid drops a ref prop on components, so the tab context prop is docRef; and urls.raw built a /raw route that never existed (404) — now the §9.5 ?raw shape.

Fixed by #183 (branch fix/issue-182, against main — not merged). Relative src/href now resolve at render time against {owner, repo, ref, dir} (images/other → ?raw endpoint, .md → blob view; anchors/schemes untouched; threads pass no ctx — documented), plus the prose CSS pass both themes. Proof: node --test 378/378 green; real-Chromium pass on a live server (README both themes, md→blob nav, sub-doc dir base, release tag base, zero console errors, zero broken images). One notable find: Solid drops a `ref` prop on components, so the tab context prop is `docRef`; and `urls.raw` built a /raw route that never existed (404) — now the §9.5 ?raw shape.
Author
Owner

Review of PR #183 (fix/issue-182, verified at 054daf3 in scratch worktree /tmp/pr183, since removed).

VERDICT: ready to merge (no browser pass per task instructions — Chromium proof of DOMPurify enforcement + prose rendering still owed by the author).

WHAT I VERIFIED

  • URL matrix (probed live with node, beyond the test file): relative/absolute/anchor/dot-segments/root-clamp/leading-slash/encoded/query/frag all behave as documented. '..' past root clamps inside the repo for both links and images (images e.g. ../../../../etc/passwd -> /o/r/api/blob/main/etc/passwd?raw). '%2e%2e' never decodes to '..' (stays a literal segment); '%20'/'%C3%A9' survive verbatim.
  • images->raw, md->blob-route, other->raw: sane + documented trade-off (non-renderables avoid the 2 MiB render-cap page). Confirmed raw shape matches the real server contract: internal/api/routes.go:62 serves /{o}/{r}/api/blob/{rev}/{path} and internal/api/blob.go:42 honors ?raw (07_api.md §9.5). Dead-raw-route claim is REAL: zero '/raw/' matches anywhere under internal/ — the old SDK urls.raw built a URL nothing served, and the Blob 'raw' pill (Blob.jsx:61) was broken too. Fix correct; repo.raw() fetch (sdk/src/repo.js:139-141) and urls.raw now agree, pinned both sides.
  • Sanitizer ordering SAFE: renderBody resolves BEFORE purify.sanitize (render-md.js). Key invariant: rewriteUrl either returns input byte-identical or a '/'-rooted same-origin URL, so output can never be javascript:/data:. javascript:/JaVaScRiPt:/data:/vbscript: pass through for the gate to drop (tests genuine). Residual hostile shapes fail safe BY CONSTRUCTION: entity-encoded 'javascript:' gets mangled into a same-origin 404 path (fragment never executes; no valid entity survives the rewrite), tab/newline-in-scheme inputs aren't parsed as links by marked at all, and control chars can't survive URL parsing to re-form a scheme (output always starts with /o/r/). One observation: double-applying resolveMarkdownUrls mangles already-resolved HTML — no such path exists (every call site renders from markdown source once), so not acting on it.
  • Threads/previews unchanged: ThreadTimeline passes optional props.mdCtx (undefined by default -> resolver returns HTML verbatim); IssueNew preview passes no ctx (class-only prose-sm -> markdown-body switch). No behavior change for relative URLs there, as documented.
  • docRef naming: legit — Solid reserves the 'ref' prop on components (ref callback), so a data prop named 'ref' would be swallowed; docRef + shortRef(t().ref) || t().sha (Tree.jsx:225) is correct. Blob uses display ref + file dir; Release {ref: tag, dir: ''}; all guarded (empty ref -> unchanged).
  • Prose CSS: every new rule scoped under .markdown-body (no global leakage; only pre-existing .tok-*/.code-view outside, untouched). h1-h6/pre+code/th+td/blockquote/hr/img/ul/ol/checkbox/:has()/nth-child(2n) all present, dark: variants throughout. prose-sm fully gone from web/src (0 matches). :has() fine for the Chromium-targeted SPA. Minor: 'table{display:block}' for scroll is a visual judgment call I could not check without a browser.
  • Deps: no package.json change; runtime stays exactly solid-js + @solidjs/router + marked@18.0.11 + dompurify@3.4.15 (D-WEB-7 clean).
  • Doc entry (12_web_ui.md #182): accurate on all call sites, the raw-endpoint mirror, and headless-vs-browser coverage split. Note: local 'main' is stale (d4d4780 vs origin/main 808ba98) — the notify/pulls test diffs in a naive main...branch diffstat belong to #179/#181, not this PR. PR-only commit is 396cae3 (+ my 054daf3 below).
  • TESTS: node --test web/test/unit/*.test.js -> 378/378 pass; vite build clean (123 modules, CSS 71.18kB / JS 424.16kB).

REVIEW FIX PUSHED (054daf3, 1 line): renderBody duplicated the marked.parse+resolveMarkdownUrls pipeline instead of calling renderMarkdownHtml(src, ctx) — behavior-identical dedup, removes drift risk between the Node-tested layer and the browser gate. Re-tested (378/378) + rebuilt after the fix.

NOT VERIFIED (waived): real-Chromium pass (DOMPurify drop enforcement, both-themes prose render, zero console errors, md->blob navigation) — author must confirm before merge.

Review of PR #183 (fix/issue-182, verified at 054daf3 in scratch worktree /tmp/pr183, since removed). VERDICT: ready to merge (no browser pass per task instructions — Chromium proof of DOMPurify enforcement + prose rendering still owed by the author). WHAT I VERIFIED - URL matrix (probed live with node, beyond the test file): relative/absolute/anchor/dot-segments/root-clamp/leading-slash/encoded/query/frag all behave as documented. '..' past root clamps inside the repo for both links and images (images e.g. ../../../../etc/passwd -> /o/r/api/blob/main/etc/passwd?raw). '%2e%2e' never decodes to '..' (stays a literal segment); '%20'/'%C3%A9' survive verbatim. - images->raw, md->blob-route, other->raw: sane + documented trade-off (non-renderables avoid the 2 MiB render-cap page). Confirmed raw shape matches the real server contract: internal/api/routes.go:62 serves /{o}/{r}/api/blob/{rev}/{path} and internal/api/blob.go:42 honors ?raw (07_api.md §9.5). Dead-raw-route claim is REAL: zero '/raw/' matches anywhere under internal/ — the old SDK urls.raw built a URL nothing served, and the Blob 'raw' pill (Blob.jsx:61) was broken too. Fix correct; repo.raw() fetch (sdk/src/repo.js:139-141) and urls.raw now agree, pinned both sides. - Sanitizer ordering SAFE: renderBody resolves BEFORE purify.sanitize (render-md.js). Key invariant: rewriteUrl either returns input byte-identical or a '/'-rooted same-origin URL, so output can never be javascript:/data:. javascript:/JaVaScRiPt:/data:/vbscript: pass through for the gate to drop (tests genuine). Residual hostile shapes fail safe BY CONSTRUCTION: entity-encoded 'javascript:' gets mangled into a same-origin 404 path (fragment never executes; no valid entity survives the rewrite), tab/newline-in-scheme inputs aren't parsed as links by marked at all, and control chars can't survive URL parsing to re-form a scheme (output always starts with /o/r/). One observation: double-applying resolveMarkdownUrls mangles already-resolved HTML — no such path exists (every call site renders from markdown source once), so not acting on it. - Threads/previews unchanged: ThreadTimeline passes optional props.mdCtx (undefined by default -> resolver returns HTML verbatim); IssueNew preview passes no ctx (class-only prose-sm -> markdown-body switch). No behavior change for relative URLs there, as documented. - docRef naming: legit — Solid reserves the 'ref' prop on components (ref callback), so a data prop named 'ref' would be swallowed; docRef + shortRef(t().ref) || t().sha (Tree.jsx:225) is correct. Blob uses display ref + file dir; Release {ref: tag, dir: ''}; all guarded (empty ref -> unchanged). - Prose CSS: every new rule scoped under .markdown-body (no global leakage; only pre-existing .tok-*/.code-view outside, untouched). h1-h6/pre+code/th+td/blockquote/hr/img/ul/ol/checkbox/:has()/nth-child(2n) all present, dark: variants throughout. prose-sm fully gone from web/src (0 matches). :has() fine for the Chromium-targeted SPA. Minor: 'table{display:block}' for scroll is a visual judgment call I could not check without a browser. - Deps: no package.json change; runtime stays exactly solid-js + @solidjs/router + marked@18.0.11 + dompurify@3.4.15 (D-WEB-7 clean). - Doc entry (12_web_ui.md #182): accurate on all call sites, the raw-endpoint mirror, and headless-vs-browser coverage split. Note: local 'main' is stale (d4d4780 vs origin/main 808ba98) — the notify/pulls test diffs in a naive main...branch diffstat belong to #179/#181, not this PR. PR-only commit is 396cae3 (+ my 054daf3 below). - TESTS: node --test web/test/unit/*.test.js -> 378/378 pass; vite build clean (123 modules, CSS 71.18kB / JS 424.16kB). REVIEW FIX PUSHED (054daf3, 1 line): renderBody duplicated the marked.parse+resolveMarkdownUrls pipeline instead of calling renderMarkdownHtml(src, ctx) — behavior-identical dedup, removes drift risk between the Node-tested layer and the browser gate. Re-tested (378/378) + rebuilt after the fix. NOT VERIFIED (waived): real-Chromium pass (DOMPurify drop enforcement, both-themes prose render, zero console errors, md->blob navigation) — author must confirm before merge.
Author
Owner

Fixed by PR #183 incl. review dedup fixup (relative URLs resolved + gated by sanitizer, dead raw route fixed, prose CSS; 378/378 node tests), merged. Closing.

Fixed by PR #183 incl. review dedup fixup (relative URLs resolved + gated by sanitizer, dead raw route fixed, prose CSS; 378/378 node tests), merged. Closing.
crueber added this to the v1 milestone 2026-09-10 22:27:15 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub#182
No description provided.