Skip to content

fix(uve): prevent duplicate document.write re-executing inline scripts on VTL pages - #36583

Merged
gortiz-dotcms merged 1 commit into
mainfrom
issue-36141-uve-iframe-dedup-write
Jul 16, 2026
Merged

fix(uve): prevent duplicate document.write re-executing inline scripts on VTL pages#36583
gortiz-dotcms merged 1 commit into
mainfrom
issue-36141-uve-iframe-dedup-write

Conversation

@gortiz-dotcms

@gortiz-dotcms gortiz-dotcms commented Jul 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Root cause: insertPageContent called document.open()/write()/close() to inject server-rendered HTML into the UVE iframe. document.open() replaces the Document object but keeps the same window — so let/const declarations from any prior render remain in the window's global lexical scope. Any subsequent write throws "Identifier '…' has already been declared" when the same scripts run again. This affects two scenarios:

    • Re-entrant writes: doc.close() fires a synthetic load event that routes back into insertPageContent, calling doc.write() a second time on a scope that already has those declarations.
    • Legitimate re-renders: switching from preview to edit mode causes the store to update pageRender with new server-rendered HTML that still contains the same let/const declarations — doc.open() does not clear the scope, so the write throws.
  • Fix: Replaces document.open()/write()/close() with iframeElement.srcdoc. Setting srcdoc navigates the iframe to a fresh browsing context on each unique render, clearing the global lexical scope so inline scripts with top-level let/const declarations always start clean.

  • Re-entrancy guard: A lastWrittenKey field (keyed on src + content) prevents redundant srcdoc assignments. Without it, the synthetic load event fired after each srcdoc navigation would route back into insertPageContent and reset srcdoc again, causing an infinite reload loop. handleInlineScripts remains unconditional so the inline-edit toggle responds to enableInlineEdit changes independently of whether a write was skipped.

What this fixes

On traditional (VTL) pages whose inline scripts declare top-level let or const (e.g. let resizeTimer;, const urlParams), opening the page in UVE Edit mode no longer throws:

Uncaught SyntaxError: Failed to execute 'write' on 'Document': Identifier '…' has already been declared

Re-render scenarios that still work correctly

Trigger Behavior
Re-entrant load from doc.close() synthetic event srcdoc assignment skipped — no reload loop
Reactive effect re-runs with unchanged signals srcdoc assignment skipped
Real content change (pageRender changes) New key → fresh srcdoc write in a clean scope ✅
Navigation to a different page (src changes) New key → fresh srcdoc write in a clean scope ✅
Preview → Edit mode switch New key → fresh srcdoc write in a clean scope ✅

Test plan

  • Open a traditional (VTL) page in UVE Edit mode where an inline <script> declares a top-level let or const — confirm no "Identifier '…' has already been declared" errors in the browser console.
  • Switch between preview and edit mode on the same page — confirm no console errors on re-render.
  • Navigate between pages in the editor — confirm each page renders correctly.
  • Run unit tests: pnpm nx test portlets-edit-ema-portlet --testPathPattern=dot-uve-iframe
uve.console.error.mov

Fixes #36141

🤖 Generated with Claude Code

This PR fixes: #36141

