fix(uve): open file-asset links in a new tab instead of loading them as pages (#35504)

[dsilvam]

Proposed Changes

• Add isAssetPath() to the UVE utils — a predicate that distinguishes file-asset URLs from HTMLPage URLs. It matches dotCMS asset-delivery prefixes (/dA/, /dotAsset/, /contentAsset/) and otherwise mirrors the backend's own extension heuristic in Identifier#setURI: no extension (or the configured VELOCITY_PAGE_EXTENSION) means a page, any other real extension means a file.
• Use it in handleInternalNav — a same-host href that resolves to a file asset now opens in a new tab and calls preventDefault(), instead of being handed to uveStore.pageLoad().
• Tests for both.

Root cause

handleInternalNav split anchor clicks into exactly two buckets: different hostname → open a new tab; anything else → uveStore.pageLoad({ url: url.pathname, ... }). There was no check for whether the same-host target was actually an HTMLPage, so a link to a file asset (/dA/<inode>/fileAsset/doc.pdf, /application/files/doc.pdf) was fed to the Page API, which cannot resolve it. The editor then rendered its "Nothing Live Here Yet" / "Page not found" state.

Because the (internalNav) binding is unconditional, this affected Preview/Published mode as well as Edit mode — the linked issue is titled edit-mode-only, so please exercise both when testing.

Checklist

• Tests
• Translations — n/a, no user-facing strings added
• Security Implications Contemplated

Security note: the new branch passes the already-resolved same-origin href to window.open. The external-host branch above it is unchanged and still handles cross-origin links, so this does not widen what can be opened; it only changes how same-origin file links are handled. No new user input is parsed — isAssetPath receives a URL.pathname that was already constructed upstream.

Test coverage

utils.spec.ts — 19 cases on isAssetPath:

• Assets: /dA/.../report.pdf, /dA/ with no extension, /dotAsset/, /contentAsset/, /application/files/report.pdf, .docx, .mp4, .tar.gz, uppercase .PDF
• Pages: /about-us/index, .html, .htm, .dot, /blog/, /
• Regression guards: /blog/release-v1.2 and /news/2024.10 must stay pages — a naive extension check would read the trailing 2/10 as a file extension and break navigation to URL-map slugs
• Edge: empty and nullish input

edit-ema-editor.component.spec.ts — 3 cases on handleInternalNav: a .pdf link and a /dA/ link each open a new tab, call preventDefault, and do not call pageLoad; an .html link still routes through pageLoad.

Full suite: 37/37 suites, 913 passed, 0 failures. nx lint portlets-edit-ema-portlet clean.

Additional Info

Verified manually against a locally built image: clicking /dA/<inode>/fileAsset/<name>.pdf in edit mode now opens the PDF in a new tab and leaves the editor on the page.

Note for reviewers/QA: the Angular bundle ships from the separate dotcms-core-web Maven module, so ./mvnw install -pl :dotcms-core -DskipTests without --am will silently test a stale frontend. Use ./mvnw install -pl :dotcms-core --am -DskipTests.

Two related items deliberately left out of scope:

• The external-host branch (edit-ema-editor.component.ts, the url.hostname !== window.location.hostname case) opens a new tab but never calls preventDefault(), so an external link also navigates the iframe away. Same class of bug, one line, but unrelated to this issue.
• An extensionless file asset served from a folder path outside /dA/ would still be treated as a page. Not reachable through the reported flow; a fully authoritative fix would need a backend round-trip per link click.

Refs: #35504, FD #36746

This PR fixes: #35504

[zJaaal]

Follow-up review on the heuristic. Three of these are behavior bugs, and two of them reintroduce the reported symptom on narrower inputs; the rest are nits.

What holds up: the three prefixes match web.xml exactly (/dotAsset/* L540, /dA/* L544, /contentAsset/* L563), so the case-sensitive startsWith is correct rather than an oversight (servlet url-patterns are case-sensitive). Placement after the external-host check and before isSamePageNavigation is right, and preventDefault() is what actually stops the iframe from navigating away too.

Also closing out my earlier question on pathname.slice(pathname.lastIndexOf('/') + 1): a pathname with no / is safe. lastIndexOf returns -1, so the slice is slice(0), the whole string. 'report.pdf' classifies as an asset and 'about' as a page. No fix needed, though it would be a cheap test case.

Review drafted by Claude (Claude Code) on behalf of @zJaaal.

[zJaaal]

@dsilvam I pushed the review fixes straight to this branch as 6cfcf1e rather than leaving you to transcribe five threads. Shout if you would rather I had left them as comments and I will happily revert.

All four findings applied, red-first: six tests failed before the change (three in utils.spec.ts, three in edit-ema-editor.component.spec.ts) and pass after.

Change	Why
Dropped htm from PAGE_PATH_EXTENSIONS	Not a dotCMS page extension. See the thread for the backend trace; a .htm upload was still being fed to the Page API, which is the bug this PR fixes
FILE_EXTENSION_PATTERN → /^(?=.*[a-z])[a-z0-9]{1,8}$/	Digit-initial extensions (7z, 3gp) are real. The slug guard only needs to reject all-digit trailing tokens, so /news/2024.10 and /blog/release-v1.2 stay pages
window.open(url.href, '_blank', 'noopener')	href can still be a raw relative attribute when the click lands on a child of the anchor, which opened /dotAdmin/files/.... Plus the host check compares hostname only, so this branch can be cross-origin on another scheme or port
JSDoc	Corrected the "mirrors the backend's own extension heuristic" claim (there is no such heuristic), and recorded the dotted-page-slug false positive and the VELOCITY_PAGE_EXTENSION config caveat

Tests: .htm, .7z and .3gp cases added; a relative-href case added at the component level; the it.each tuples lost their constant expected column, which also removed the as string casts.

Verified locally: nx test portlets-edit-ema-portlet 65/65 suites and 1392 tests passing, nx lint portlets-edit-ema-portlet clean, nx format:write applied.

Still out of scope and untouched, as you flagged: the external-host branch missing preventDefault(), and extensionless file assets served outside /dA/.

Changes authored by Claude (Claude Code) on behalf of @zJaaal.

[claude]

Claude finished @zJaaal's task in 1m 45s —— View job

---

SDK Compatibility Analysis

• Read docs/core/SDK_BREAKING_CHANGE_CATEGORIES.md
• Get full PR diff (4a4612cc9c...d594249a4a)
• Analyze diff against each category (G-1/G-2/G-3, R-1, U-1/U-2, H-1)
• Conclusion / action

Result: No SDK breaking change detected.

All 4 changed files live in core-web/libs/portlets/edit-ema/portlet/src/lib/{edit-ema-editor,utils} — the UVE admin-UI editor portlet (the iframe host that renders the editor chrome around the page preview), not any @dotcms/* SDK package.

Walked through each surface called out in this task:

• GraphQL (G-1/G-2/G-3): No changes to schema, resolvers, or page-api.ts/buildPageQuery. Not touched.
• REST shapes (R-1): No changes to /api/v1/page/*, /api/v1/content, or /api/v1/nav response building/parsing. Not touched.
• UVE postMessage protocol (U-1/U-2): edit-ema-editor.component.ts does use DotCMSUVEAction and __DOTCMS_UVE_EVENT__ elsewhere (e.g. lines 686, 1109-1112, 1216, 1269, 2010), but this PR's diff doesn't touch any of those — the new code (isAssetPath check at line 798, #openInNewTab at line ~818) sits entirely inside handleInternalNav, a local click-interception handler that decides between "let the browser navigate," "call uveStore.pageLoad()," or (new) "open a new tab via a synthetic <a> element." None of these three paths is a postMessage — no message name, payload shape, or enum wire value is added, removed, or renamed.
• SDK compat headers (H-1): SdkVersionWebInterceptor / sdk-compatibility.ts / MinSdkVersion.VALUE are untouched.

The new isAssetPath() heuristic in utils/index.ts and its use in handleInternalNav only change how a same-host anchor click inside the editor iframe is routed client-side (new tab vs. pageLoad()); it has no effect on what any @dotcms/client, @dotcms/react, @dotcms/angular, or @dotcms/uve consumer sends to or receives from the server. This matches the doc's "Non-Breaking Examples" calibration list — an internal refactor/admin-UI-adjacent change with no wire-format change.

No comment or label action taken, per instructions.

[zJaaal]

Accepting this one, and making the acceptance explicit rather than implicit. Pushed 96b01be (JSDoc) and 76fc64c (test).

First, the finding is real, so it is worth stating why rather than waving it through. The page url is a plain required TEXT field (HTMLPageAssetAPIImpl.java:126) and I found no character-class sanitizer on the save path, so a dotted page slug is genuinely authorable.

The reason not to fix it is that the heuristic already fails in the correct direction, and the only real alternative would reverse that:

Misclassification	Consequence
Page read as file (this finding)	Opens in a new tab. The page still renders, the editor keeps its state, the author sees immediately what happened
File read as page (the reported bug)	Handed to the Page API, which cannot resolve it, and the editor is stranded on "Page not found"

The permissive extension test is what buys the first failure mode. Swapping it for a known-extension allowlist would fix /store/product.detail and simultaneously send every uncommon file type (.dwg, .sketch, .pages) down the second path, which is the exact defect this PR exists to close. Tightening the 8-character bound to 5 would trade one silent misclassification for another (.numbers, .torrent) on an arbitrary cutoff. Neither is an improvement.

So, two changes instead of a fix:

• 96b01be records the reasoning in the JSDoc, so the next reader sees a deliberate bias rather than an unhandled edge case.
• 76fc64c pins it as a test:

it.each(['/store/product.detail', '/pages/about.us', '/docs/getting.started'])(
    'should knowingly misread the page %s as a file asset',
    (pathname) => {
        expect(isAssetPath(pathname)).toBe(true);
    }
);

That turns the trade into a tripwire: anyone who "fixes" the false positive has to delete a test whose comment explains what they are giving up, which is the point where the allowlist bias gets reconsidered on purpose.

The authoritative fix remains a backend round-trip per link click against CMSUrlUtil#resolveResourceType. Worth its own issue if this ever bites in the field, not worth the per-click latency now.

Investigated and applied by Claude (Claude Code) on behalf of @zJaaal.

[Copilot]

Pull request overview

Adds file-asset detection to UVE navigation so same-host assets open safely in a new tab instead of invoking the Page API.

Changes:

• Adds asset-path classification using delivery prefixes and extensions.
• Opens detected assets through a noopener anchor.
• Adds utility and navigation tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
utils/index.ts	Implements asset-path detection.
utils/utils.spec.ts	Tests path classification.
edit-ema-editor.component.ts	Routes asset links to a new tab.
edit-ema-editor.component.spec.ts	Tests asset navigation behavior.

Suppressed comments (1)

core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts:1194

• The arbitrary 8-character cap leaves valid file extensions such as .webmanifest classified as pages, so handleInternalNav still sends those assets to the Page API. The backend heuristic accepts any nonempty extension, and the surrounding rationale explicitly aims not to break uncommon file types; remove the length cap while retaining the letter guard, and add a regression case.

const FILE_EXTENSION_PATTERN = /^(?=.*[a-z])[a-z0-9]{1,8}$/;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zJaaal]

Copilot review addressed in 0bb9493. Both inline findings applied; recording why the suppressed one was not.

Applied — dot removed from PAGE_PATH_EXTENSIONS. It is only the absent-property fallback in Config.getStringProperty("VELOCITY_PAGE_EXTENSION", "dot"), and dotmarketing-config.properties:91 ships the value as html, so dot is never active in a standard install. It is also the Word 97-2003 template extension, so listing it sent a real upload type to the Page API. Test case moved to the asset table.

Applied — #openInNewTab now removes the temporary anchor in a finally. click() is the call that throws when the open is refused, so the previous ordering stranded an anchor in the admin document on every failure. The existing throw test now asserts cleanup.

Declined — dropping the 8-character cap from FILE_EXTENSION_PATTERN so .webmanifest classifies as an asset:

• A .webmanifest is referenced through <link rel="manifest">, not an anchor an author clicks, so it does not reach handleInternalNav.
• Removing the cap widens the dotted-page-slug false positive that 76fc64c deliberately pinned with tests. /about.information would flip from page to asset.
• The cap is arbitrary in both directions, so the tested and documented behaviour wins absent a real trigger.

Happy to revisit if someone produces a case where an author actually links to a long-extension asset.

Verified: nx test portlets-edit-ema-portlet 64/64 suites and 1397 tests passing, nx lint clean.

Reviewed and applied by Claude (Claude Code) on behalf of @zJaaal.

I applied my own comments, we are good from my side