fix(web,daemon): preview snapshot bridge captured every real page as blank (empty-render) - #7125
Open
huynextlevel wants to merge 1 commit into
Open
Conversation
…blank
Two stacked bugs in the foreignObject snapshot bridge (the srcDoc bridge
in apps/web/src/runtime/srcdoc.ts and the URL-preview bridge the daemon
injects in apps/daemon/src/routes/project/index.ts share this code):
1. inlineSnapshotStyles stripped clone <script>/<link> nodes BEFORE
pruneHiddenSnapshotNodes ran, but prune pairs the original/clone
querySelectorAll('*') lists by index. The early removals shifted every
later clone under the wrong original, so the misdirected hidden-node
removals deleted visible content (often the <body> itself). Any page
containing a script or stylesheet link rasterized as a uniform frame
and the capture failed with 'empty-render' ("Preview is still
loading. Try again in a moment."). The stripping now runs after prune
(stripSnapshotResources).
2. bodyContent was serialized with innerHTML, whose HTML serialization
emits void elements (<br>, <img>) without self-closing slashes -
invalid XML inside <foreignObject>, so once bug 1 is fixed the SVG
image fails to parse ('snapshot image failed'). Now serialized via
XMLSerializer (serializeSnapshotXhtml), and XML-invalid attribute
names (@click, :href) are dropped from the clone.
Desktop is unaffected because the host compositor path short-circuits
the bridge; every pure-web deployment (browser against the daemon, e.g.
the Docker/K8s setup in nexu-io#5444) always takes it and has been broken.
Fixes nexu-io#5444
lefarcen
added
size/M
2 tasks
Contributor
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable). |
mrcfps
approved these changes
Aug 19, 2026
mrcfps
left a comment
mrcfps
left a comment
Contributor
There was a problem hiding this comment.
@huynextlevel thanks for the careful diagnosis and fix on #5444 — this is a really solid bugfix.
I reviewed the changed ranges in apps/web/src/runtime/srcdoc.ts, the matching daemon URL-preview bridge in apps/daemon/src/routes/project/index.ts, and the two regression specs in apps/web/tests/runtime/srcdoc.test.ts.
What I verified
- Root cause 1 is real:
pruneHiddenSnapshotNodesstill pairs original/clonequerySelectorAll('*')lists by index, so stripping scripts/links before prune misaligns the clone and can delete visible content (includingbody). Moving that work intostripSnapshotResourcesand calling it only after prune is the right ordering fix. - Root cause 2 is real:
innerHTMLis an HTML serializer and can emit non-well-formed void tags inside SVGforeignObject;serializeSnapshotXhtmlviaXMLSerializer, plus dropping XML-invalid attribute names, is the correct follow-on fix once content survives prune. - Both bridge copies stay byte-identical for the new helpers, and both capture paths now run
inlineSnapshotStyles→pruneHiddenSnapshotNodes→stripSnapshotResources→serializeSnapshotXhtml. - The new tests lock the call order and the XHTML serialization path in the same style as the existing srcdoc bridge suite.
No actionable correctness, safety, or maintainability issues in the changed ranges. Nice work tracking this through the empty-render signature all the way to the index-alignment bug.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
Contributor
Visual regression reviewHead: 0 changed · 49 unchanged · 0 new without baseline · 0 failed Unchanged cases
Visual diff is advisory only and does not block merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5444
Why
I run Open Design in a browser-only deployment (web app + daemon, no desktop shell) and hit exactly what #5444 reports: every click on the preview's screenshot capture immediately fails with "Preview is still loading. Try again in a moment.", and Mark-mode annotations lose their screenshot — on every real page, while the preview itself renders fine.
Desktop never sees this because
captureHostIframeSnapshot(the Electron compositor path) short-circuits the capture; pure-web deployments always fall through to the SVG<foreignObject>snapshot bridge, which turns out to have two stacked bugs. The maintainer's diagnosis in #5444 (the network dump showing an SVG with an empty<foreignObject><div>shell) is precisely bug 1's signature.What happens (root cause)
Bug 1 — the prune step deletes the page content.
inlineSnapshotStylesstrips<script>/<link>nodes from the clone, and thenpruneHiddenSnapshotNodesruns — but prune pairs the original and clonequerySelectorAll('*')lists by index. The earlier removals shift every later clone under the wrong original, so the hidden-node verdicts land on the wrong nodes and the removals delete visible content. Instrumented on a real page: original list 782 elements vs clone 776 after stripping → the misaligned prune removed the clone's<body>itself, so the serialized SVG was a 1.6 KB empty wrapper and the canvas came back uniform →empty-render. Any page with at least one script or stylesheet link (i.e. every real artifact) is affected; a trivial static page with neither stays aligned, which is why this survived so long.Bug 2 — innerHTML is not valid XML. With bug 1 fixed, the next failure surfaces:
bodyContentis serialized withinnerHTML, whose HTML serialization emits void elements (<br>,<img>) without self-closing slashes. That is invalid XML inside<foreignObject>, so the SVG<img>firesonerror→'snapshot image failed'for any page containing a single void element.The fix
Same change in both copies of the bridge (
apps/web/src/runtime/srcdoc.tsand the daemon-injectedURL_PREVIEW_SNAPSHOT_BRIDGEinapps/daemon/src/routes/project/index.ts):inlineSnapshotStylesinto a newstripSnapshotResources, called afterpruneHiddenSnapshotNodes— nothing may remove clone nodes while the original/clone lists still need to pair by index.XMLSerializer(serializeSnapshotXhtml) instead ofinnerHTML, and drop attribute names that are not valid XML Names (@click,:href,{shorthand}— HTML tolerates them, the SVG XML parser does not).What users will see
In browser deployments (Docker/K8s, or
pnpm tools-dev run webopened in a browser), the preview toolbar's screenshot capture and Mark-mode annotation screenshots now produce a real image instead of "Preview is still loading. Try again in a moment." / "Could not capture the preview." Desktop behavior is unchanged (compositor path still wins).Surface area
Screenshots
No UI change (the fix makes the existing button work).
Bug fix verification
apps/web/tests/runtime/srcdoc.test.ts— two new specs: "strips snapshot resources only AFTER pruning so the original/clone lists stay index-aligned" and "serializes snapshot content as XHTML so void elements do not break the SVG parse".main, green on this branch? Yes — onmainthe two specs fail (2 failed | 39 passed), on this branch all 41 pass.<h1>+<div>captured fine while the same page plus one<script src>/<link>returnedempty-render, and one<br>returnedsnapshot image failed; after the fix a real ~10,000 px-tall artifact page returns a correct PNG and the screenshot flows end-to-end (capture → upload → staged chat attachment).Validation
pnpm guard— passpnpm --filter @open-design/web typecheck/pnpm --filter @open-design/daemon typecheck— passapps/web:vitest run tests/runtime/srcdoc.test.ts— 41 passed (includes the 2 new regression specs)apps/daemon:vitest run tests/project-preview-containment.test.ts tests/project-file-range.test.ts(the suites covering the URL-preview serving path) — 54 passed