fix: handle semicolon font fallback separators #1991
Conversation
![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: e914e3ccac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (trimmed.includes(';')) { | ||
| trimmed = trimmed | ||
| .split(';') | ||
| .map((part) => part.trim()) |
There was a problem hiding this comment.
The new semicolon normalization splits on every ; without considering quotes, so a literal family like "Foo;Bar" is rewritten to "Foo, Bar" before the early comma return. That changes the actual font identifier and causes incorrect font matching for any document that uses quoted font names containing semicolons, which this function previously preserved.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@mattConnHarbour check and add a response if applicable/or not
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
Overall small change, make sure to add tests
| const trimmed = fontName.trim(); | ||
| let trimmed = fontName.trim(); | ||
| // replace semicolon font fallback separators | ||
| if (trimmed.includes(';')) { |
There was a problem hiding this comment.
once semicolons become commas, the comma check on line 202 returns early -- no sans-serif fallback gets added.
works today because the import path already adds one, but if this function gets called with raw "Liberation Sans;Arial" it'll be missing a generic fallback.
There was a problem hiding this comment.
also.. needs tests for semicolon inputs -- the early return on line 202 skipping fallbacks is easy to miss without one. toCssFontFamily("Liberation Sans;Arial") and toCssFontFamily("Foo;Bar", { wordFamily: 'swiss' }) would cover it.
| if (trimmed.includes(';')) { | ||
| trimmed = trimmed | ||
| .split(';') | ||
| .map((part) => part.trim()) |
There was a problem hiding this comment.
@mattConnHarbour check and add a response if applicable/or not
2 participants
Some DOCX generators list fonts with a semicolon separator. This PR makes sure those are replaced with commas before turning into CSS rules.
This fixes an issue with incorrect font being rendering in the layout engine, leading to text extending beyond document boundaries.