fix(core): applyPositionEdits fails silently across iframe realms#2096
Conversation
applyPositionEdits(doc) guarded each element with `instanceof HTMLElement` — but `doc` is frequently an iframe's document (the SDK's edit preview, any host embedding a composition), whose elements are HTMLElement instances of THAT frame's realm, never this module's. The check silently no-ops on every single element cross-realm, so bulk position edits never apply inside an iframe. Use the document's own realm's HTMLElement constructor (doc.defaultView); duck-type on `.style` when defaultView is unavailable (a detached/synthetic document). The single-element applyPositionEditToElement was already realm-safe — only the bulk wrapper had the bug. Added a regression test using a real jsdom iframe, confirmed it fails on the old `instanceof HTMLElement` check and passes with the fix.
vanceingalls
force-pushed
the
fix/position-edits-cross-realm
branch
from
d1bf353 to
5a48321
Compare
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Clean fix for a classic cross-realm instanceof trap. The approach is correct: use the document's own realm's HTMLElement constructor, fall back to duck-typing for detached documents. The regression test is well-constructed — validating the premise (iframeWindow.HTMLElement !== globalThis.HTMLElement) before exercising the fix. applyPositionEditToElement was already realm-safe so the change surface is minimal.
LGTM.
— Miga
james-russo-rames-d-jusso
left a comment
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at 5a48321.
Small, focused fix. doc.defaultView?.HTMLElement is the right realm-correct primitive, the .style.setProperty duck-type fallback covers the detached/synthetic-doc case, and the regression test's premise sanity check (iframeWindow?.HTMLElement).not.toBe(globalThis.HTMLElement)) pins the exact realm-mismatch condition — nice touch. LGTM from my side.
One micro-observation, non-blocking: the fallback duck-type path (typeof el.style?.setProperty === "function") will accept SVGElements (which have .style.setProperty), whereas the primary RealmHTMLElement path won't. For real usage this is inert — EDIT_BASE_X_ATTR / EDIT_BASE_Y_ATTR are HTML authoring conventions, and no SVG element would carry them — but if the invariant ever loosens, the fallback path becomes slightly less strict than the primary. Not worth fixing now; noting for future.
CI green at HEAD (all Detect changes gates + preflights passing, main matrix still cooking at review time).
— Review by Rames D Jusso
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Approving — realm-local HTMLElement lookup + duck-type fallback for the cross-realm instanceof trap; no changes since RDJ/Miga R1, CI green. Layered on RDJ + Miga's reviews. — Rames Jusso
4 participants
What
applyPositionEdits(doc)in@hyperframes/core/runtime/position-editsguarded each candidate element withinstanceof HTMLElement.docis frequently an iframe's document (the SDK's edit preview, any host embedding a composition), whose elements areHTMLElementinstances of that frame's realm — never this module's. The check silently no-ops on every single element cross-realm, so bulk position edits never apply inside an iframe.Why
Found during an audit of
@hyperframes/sdk's surface against pacific's movio integration. Pacific'scanvas-reactcode has an explicit workaround comment for this exact bug: "Upstream fix would be duck-typing in@hyperframes/core— until then, all host code must use this wrapper." Every iframe-hosted consumer has had to reimplement the bulk-apply loop themselves to avoid it.How
Use the document's own realm's
HTMLElementconstructor (doc.defaultView?.HTMLElement) instead of the module-scope global. Duck-type on.stylewhendefaultViewis unavailable (a detached/synthetic document). The single-elementapplyPositionEditToElementwas already realm-safe — only the bulk wrapper had the bug.Test plan
instanceof HTMLElementcheck (0 applied, expected 1) and passes with the fixpositionEdits.test.tssuite passes (14/14)@hyperframes/coresuite passes (81 files / 1131 tests)bun run buildclean (core + full workspace, incl. studio)