Skip to content

fix(studio): resolve project-root-relative asset URLs in preview iframe#1698

Merged
jrusso1020 merged 2 commits into
mainfrom
fix/studio-asset-path-resolution-from-root
Jun 24, 2026
Merged

fix(studio): resolve project-root-relative asset URLs in preview iframe#1698
jrusso1020 merged 2 commits into
mainfrom
fix/studio-asset-path-resolution-from-root

Conversation

@jrusso1020

Copy link
Copy Markdown
Collaborator

What

Studio preview now resolves <video src="../../assets/x.mp4"> (and the
same shape for <img>, <audio>, inline style url(), and <style>
CSS url()) against the sub-composition's URL — matching what the
server-side bundler already does for the render path.

Why

Authored compositions live at compositions/frames/*.html and reference
project-root assets either as plain assets/x.mp4 (already correct
because the main document's <base href> points at the project preview
root) or as ../../assets/x.mp4 (the explicit project-root-relative
form). The server-side inlineSubCompositions flattens sub-comps into
index.html and rewrites the ../-form against the sub-comp's source
path so it resolves against the project root in the baked render.

The browser-side runtime that mounts external sub-compositions via
fetch did no such rewriting. So <video src="../../assets/x.mp4">
authored inside a compositions/frames/scene.html resolved against the
main document's base href, climbed above the project root, and 404'd in
Studio preview — even though the same path rendered correctly in the
final video. An OSS user (Miao Yang) hit this in a real project.

How

Added rewriteSubCompositionAssetPaths to the runtime
compositionLoader. After parsing the fetched sub-composition HTML and
before extracting any nodes, walk the parsed document and rewrite the
same surface the server-side path touches:

  • [src] and [href] attributes on every element
  • [style] attribute url(...) references
  • <style> element CSS url(...) references

The rewrite mirrors the producer's semantics exactly: only values that
start with ../ (or are literal ..) are rewritten — against the
sub-composition's URL via new URL(value, compositionUrl). Absolute
URLs, root-relative paths, data:, hash refs, and plain
assets/x.mp4 are left untouched. Plain relative paths must not be
rewritten
because the main document's <base href> already covers
them; rewriting would double-prefix the URL.

The walk recurses into <template> content because authored
compositions typically wrap their rendered body in a <template> and
querySelectorAll does not enter template content (it lives in a
detached DocumentFragment).

Test plan

  • Unit tests added (6 new tests in
    compositionLoader.test.ts): rewrites ../-traversing src on
    template-wrapped sub-comps; leaves plain relative paths untouched (no
    double-prefix); leaves absolute / data / hash / root-relative URLs
    untouched; rewrites CSS url() in <style> blocks and inline
    style attributes; rewrites for non-template (full-HTML-doc)
    sub-comps.
  • Full core test suite green (2065 tests).
  • Full studio test suite green (1148 tests).
  • Manual verification with the reporter's actual project:
    before the fix one <video> with a ../../assets/... src returned
    MEDIA_ELEMENT_ERROR: Format error; after the fix all 7 <video>
    elements load (readyState=4, correct currentSrc). The 6 plain
    assets/... paths are unchanged (no double-prefix) and continue
    to resolve via <base href> as before.
  • bun run lint, bun run format:check, bun run typecheck,
    fallow audit all green.

Reported by Miao Yang.

— Jerrai (https://claude.com/claude-code)

…nder

The server-side bundler (`inlineSubCompositions`) rewrites `../`-traversing
asset paths inside sub-compositions against the sub-comp's source path so
they resolve against the project root in the baked render. The browser-side
runtime that mounts external sub-compositions via `fetch` did no such
rewriting — so an authored `<video src="../../assets/x.mp4">` inside a
`compositions/frames/scene.html` resolved against the main document's base
href (the project preview root) and climbed above it, producing a 404 in
Studio preview while the render output worked correctly.

Apply the same `../`-only rewrite at runtime, against the sub-composition's
URL, covering `[src]` / `[href]` attributes, `[style]` url(...) references,
and `<style>` element CSS — the same surface the server-side path touches.
Plain project-root-relative paths (`assets/x.mp4`) are intentionally left
untouched: the main document's `<base href>` already covers them, and
rewriting would double-prefix.

The walk recurses into `<template>` content because authored compositions
typically wrap their rendered body in a `<template>` and querySelectorAll
does not enter template content (it lives in a detached DocumentFragment).

Reported-By: Miao Yang

— Jerrai

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: fix(studio): resolve project-root-relative asset URLs in preview iframe

Solid fix for a real render-vs-preview divergence. The diagnosis is spot-on and the implementation mirrors the server-side path cleanly.

What I checked

  1. Rewrite scope is correct — Only ../-traversing paths get rewritten. Plain relative (assets/x.mp4) are untouched because <base href> already covers them. Absolute URLs, data:, hash refs, root-relative, and protocol-relative all skip via isNonRelativeRuntimeUrl. This avoids the double-prefix trap.

  2. CSS surface covered<style> element text AND inline style attribute url() references both handled via CSS_URL_RE. The regex preserves the original quote style (single, double, or bare) via backreference — no cosmetic mutations.

  3. Template recursionquerySelectorAll doesn't enter <template> content DocumentFragments, so the explicit recursion into templateEl.content is essential. Without it, every asset ref inside the canonical template wrapper (which is the majority of authored compositions) would be missed. Correct.

  4. Graceful failurerewriteRuntimeAssetPath catches URL construction errors, rewriteSubCompositionAssetPaths early-returns on null compositionUrl. No crash path.

  5. Placement — The rewrite happens after DOMParser.parseFromString and before node extraction. Right timing — paths are rewritten before anything gets mounted into the live document.

  6. Test coverage — 6 tests covering the full contract: ../-traversing src on template-wrapped sub-comps, plain paths untouched (no double-prefix), absolute/data/hash/root-relative untouched, CSS url() in <style> blocks, inline style url(), and non-template full-HTML-doc sub-comps. All the important edge cases.

  7. CI — Nearly all green, just Analyze (JS/TS) and Windows tests still pending. Core test suite (2065), Studio tests (1148), all passed.

One observation (non-blocking)

The CSS_URL_RE regex (/\burl\(\s*(["']?)([^)"']+)\1\s*\)/g) won't handle CSS values containing escaped parentheses or quotes (e.g. url("path with \) parens")). Not a concern for real asset paths — just noting the boundary.

LGTM — ship it.

Review by Miga

…sal in asset paths

The old `invalid_capture_path` rule caught only `../capture/` in raw source.
The same root cause applies to every `../`-traversing asset path inside a
sub-composition — they all climb above the project root and 404 in Studio
preview, because compositions are served with the project root as their base
URL. The bundler rewrites these correctly for the baked render (and the
runtime now mirrors that fallback in `rewriteSubCompositionAssetPaths`), but
the authoring-time signal is still wrong and worth flagging.

Replace the old rule with `invalid_parent_traversal_in_asset_path`. The new
rule walks parsed tag attributes (`src`, `href`, inline `style` url()) and
the extracted `<style>` blocks' url() references, and flags any value that
starts with `../`. The check covers the same surface the runtime fallback
touches, so the lint catches exactly what the runtime would otherwise
silently rewrite at preview time.

Subsumes the older rule (every `../capture/` value still fires under the new
code), with broader coverage for `../assets/`, `../../assets/`, `../fonts/`,
`../voice/`, and other per-traversal asset directories. Plain relative paths,
absolute URLs, `data:` URIs, root-relative paths, and hash anchors are
intentionally left untouched.

— Jerrai
@jrusso1020

Copy link
Copy Markdown
Collaborator Author

Added generalized lint rule per James's request — invalid_capture_path is now invalid_parent_traversal_in_asset_path and catches any ../ traversal in src / href / inline style url() / <style> url() references. Subsumes the old rule (every ../capture/ case still fires under the new code) and adds per-traversal coverage for ../assets/, ../../assets/, ../fonts/, etc. Walks parsed tag attributes + extracted <style> blocks instead of the old raw-source regex, so the surface matches the runtime fallback exactly. Tests added — 16 cases (8 positives + 8 negatives + 1 pinning that the old rule code is gone). All 74 composition lint tests green, full core suite (2077 tests) green, repo lint+format+typecheck green.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verified current head d7c8fb74 after the lint-rule follow-up.

Audited: packages/core/src/lint/rules/composition.ts, packages/core/src/lint/rules/composition.test.ts, and the runtime rewrite integration in packages/core/src/runtime/compositionLoader.ts / compositionLoader.test.ts.

The generalized invalid_parent_traversal_in_asset_path rule now covers the same surface the runtime fallback rewrites: src, href, inline style url(), and <style> block url(). It preserves the intended skips for absolute, root-relative, hash, data URI, plain assets/..., registry source, and installed registry cases. Runtime rewrite still mirrors the render bundler path and recurses through <template> content.

Validation: GitHub checks are green at head, including build/typecheck/test/lint, Windows tests/render, perf, preview, and all regression shards. Local focused lint-rule check also passed: bunx vitest run src/lint/rules/composition.test.ts --environment node (74/74). I could not run the runtime test locally because this checkout's Bun/jsdom setup fails before collection on an unrelated html-encoding-sniffer CJS/ESM interop issue; CI covered it successfully.

Verdict: APPROVE
Reasoning: The runtime fix plus generalized author-time lint closes the preview/render asset-path divergence, and the expanded tests/CI cover the affected surfaces.

— Magi

@jrusso1020 jrusso1020 merged commit c7b9bf3 into main Jun 24, 2026
47 checks passed
@jrusso1020 jrusso1020 deleted the fix/studio-asset-path-resolution-from-root branch June 24, 2026 14:48
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.

3 participants