fix: prevent broken script execution in widget iframe#46
Conversation
Idiomorph's innerHTML morphStyle expects a node reference, not a raw HTML string. Passing tmp (the wrapper element) instead of tmp.innerHTML fixes SVG rendering issues where nodes were being incorrectly diffed.
Three bugs in the script execution code caused interactive widgets (BFS/DFS graphs, animations) to fail: - btoa() output contains =, +, / which are invalid in HTML attribute names, causing setAttribute to throw and abort the entire forEach - Incomplete <script> tags during streaming produced broken fragments that failed on execution - No error isolation meant one bad script blocked all others Fix: defer script execution until all <script> tags are fully closed, sanitize base64 hashes for attribute names, skip empty scripts, and wrap individual executions in try/catch.
GeneralJerel
left a comment
There was a problem hiding this comment.
PR Review: fix: prevent broken script execution in widget iframe
Files changed: 1 (widget-renderer.tsx) — +25 / -15
Overall
Good, focused bug fix. The three root causes (invalid base64 chars in attribute names, partial script execution during streaming, no error isolation) are clearly identified and addressed. The PR description is excellent.
Issues
1. Regex escape bug in the string template literal (Medium)
widget-renderer.tsx:368:
var scriptOpens = (rawHtml.match(/<script[\\s>]/gi) || []).length;
var scriptCloses = (rawHtml.match(/<\\/script>/gi) || []).length;This code lives inside a JS template literal (backtick string). The \\s will be interpreted as the literal string \s by the template literal, which then becomes a valid regex \s. However, \\/ becomes \/ in the regex, which is fine — it matches /. So the escaping happens to work, but it's fragile and confusing. Worth a comment or using String.raw if this pattern grows.
2. Silent catch swallows all errors (Low)
widget-renderer.tsx:433:
} catch(e) {}The empty catch block silently swallows script execution errors. While the PR description explains the rationale (one failure shouldn't kill the loop), logging the error would help debugging:
} catch(e) { console.warn('[widget] script exec failed:', e); }3. unescape() is deprecated (Low)
widget-renderer.tsx:422:
var hash = btoa(unescape(encodeURIComponent(key))).slice(0, 16).replace(/[^a-zA-Z0-9]/g, '');unescape() is deprecated. The encodeURIComponent + unescape + btoa pattern is a known workaround for btoa not handling Unicode, which is fine, but the replace(/[^a-zA-Z0-9]/g, '') at the end could theoretically produce collisions or an empty string after stripping. The !hash guard on line 423 handles the empty case, but collisions between different scripts that hash to the same alphanumeric prefix are possible (though unlikely with 16 chars of base64).
4. Script open/close counting is naive (Low)
The regex /<script[\s>]/gi won't match self-closing or attribute-heavy script tags perfectly, and could false-match strings like <scripting>. A more precise pattern would be /<script(?:\s[^>]*)?> and <\/script>. In practice, since this is LLM-generated HTML, the simple patterns are probably fine, but worth noting.
What looks good
- Passing
tmp(DOM node) instead oftmp.innerHTMLto Idiomorph — correct fix per the Idiomorph API forinnerHTMLmorphStyle - Deferring script execution until all script tags are closed is a clean approach to the streaming problem
- The base64 sanitization with
replace(/[^a-zA-Z0-9]/g, '')directly addresses theInvalidCharacterError - Empty/whitespace script skipping is a good guard
Verdict
The core fixes are sound. I'd recommend adding console.warn to the catch block (issue #2) before merging — the rest are minor. Ship it with that small tweak.
Closes #44
Summary
<script>tags are fully closed — prevents broken fragments from mid-stream content from executing and blocking subsequent scriptsdata-exec-*attribute names (=,+,/are invalid in HTML attributes)Problem
Interactive widgets (BFS/DFS graph visualizations, 3D animations) failed to render because:
btoa()output contains=,+,/which are invalid in HTML attribute names —setAttribute('data-exec-Cg==')throwsInvalidCharacterError, aborting the entire script execution loop<script>tags produce broken code fragments (e.g.\n const) that fail on executionforEachloop, preventing later valid scripts from runningTest plan
InvalidCharacterErrororsetAttributeerrors in console