Expose page render source metadata for a11y agent#36264
Conversation
|
Claude finished @fmontes's task in 5m 40s —— View job Rollback Safety Analysis
Result: Pull Request Unsafe to Rollback — two categories matched. M-3 — REST / GraphQL / Headless API Contract Change 🟡 MEDIUM
Files: Safer alternative: Keep H-8 — VTL Viewtool Contract Change 🟠 HIGH
Files: The label |
🤖 Bedrock Review —
|
|
Pull Request Unsafe to Rollback!!!
|
fileUri.substring(fileUri.lastIndexOf('/')) throws
StringIndexOutOfBoundsException when the URI has no slash. Use
lastIndexOf('/') + 1 so it returns the bare file name and safely
falls back to the full URI when no slash is present.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review triage — addressed in
|
There was a problem hiding this comment.
Pull request overview
This PR adds backend support for the a11y agent to (1) identify which theme/template assets participate in a page render and (2) optionally trace compiled CSS back to editable preprocessor sources via inline source maps, while also fixing page-mode consistency for backend-user asset requests.
Changes:
- Expanded page render-sources output to return all theme files as a generic
files[]list with(path, identifier, extension)via the newFileRefView. - Made sub-resource page-mode resolution referer-aware for backend users to avoid mixing working assets into
?mode=LIVErenders; updated theme template inclusion to be version-aware. - Added
?sourcemap=truesupport to emit inline Sass source maps (uncached), plus OpenAPI and integration test updates.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml | Adds FileRefView schema and updates theme source schema from VTL-only to full file list. |
| dotCMS/src/main/webapp/WEB-INF/messages/Language.properties | Adds i18n strings for the Accessibility Studio portlet UI. |
| dotCMS/src/main/java/com/dotmarketing/util/PageMode.java | Adds getWithReferer() to resolve page mode for sub-resource requests using referer for backend users. |
| dotCMS/src/main/java/com/dotmarketing/servlets/SpeedyAssetServlet.java | Switches asset serving mode resolution to PageMode.getWithReferer() and updates debug logging. |
| dotCMS/src/main/java/com/dotcms/rest/api/v1/page/VtlFileRefView.java | Removes VTL-specific file-ref DTO in favor of a generic one. |
| dotCMS/src/main/java/com/dotcms/rest/api/v1/page/ThemeSourceView.java | Replaces vtls with generic files list of FileRefView. |
| dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageResourceHelper.java | Builds full theme file list (all extensions) and reuses FileRefView for widget file refs. |
| dotCMS/src/main/java/com/dotcms/rest/api/v1/page/PageRenderSourcesView.java | Updates schema description to reflect broader theme file coverage. |
| dotCMS/src/main/java/com/dotcms/rest/api/v1/page/FileRefView.java | Introduces generic file reference DTO including extension metadata. |
| dotCMS/src/main/java/com/dotcms/rendering/velocity/viewtools/DotTemplateTool.java | Emits logical theme template path plus a flag indicating whether it’s a file-asset theme template. |
| dotCMS/src/main/java/com/dotcms/rendering/velocity/services/PageLoader.java | Uses #dotParse for file-asset theme templates (version-aware) and preserves #parse for static fallbacks. |
| dotCMS/src/main/java/com/dotcms/rendering/velocity/directive/DotParse.java | Adds one-shot suppression flag to avoid injecting an edit-control icon for the theme shell include. |
| dotCMS/src/main/java/com/dotcms/csspreproc/DotLibSassCompiler.java | Adds optional inline source-map compilation option passed into Dart Sass compiler. |
| dotCMS/src/main/java/com/dotcms/csspreproc/dartsass/CompilerOptions.java | Adds sourceMap flag and command generation for inline source maps vs explicit --no-source-map. |
| dotCMS/src/main/java/com/dotcms/csspreproc/CSSPreProcessServlet.java | Derives live from page mode (referer-aware) and adds ?sourcemap=true uncached serving path. |
| dotcms-integration/src/test/java/com/dotcms/rest/api/v1/page/PageRenderSourcesResourceTest.java | Extends coverage to assert multiple theme file types are returned with correct extensions. |
| dotcms-integration/src/test/java/com/dotcms/csspreproc/CSSPreProcessServletTest.java | Adds integration coverage asserting inline source map marker is present when requested. |
Copilot review findings: - CSSPreProcessServlet: ?sourcemap=true uses --embed-sources, inlining the raw SCSS (including @imported partials never served directly) into the response. Previously any caller able to fetch the compiled CSS could also retrieve the source by adding the flag. Require EDIT permission on the asset before embedding sources; return 403 otherwise. Edit perms are resolved for LIVE requests too, since the a11y agent scans pages in ?mode=LIVE. - SpeedyAssetServlet: drop the raw Referer header from the debug log; it can carry sensitive query-string data and the resolved PageMode already captures what the log needs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot review — both findings addressed in
|
| * The {@code extension} lets consumers filter by type (e.g. vtl, css, scss, sass, js) | ||
| * without the API having to whitelist which extensions a theme may contain. | ||
| */ | ||
| public class FileRefView { |
There was a problem hiding this comment.
Prefer records now that we are in Java 25
There was a problem hiding this comment.
Done in 000a59b — FileRefView is now a record. Regenerated openapi.yaml is byte-identical, so the REST contract is unchanged.
| .map(f -> toFileRef(f, host)) | ||
| .collect(Collectors.toList()); | ||
|
|
||
| return new ThemeSourceView(theme.getIdentifier(), theme.getName(), folderPath, vtls); |
There was a problem hiding this comment.
A test would be nice here
There was a problem hiding this comment.
Tracked in #36269 (test for the toFileRef/widget file-ref path).
| * @param request the current sub-resource request | ||
| * @return the resolved {@link PageMode} | ||
| */ | ||
| public static PageMode getWithReferer(final HttpServletRequest request) { |
There was a problem hiding this comment.
Add cobertura to PageModeTest
There was a problem hiding this comment.
Done in d24909c — added getWithReferer coverage to PageModeTest: backend honors referer mode, anon/front-end ignore it (forced LIVE), explicit ?mode= wins, no-referer falls back to PREVIEW.
fabrizzio-dotCMS
left a comment
There was a problem hiding this comment.
Two notes on the theme-template #parse → #dotParse switch. The change is good and actually improves content invalidation (it moves template.vtl onto the same version-aware file-asset velocity path that header.vtl/footer.vtl/html_head.vtl already use, and removes the old up-to-5s stale-binary pin). But it's a sensitive, high-blast-radius path, so flagging one concrete cross-host cache-key risk and one test gap inline.
| themeTemplatePath = themeTemplate.getFileAsset().getPath(); | ||
| // Logical, host-relative file-asset path (NOT the physical working-inode disk path) so the | ||
| // page render can include it with #dotParse and resolve live vs working from the PageMode. | ||
| themeTemplatePath = themePath + Template.THEME_TEMPLATE; // themePath already ends with "/" |
There was a problem hiding this comment.
Cache-key scoping risk with the new host-relative templatePath 🟠
Going from themeTemplate.getFileAsset().getPath() (an absolute, host-independent path to the working binary on disk) to themePath + THEME_TEMPLATE is correct for version-aware live/working resolution via #dotParse, and it improves invalidation. But it interacts badly with the themeMap cache key.
The cache (DotTemplateTool.java:55 — 5s expireAfterWrite, no event invalidation) is keyed by theme folder identifier only ("themeMap" + themeFolder.getIdentifier(), line 336), while themePath is host-relative whenever themeFolder.getHostId().equals(hostId). And #dotParse resolves a relative path against params.currentHost (DotParse.resolveFileAsset:295) — the rendering page's host, not the theme's.
So for a theme shared across hosts (the //host branch right above exists precisely because that's supported):
- A page on the theme's own host renders first → relative
templatePathgets cached. - Within the 5s window, a page on a different host whose template points at this same theme reads the cached relative path →
#dotParseresolves/application/themes/<x>/template.vtlagainst the wrong host →ResourceNotFound, or worse, a different theme that happens to sit at that path.
The old absolute disk path didn't have this failure mode for the include. Two clean fixes:
- Always store the host-qualified (
//<themeHost>/...) path for file-asset themes so it stays host-independent like before (the theme host is already resolved in theelsebranch), or - Add the rendering host to the cache key (
"themeMap" + folderId + hostId).
I'd lean toward (1) — it keeps #dotParse resolution unambiguous regardless of which host renders.
There was a problem hiding this comment.
Good catch — fixed in 7f27ee8. The file-asset templatePath is now always host-qualified (//<themeHost>/..., your option 1), so #dotParse resolves it against the theme's own host regardless of the rendering host. $dotTheme.path stays host-relative so existing consumers are unaffected.
| // (html_head/header/footer) inside the theme still emit their icons. | ||
| sb.append("#if($dotTheme.templatePathIsFileAsset)") | ||
| .append("#set($dontShowThemeTemplateIcon = true)") | ||
| .append("#dotParse(\"$dotTheme.templatePath\")") |
There was a problem hiding this comment.
High-blast-radius behavior change with no direct render test 🟠
This swaps the theme-shell include from #parse (filesystem, always the working binary) to #dotParse (version-aware) for every page that uses a file-asset theme. Net effect: a LIVE render now uses the published template.vtl instead of the latest working copy — that's the intended fix, but it's a semantic change on the hottest render path and nothing in the PR exercises it end-to-end.
Worth an integration test that renders a file-asset-theme page:
- EDIT mode: assert no stray
data-dot-object='vtl-file'control is emitted for the shell (the one-shotdontShowThemeTemplateIconsuppression), while nested#dotParseincludes (html_head/header/footer) still emit theirs. - LIVE vs WORKING: publish
template.vtl, then edit the working copy, and assert LIVE renders the published version while PREVIEW/EDIT renders the working one.
Static-fallback themes correctly stay on #parse — good. See also the cross-host themeMap cache-key concern on DotTemplateTool line 369, which this include now depends on.
The themeMap cache is keyed by theme folder identifier only, but the
file-asset template path was host-relative when the rendering host
matched the theme host. For a theme shared across hosts, a relative
path cached by one host's render would be resolved by #dotParse against
a different rendering host within the cache window, causing
ResourceNotFound or the wrong theme. Always store the host-qualified
("//<themeHost>/...") path for the file-asset template include so
#dotParse resolves it against the theme's own host regardless of which
host renders the page.
Reported by @fabrizzio-dotCMS in review.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses review feedback from @fabrizzio-dotCMS: - Rename CSSPreProcessServletTest -> CSSPreProcessServletIT to match the integration-test naming convention; update the MainSuite2b registration. - Document the templatePathIsFileAsset map entry and the host-qualified file-asset template path on DotTemplateTool.setThemeData. - Add PageModeTest coverage for the new PageMode.getWithReferer: backend user honors referer mode, anonymous and front-end users ignore it (forced LIVE), explicit ?mode= wins over referer, and no-referer-mode falls back to PREVIEW. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per review (@fabrizzio-dotCMS) — prefer records on Java 25. Updated the record-accessor call sites (PageResourceHelper widget refs, the PageRenderSourcesResourceTest assertions). Regenerated openapi.yaml is byte-identical, so the REST contract is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Pull Request Unsafe to Rollback!!!
|
Summary
This is the backend slice of the a11y-agent work. It gives the agent enough render-source metadata to understand which theme/template files participate in a page render, and enough stylesheet source information to trace compiled CSS back to editable source files.
What changed
GET /api/v1/page/_render-sourcesso theme sources are returned as a genericfiles[]list instead of only VTL-oriented refs. Each file now includes its path, identifier, and lowercased extension, so consumers can decide which files are relevant.FileRefView, since the same shape is now used for VTL, CSS, SCSS/SASS/LESS, JS, and widget refs.?sourcemap=truefor compiled stylesheet requests. This keeps normal stylesheet responses unchanged, while allowing the agent to map CSS rules back to editable SCSS sources when needed.LIVE,PREVIEW, or edit-related modes get assets that match the requested page mode instead of accidentally mixing in working content.?mode=LIVEit leaksvtland orcssfiles in working state.Why
The a11y agent needs to propose or apply fixes against the files editors can actually change. For contrast fixes and template attribution, looking only at rendered HTML/CSS is not enough: the agent needs to know which theme files exist, which ones are editable stylesheet/template sources, and how compiled CSS maps back to source SCSS.
This PR keeps that backend/API support separate from the frontend and agent runtime changes so it is easier to review and QA independently.
Verification
git diff --checkbefore commitdotcms-integrationsuite; it uses the heavier Maven/Failsafe integration setup.This PR fixes: #36265