…s on VTL pages (#36141)

Switches insertPageContent from document.open()/write()/close() to
iframeElement.srcdoc so each unique render gets a fresh browsing context.
document.open() reuses the same window object, meaning top-level let/const
declarations from any prior render remain in the global lexical scope and
throw "Identifier '…' has already been declared" on every subsequent write —
even across legitimate re-renders such as switching from preview to edit mode,
where the server returns different HTML that still contains the same declarations.

srcdoc navigates the iframe to a new browsing context on each unique render,
clearing the global scope entirely. A lastWrittenKey guard (src + content)
prevents re-entrant writes: doc.close() fires a synthetic load event that
routes back into insertPageContent, and the reactive effect and the (load)
handler can both fire for the same render. Without the guard, srcdoc would
be reset on each re-entrant call, causing an infinite reload loop.
handleInlineScripts remains unconditional so the inline-edit toggle responds
to enableInlineEdit changes independently of whether a write was skipped.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Claude finished @gortiz-dotcms's task in 8m 33s —— View job


PR Review: prevent duplicate document.write re-executing inline scripts

  • Read changed files (dot-uve-iframe.component.ts, .spec.ts)
  • Trace iframe wiring ($iframeSrc$iframeURL) and re-render paths
  • Analyze srcdoc semantics vs. old document.open/write/close

The root-cause analysis is correct: document.open() reuses the window, so top-level let/const from a prior render survive into the new one and throw Identifier '…' has already been declared. Switching to srcdoc navigates to a fresh browsing context and clears the global lexical scope. The dedup guard + unconditional handleInlineScripts design is sound, and the self-correcting re-entrant load is the right mechanism. No blocking issues.

A couple of non-blocking observations:

New Issues

  • 🟡 Medium: dot-uve-iframe.component.ts:165 — The src half of the dedup key is inert in production for traditional pages, which are the only page type that reaches this code (headless returns early in onIframeLoad). [src]="$iframeSrc()" and $iframeSrc() returns '' whenever pageType === TRADITIONAL (see edit-ema-editor.component.ts:396withEditor.ts:288, which returns new String(''), coerced to '' by the typeof url === 'string' check). So writeKey collapses to content-only in practice. The spec test should re-write srcdoc when src changes (page navigation) (dot-uve-iframe.component.spec.ts:592) passes only because it sets src to a literal string via setInput — a value the real wiring never produces for traditional pages. Dedup still works (two distinct pages have distinct content), but the guard is narrower than the PR table implies, and that test doesn't reflect production. Consider dropping src from the key or documenting that it's content-only for VTL.

  • 🟡 Medium: dot-uve-iframe.component.ts:177srcdoc assignment navigates asynchronously, but handleInlineScripts() (line 180) and setSeoData() (line 142) run synchronously in the same tick against the pre-navigation (about:blank) document. The old document.write/close() was synchronous, so the first pass already saw the populated document. Now the correct document is only observed on the second, navigation-triggered load. This self-corrects (the dedup guard skips the re-write and handleInlineScripts re-runs against the real doc), so it's not a correctness bug — but it means setSeoData first reads an empty document and pushes empty ogTags/ogTagsResults into the store before the real values land, i.e. a transient empty-SEO state on every VTL render. Assumption: the SEO store push is idempotent/harmless when overwritten milliseconds later. What to verify: confirm no consumer of setSeoData flickers or logs on the transient empty push. If it matters, gate setSeoData()/handleInlineScripts() so they only run on the navigation load (e.g. skip them when the srcdoc write was skipped-because-fresh vs. actually-written).

Everything else — the re-entrancy rationale, keeping handleInlineScripts unconditional, and the test additions — looks correct. Relative-URL base resolution is unchanged (both empty src + document.write and srcdoc inherit the parent document's base URL), so no resource-loading regression.
· issue-36141-uve-iframe-dedup-write

zJaaal
zJaaal previously requested changes Jul 16, 2026
@gortiz-dotcms
gortiz-dotcms enabled auto-merge July 16, 2026 17:01
@dsilvam
dsilvam dismissed zJaaal’s stale review July 16, 2026 18:48

Jali's OOO and feedback already addressed

@gortiz-dotcms
gortiz-dotcms added this pull request to the merge queue Jul 16, 2026
@mergify

mergify Bot commented Jul 16, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Merged via the queue into main with commit c4be861 Jul 16, 2026
84 checks passed
@gortiz-dotcms
gortiz-dotcms deleted the issue-36141-uve-iframe-dedup-write branch July 16, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

UVE: Unable to edit page when document.write re-executes inline scripts and throws 'already declared'

4 participants