Skip to content

refactor: enhance UVE script injection logic #35128

Open
nicobytes wants to merge 9 commits intomainfrom
fix-uve-issue
Open

refactor: enhance UVE script injection logic #35128
nicobytes wants to merge 9 commits intomainfrom
fix-uve-issue

Conversation

@nicobytes
Copy link
Copy Markdown
Member

@nicobytes nicobytes commented Mar 26, 2026

This pull request improves the logging and error handling in the UVE script injection process within the HTML page rendering flow. The changes make it easier to trace when and how the UVE script is injected, and ensure that failures in script building are handled gracefully.

Enhanced logging and error handling for UVE script injection:

  • Added debug logs to indicate when the UVE script is injected or skipped based on the page mode in the build method of HTMLPageAssetRenderedBuilder (dotCMS/src/main/java/com/dotmarketing/portlets/htmlpageasset/business/render/page/HTMLPageAssetRenderedBuilder.java).
  • Improved the injectUVEScript method to:
    • Log when the rendered HTML is empty or null and script injection is skipped.
    • Use a Try block to handle exceptions during script building, logging errors and falling back to a default script if needed.
    • Add debug logs to indicate where the script is injected (before </body> or at the end if no </body> tag is found).

#35127

@nicobytes nicobytes requested review from Copilot and dario-daza March 26, 2026 14:32
@github-actions github-actions bot added the Area : Backend PR changes Java/Maven backend code label Mar 26, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 26, 2026 •
edited
Loading

Claude finished @nicobytes's task in 2m 36s —— View job

Code Review

  • Gather context and read changed files
  • Analyze for bugs, design issues, and risks

Issues Found

1. Java: Logger.warn for missing </body> will flood logs in production

HTMLPageAssetRenderedBuilder.java:414:

Logger.warn(this, "No </body> tag found in page HTML, appending UVE script at end");

This fires on every non-LIVE render of an advanced template or page fragment that legitimately omits a <body> tag. WARN implies something unexpected and actionable; this is a normal code path. Should be Logger.debug.

2. Java: Logger.debug with lambda still evaluates eagerly

Lines 228–235 and 407–409 use Logger.debug(this, () -> String.format(...)). As flagged in the copilot review, dotCMS's Logger.debug(Object, Supplier<String>) calls .get() immediately — the lambda is not deferred. String.format runs on every page render regardless of log level. Needs isDebugEnabled guards or use Logger.debug(this, "literal") with no formatting.

3. Frontend/Java: </body> search inconsistency

ema-legacy-script-injection.ts:60:

return rendered.replace('</body>', scriptString + '</body>');

HTMLPageAssetRenderedBuilder.java:410:

final int closingBodyIndex = html.toLowerCase().lastIndexOf("</body>");

Java searches case-insensitively (toLowerCase()) and finds the last match. TypeScript uses String.replace which is case-sensitive and matches the first occurrence. For any page with </BODY> (uppercase) or multiple </body> occurrences (page fragments concatenated together), these will behave differently. The Java side handles </BODY> correctly; the TypeScript side silently falls back to appending at end.

Fix this →

4. Potential double injection for EMA VTL pages

The module comment in ema-legacy-script-injection.ts states "EMA runs headless — the iframe loads the customer's own frontend and does not consume the backend-rendered HTML." But setupIframeForVTLPage (line 787) does exactly this — it writes this.uveStore.$pageRender() directly into the iframe via document.write():

const newDoc = this.injectCodeToVTL(pageRender);
doc.open();
doc.write(newDoc);

If $pageRender() is the HTML from entity.page.rendered (which now contains the backend-injected dot-uve.js from PR #34927), and FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION is also true, the script gets injected twice. The existing tests don't cover this scenario. The module comment's description of the headless flow needs to reconcile with what setupIframeForVTLPage actually does, or the flag should only apply when $pageRender() does NOT already contain the script.

5. Frontend: === true strict equality is inconsistent with other flags

withPageContext.ts:

const $isEmaLegacyScriptInjectionEnabled = computed(
    () => flags().FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION === true
);

All other flags in this file (e.g. $isStyleEditorEnabled) return flags().FEATURE_FLAG_* directly without === true. If the backend returns a string "true" or any other truthy value, this flag silently stays disabled while all other flags work. Should be consistent — either add === true everywhere, or remove it here.

6. Java test: reflection-based private method testing is fragile

HTMLPageAssetRenderedBuilderUVEScriptTest.java:55-58:

final Method method = HTMLPageAssetRenderedBuilder.class.getDeclaredMethod(
    "injectUVEScript", String.class, Collection.class);
method.setAccessible(true);

If injectUVEScript is ever renamed or its signature changes, the test throws NoSuchMethodException at runtime (not a compile error). Making the method package-private would be safer and allow direct invocation without reflection.

Overall: The core logic change (rawHTML extracted once instead of calling getPageHTML(mode) twice) is correct. Documentation is thorough. The main risks are the double injection edge case (#4, needs design clarification) and the warn-log noise (#1). The logger eager-evaluation issue (#2) is a real perf concern on every page render.

@claude
Copy link
Copy Markdown
Contributor

claude bot commented Mar 26, 2026 •
edited
Loading

Claude encountered an error —— View job

I'll analyze this and get back to you.

@nicobytes nicobytes linked an issue Mar 26, 2026 that may be closed by this pull request
UVE editor fails to load after v26.03.20-01 — dot-uve.js script injection regression #35127
Open
6 tasks
Copilot AI reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves observability and resilience of the backend UVE (dot-uve.js) script injection performed during HTML page rendering, helping diagnose and prevent regressions where the UVE editor fails to load in non-LIVE modes.

Changes:

  • Adds debug logging to clearly indicate when UVE script injection is performed vs skipped (LIVE mode) during page rendering.
  • Enhances injectUVEScript to skip injection on empty HTML, log injection position, and fall back gracefully if script building fails.
  • Wraps UVE style-editor script building with exception handling to avoid breaking page rendering when schema/script generation errors occur.

@nicobytes nicobytes changed the title refactor: enhance UVE script injection logic i refactor: enhance UVE script injection logic Mar 26, 2026
@github-actions github-actions bot added the Area : Frontend PR changes Angular/TypeScript frontend code label Mar 30, 2026
@dario-daza
Copy link
Copy Markdown
Member

dario-daza commented Mar 30, 2026

You might need to include the new FF for EMA in other files like here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@erickgonzalez erickgonzalez erickgonzalez approved these changes

@dario-daza dario-daza dario-daza approved these changes

Assignees

No one assigned

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code Team : Maintenance

Projects

dotCMS - Product Planning
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UVE editor fails to load after v26.03.20-01 — dot-uve.js script injection regression

4 participants

@nicobytes @dario-daza @erickgonzalez