From 476798523ccc494e816f8cf59708a0556b906363 Mon Sep 17 00:00:00 2001 From: Daniel Silva Date: Thu, 6 Aug 2026 21:03:25 +0200 Subject: [PATCH] fix(uve): open file-asset links in a new tab instead of loading them as pages (#35504) handleInternalNav treated every same-host link as an HTMLPage and fed it to the Page API, so a link to a PDF resolved to a 404 "Page not found" in both edit and preview mode. Add an isAssetPath() predicate that mirrors the backend extension heuristic, and route hrefs resolving to a file asset to a new tab instead. Refs: #35504, FD #36746 Co-Authored-By: Claude Opus 5 (1M context) --- .../edit-ema-editor.component.spec.ts | 35 ++++++++++++ .../edit-ema-editor.component.ts | 11 ++++ .../edit-ema/portlet/src/lib/utils/index.ts | 54 +++++++++++++++++++ .../portlet/src/lib/utils/utils.spec.ts | 40 +++++++++++++- 4 files changed, 139 insertions(+), 1 deletion(-) diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.spec.ts index 259ee5c1209a..23dfa84c6cc5 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.spec.ts @@ -3115,6 +3115,41 @@ describe('EditEmaEditorComponent', () => { expect(mockEvent.preventDefault).toHaveBeenCalled(); }); + it('should open a same-host PDF link in a new tab instead of loading it as a page', () => { + const pdfUrl = 'http://localhost:3000/application/files/report.pdf'; + const mockEvent = createMockEvent(pdfUrl); + + spectator.component.handleInternalNav(mockEvent); + + expect(windowOpenSpy).toHaveBeenCalledWith(pdfUrl, '_blank'); + expect(pageLoadSpy).not.toHaveBeenCalled(); + expect(mockEvent.preventDefault).toHaveBeenCalled(); + }); + + it('should open a /dA/ asset link in a new tab instead of loading it as a page', () => { + const assetUrl = 'http://localhost:3000/dA/abc123/asset/report.pdf'; + const mockEvent = createMockEvent(assetUrl); + + spectator.component.handleInternalNav(mockEvent); + + expect(windowOpenSpy).toHaveBeenCalledWith(assetUrl, '_blank'); + expect(pageLoadSpy).not.toHaveBeenCalled(); + expect(mockEvent.preventDefault).toHaveBeenCalled(); + }); + + it('should still load a page when the URL uses the page extension', () => { + const pageUrl = 'http://localhost:3000/test-page/index.html'; + const mockEvent = createMockEvent(pageUrl); + + spectator.component.handleInternalNav(mockEvent); + + expect(windowOpenSpy).not.toHaveBeenCalled(); + expect(pageLoadSpy).toHaveBeenCalledWith({ + url: '/test-page/index.html' + }); + expect(mockEvent.preventDefault).toHaveBeenCalled(); + }); + it('should extract and pass query parameters from URL', () => { const urlWithParams = 'http://localhost:3000/test-page?param1=value1¶m2=value2'; diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.ts index 22f7e6c36920..6bc1b588e9df 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/edit-ema-editor/edit-ema-editor.component.ts @@ -118,6 +118,7 @@ import { deleteContentletFromContainer, getTargetUrl, insertContentletInContainer, + isAssetPath, isSamePageNavigation, measureCanvasAvailableSize, shouldNavigate @@ -768,6 +769,16 @@ export class EditEmaEditorComponent implements OnDestroy, AfterViewInit { return; } + // Files (PDFs, images, docs…) are not pages: the Page API cannot resolve + // them and the editor would show "Page not found". Open them in a new tab + // so the author can verify the link without leaving the editor. + if (isAssetPath(url.pathname)) { + this.window.open(href, '_blank'); + e.preventDefault(); + + return; + } + // Same pathname (any hash/query): let the browser handle it (anchors, query-driven UI) if (isSamePageNavigation(href, this.uveStore.pageParams()?.url)) { return; diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts index e94e86fc9429..f42cb0e17d55 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/index.ts @@ -1168,3 +1168,57 @@ export const isSamePageNavigation = (incomingUrl: string, currentUrl: string): b return target.pathname === current.pathname; }; + +/** dotCMS path prefixes that stream a binary asset instead of rendering a page. */ +const ASSET_PATH_PREFIXES = ['/dA/', '/dotAsset/', '/contentAsset/']; + +/** + * 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']); + +/** + * 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}$/; + +/** + * Checks whether a pathname targets a file asset rather than an HTMLPage. + * + * Mirrors the backend's own extension heuristic: no extension (or the page + * extension) means a page; any other real extension means a file. + * + * @param {string} pathname - The pathname to check (query and hash excluded) + * @returns {boolean} True when the pathname points at a file asset + * + * @example + * isAssetPath('/application/files/doc.pdf') // true + * isAssetPath('/dA/abc123/asset/doc.pdf') // true + * isAssetPath('/about-us/index') // false + * isAssetPath('/about-us/index.html') // false + * isAssetPath('/blog/release-v1.2') // false + */ +export const isAssetPath = (pathname: string): boolean => { + if (!pathname) { + return false; + } + + if (ASSET_PATH_PREFIXES.some((prefix) => pathname.startsWith(prefix))) { + return true; + } + + const lastSegment = pathname.slice(pathname.lastIndexOf('/') + 1); + const dotIndex = lastSegment.lastIndexOf('.'); + + if (dotIndex === -1) { + return false; + } + + const extension = lastSegment.slice(dotIndex + 1).toLowerCase(); + + return FILE_EXTENSION_PATTERN.test(extension) && !PAGE_PATH_EXTENSIONS.has(extension); +}; diff --git a/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/utils.spec.ts b/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/utils.spec.ts index 8f8718e357c6..064fddd20a1a 100644 --- a/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/utils.spec.ts +++ b/core-web/libs/portlets/edit-ema/portlet/src/lib/utils/utils.spec.ts @@ -31,7 +31,8 @@ import { normalizeQueryParams, convertUTCToLocalTime, escapeHtmlAttributeValue, - isSamePageNavigation + isSamePageNavigation, + isAssetPath } from '.'; import { DEFAULT_PERSONA, PERSONA_KEY } from '../shared/consts'; @@ -1648,4 +1649,41 @@ describe('utils functions', () => { expect(result.getHours()).toBe(12); }); }); + + describe('isAssetPath', () => { + 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([ + ['/about-us/index', false], + ['/about-us/index.html', false], + ['/about-us/index.htm', false], + ['/legacy/index.dot', false], + ['/blog/', false], + ['/', false], + ['/blog/release-v1.2', false], + ['/news/2024.10', false] + ])('should treat %s as a page', (pathname, expected) => { + expect(isAssetPath(pathname as string)).toBe(expected); + }); + + it('should return false for an empty pathname', () => { + expect(isAssetPath('')).toBe(false); + }); + + it('should return false for a nullish pathname', () => { + expect(isAssetPath(undefined as unknown as string)).toBe(false); + }); + }); });