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

[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]

The branch decision is made on url.pathname, which is resolved against window.location.origin, but the open uses the unresolved href. Those diverge when the click target is a child of the anchor:

<a href="files/report.pdf"><span>Download</span></a>

e.target is the span, so target.href is undefined and href falls back to rawHref, the raw relative attribute. new URL('files/report.pdf', origin) correctly yields /files/report.pdf so the asset branch is entered, but window.open('files/report.pdf') resolves against the admin document (/dotAdmin/...) and opens a 404 tab. url.href is already computed a few lines above and is exactly what the decision was based on.

Separately, the guard above is url.hostname !== window.location.hostname, hostname only, so this branch can still be cross-origin on a different scheme or port. Since this adds a new window.open, noopener closes reverse-tabnabbing for the cost of one argument. (The external branch has the same gap, which you already called out as out of scope.)

Suggested change

	this.window.open(href, '_blank');
	this.window.open(url.href, '_blank', 'noopener');

[zJaaal]

htm is not a dotCMS page extension. VELOCITY_PAGE_EXTENSION = html in dotmarketing-config.properties:91, and dot is the code-level fallback in Identifier#setURI (Config.getStringProperty("VELOCITY_PAGE_EXTENSION", "dot")). The doc comment right above the Set documents only those two, so the comment and the code already disagree.

The consequence is not cosmetic: a .htm file uploaded as a file asset still gets handed to pageLoad, which is the exact bug this PR fixes. There is no page case on the other side of the trade to pay for it. The ['/about-us/index.htm', false] case in utils.spec.ts should flip to true with this.

Suggested change

	/**
	* Extensions that still resolve to an HTMLPage. `html` is the default
	* `VELOCITY_PAGE_EXTENSION`; `dot` is the legacy fallback the backend uses
	* (see `Identifier#setURI`).
	*/
	const PAGE_PATH_EXTENSIONS = new Set(['html', 'htm', 'dot']);
	/**
	* Extensions that still resolve to an HTMLPage. `html` is the default
	* `VELOCITY_PAGE_EXTENSION`; `dot` is the legacy fallback the backend uses
	* (see `Identifier#setURI`).
	*/
	const PAGE_PATH_EXTENSIONS = new Set(['html', 'dot']);

Worth a comment noting that VELOCITY_PAGE_EXTENSION is configurable, so a site that overrides it will see page links open in a new tab. Not fixable client-side without plumbing the value through, but it should be written down.

[zJaaal]

The letter-initial rule drops real extensions that start with a digit: 7z, 3gp, 3ds. I ran the function standalone and /files/archive.7z returns false, so a .7z download still hits "Page not found".

The thing actually protecting /blog/release-v1.2 and /news/2024.10 is that their trailing tokens are all digits, not that they start with one. Requiring at least one letter anywhere keeps both guards and fixes the digit-initial case. Verified against the full case list in this PR plus 7z/3gp: 2024.10 and release-v1.2 stay pages, 7z becomes an asset, nothing else changes.

Suggested change

	/**
	* Matches a plausible file extension: letter-initial, up to 8 alphanumerics.
	* Guards URL-map slugs such as `/blog/release-v1.2`, whose trailing `2` must
	* not be mistaken for a file extension.
	*/
	const FILE_EXTENSION_PATTERN = /^[a-z][a-z0-9]{0,7}$/;
	/**
	* Matches a plausible file extension: 1-8 alphanumerics containing at least
	* one letter. The letter requirement is what guards URL-map slugs such as
	* `/blog/release-v1.2` and `/news/2024.10`, whose all-digit trailing token
	* must not be mistaken for a file extension.
	*/
	const FILE_EXTENSION_PATTERN = /^(?=.*[a-z])[a-z0-9]{1,8}$/;

Worth adding ['/files/archive.7z', true] to the asset cases so the rule is pinned.

[zJaaal]

The PR description lists two known limitations but not this one, and it is the most reachable of the three: a page whose last segment contains a dot followed by a short alpha token is classified as an asset. Both /store/product.detail and /pages/about.us return true and would open a new tab instead of navigating, since URL-map slugs are author-controlled and can contain dots.

No code change requested, it is inherent to an extension heuristic without a backend round-trip. But it belongs in this JSDoc next to the release-v1.2 example so the next reader knows it was a considered trade rather than an oversight.

[zJaaal]

Nit: the expected column is constant within each block, and carrying it is what forces the pathname as string cast (the mixed tuple infers as (string | boolean)[]). Dropping it removes the cast and makes the block's intent read off the title.

Suggested change

	it.each([
	['/dA/abc123/asset/report.pdf', true],
	['/dA/abc123/asset/no-extension', true],
	['/dotAsset/abc123', true],
	['/contentAsset/raw-data/abc123/asset', true],
	['/application/files/report.pdf', true],
	['/files/quarterly.docx', true],
	['/media/promo.mp4', true],
	['/backups/site.tar.gz', true],
	['/files/REPORT.PDF', true]
	])('should treat %s as a file asset', (pathname, expected) => {
	expect(isAssetPath(pathname as string)).toBe(expected);
	});
	it.each([
	'/dA/abc123/asset/report.pdf',
	'/dA/abc123/asset/no-extension',
	'/dotAsset/abc123',
	'/contentAsset/raw-data/abc123/asset',
	'/application/files/report.pdf',
	'/files/quarterly.docx',
	'/media/promo.mp4',
	'/backups/site.tar.gz',
	'/files/REPORT.PDF'
	])('should treat %s as a file asset', (pathname) => {
	expect(isAssetPath(pathname)).toBe(true);
	});

Same shape applies to the page block below (.toBe(false)).