fix: inject charset meta tag at start of head - #92
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| item.tagName === 'meta' && | ||
| item.attributes && | ||
| item.attributes.name === 'viewport' && | ||
| metaViewportRegExp.test(html) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63e14a1b69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * @returns {boolean} | ||
| */ | ||
| function hasCharsetMetaTag(html) { | ||
| const metaTags = html.match(/<meta\b[^>]*>/gi) || []; |
There was a problem hiding this comment.
When a template contains charset-looking text inside a comment or script, such as <!-- <meta charset="utf-8"> -->, this raw match reports that a declaration already exists. The generated charset is then skipped even though the browser sees no active charset meta tag, leaving the document without the declaration the option requested.
Useful? React with 👍 / 👎.
| if (this.options.inject) { | ||
| const htmlRegExp = /(<html[^>]*>)/i; | ||
| const headRegExp = /(<\/head\s*>)/i; | ||
| const headOpenRegExp = /(<head(?:\s[^>]*)?>)/i; |
There was a problem hiding this comment.
When text resembling <head> occurs in a leading HTML comment, this regex matches that text rather than the document's actual head opener, so the charset is inserted into the comment and remains inert. It can also terminate at a > inside a quoted head attribute and splice the meta tag into the attribute value; the insertion target therefore needs to distinguish a real, complete opening tag.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d8b107300
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| html = html.replace( | ||
| headOpenRegExp, | ||
| (match) => match + headStart.join(''), | ||
| ); |
There was a problem hiding this comment.
When a valid template omits the optional <head> start tag but retains </head>, such as <html><title>x</title></head><body>…, headCloseRegExp prevents creation of a head while this replacement finds no headOpenRegExp match. The configured charset is therefore silently omitted, whereas the previous implementation injected it before the closing tag; fall back to that closing tag or synthesize an explicit opener in this case.
Useful? React with 👍 / 👎.
| html = html.replace( | ||
| headOpenRegExp, | ||
| (match) => match + headStart.join(''), | ||
| ); |
There was a problem hiding this comment.
When a template has more than roughly 1 KB before its real <head>—for example, a long license comment or large attributes on <html>/<head>—appending the declaration after that opener still places it beyond the first 1024 bytes. Without an HTTP charset, browsers will not use such a late declaration, so this change does not satisfy the constraint it is intended to enforce for those documents; the insertion logic should verify the resulting byte offset and handle an already-late head.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74a6c13fd8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isCharsetMetaTag(item)) { | ||
| // A charset declaration has to be entirely within the first 1024 bytes | ||
| // of the document, so inject it at the start of the head. | ||
| headStart.push(htmlTagObjectToString(item, this.options.xhtml)); |
There was a problem hiding this comment.
When the template already contains an active charset declaration and meta generates another with a different value, this loop unconditionally places the generated declaration before the authored one. Previously the generated tag was appended near </head>, so the template's earlier declaration retained precedence; the new order changes the browser's effective encoding and leaves conflicting declarations. Detect an active template charset and avoid prepending another one.
Useful? React with 👍 / 👎.
2 participants
Summary
This PR ensures generated charset meta tags are injected immediately after the opening
<head>tag, keeping the declaration within the first 1024 bytes of the document. It preserves charset declarations authored by templates and deduplicates additional generated declarations.Related Links
web-infra-dev/rsbuild#8275