Skip to content

fix: properly serve .plain.html requests for documents in local content directory#2733

Open
shsteimer wants to merge 6 commits into
mainfrom
fix/content-dir-plain-html-fallback
Open

fix: properly serve .plain.html requests for documents in local content directory#2733
shsteimer wants to merge 6 commits into
mainfrom
fix/content-dir-plain-html-fallback

Conversation

@shsteimer
Copy link
Copy Markdown

fix: #2732

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

Thanks for contributing!

@shsteimer shsteimer requested a review from kptdobe May 27, 2026 23:17
… fixture

Parse the full HTML document and return only the innerHTML of <main>
rather than sending the raw file content. Uses existing hast-util-select
and hast-util-to-html deps. Test fixture updated to use a realistic full
document and a multiline fragment string for readability.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

This PR will trigger a patch release when merged.

@shsteimer shsteimer removed the request for review from kptdobe May 27, 2026 23:43
…hast

Remove inner try/catch in favor of upfront path rewriting — .plain.html
is a virtual URL convention and AEM never stores such files in content/.
Extract <main> innerHTML using hast-util-select/to-html (already
transitive deps) rather than sending the raw file. Update test fixture to
use a realistic full HTML document and tighten assertion to strictEqual.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread src/server/HelixServer.js
let htmlContent = await readFile(contentFilePath, 'utf-8');
// .plain.html is a virtual URL convention — AEM never stores .plain.html files,
// so always read the corresponding .html file and serve the <main> fragment.
const isPlainFallback = contentFilePath.endsWith('.plain.html');
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

worth noting that this assumes there will never be a .plain.html file in the content checkout directory. If that assumption doesn't hold, then this will need to be adapted to handle that case.

@shsteimer shsteimer requested review from kptdobe and tripodsan May 27, 2026 23:48
@shsteimer shsteimer marked this pull request as ready for review May 27, 2026 23:48
shsteimer and others added 2 commits May 28, 2026 10:20
Run npm install to restore @adobe/helix-shared-process-queue and
xdg-basedir that were in package.json but absent from node_modules.
Expand single-line if in extractMainContent to satisfy the curly
and max-statements-per-line ESLint rules.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread src/server/utils.js
* @param {string} html full HTML document
* @returns {string} innerHTML of <main>, or original html if no <main> present
*/
extractMainContent(html) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing unit tests

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

tests added

Comment thread src/server/HelixServer.js
Comment on lines +475 to +478
res.set({
'content-type': 'text/html; charset=utf-8',
'access-control-allow-origin': '*',
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is access-control-allow-origin needed ? this is not happening in prod...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It looks to me like we are setting access-control-allow-origin: * on all responses from the local dev server, originally added in #1900. so this just follows that same pattern as all other locally-served responses in this handler (lines 512, 521, 541, 551).

Adds tests for the normal case, empty <main>, no <main> fallback,
nested markup, and whitespace-only content. Also removes a redundant
inline comment made obvious by the function name.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

.plain.html requests not correctly handled for local content files

2 participants