Fonts: Create sanitized CSS @font-face font-family on upload#76782
Fonts: Create sanitized CSS @font-face font-family on upload#76782
Conversation
|
Size Change: +598 B (+0.01%) Total Size: 7.74 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 5c9dbb8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23845487522
|
83fa554 to
6613fb8
Compare
There was a problem hiding this comment.
Pull request overview
Updates the global styles font upload flow to generate CSS-safe @font-face rules by quoting/escaping font-family values and switching the in-browser loading mechanism from FontFace API usage to managed constructable stylesheets.
Changes:
- Add
createCssString()to serialize font names (and URLs) into quoted, escaped CSS strings. - Change browser font loading/unloading to insert/delete
@font-facerules in managedadoptedStyleSheets(document + editor iframe). - Split uploaded font metadata into
fontDisplayName(UI) vs escapedfontFamily(CSS) via a newFontFileMetadatatype.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/global-styles-ui/tsconfig.json | Adds gutenberg-env typings for the package. |
| packages/global-styles-ui/src/font-library/utils/make-families-from-faces.ts | Builds upload payloads using display name + escaped CSS fontFamily. |
| packages/global-styles-ui/src/font-library/utils/index.ts | Introduces createCssString() and replaces FontFace loading with stylesheet rule management. |
| packages/global-styles-ui/src/font-library/upload-fonts.tsx | Updates upload flow to use FontFileMetadata and escaped fontFamily for preview/install. |
| packages/global-styles-ui/src/font-library/types.ts | Adds FontFileMetadata to separate UI name from CSS family value. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c5ea4cb to
fa23641
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/global-styles-ui/src/font-library/utils/create-css-string.ts
Outdated
Show resolved
Hide resolved
FontFace has inconsistent behavior across browsers and is hard to use correctly with CSS font face font-family values
This reverts commit 2aef1bbb4ee0105d9304bbf34689a9ff64d64763.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/global-styles-ui/src/font-library/utils/create-css-string.ts
Outdated
Show resolved
Hide resolved
packages/global-styles-ui/src/font-library/utils/make-families-from-faces.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
This comment was marked as outdated.
This comment was marked as outdated.
This reverts commit 82f1bb1.
845f78f to
5c9dbb8
Compare
What?
Fixes Trac #63568 / #70426
Use quoted and escaped strings for
@font-facefont-familyvalues of uploaded fonts.The problem was that the REST API request was made with a plain string for the
font-family, whereas the backend expects a string containing CSS text. Something likeWordPress' fontis invalid, it is unquoted (not a CSS string) the unescaped'would start a CSS string.With this change, arbitrary font names from uploaded fonts are likely to work well. The font family name is transformed into a CSS string with a number of CSS Unicode escape sequences to remove characters that are often problematic for other parts of the application like KSES or font normalization.
Why?
The REST API call was made incorrectly, providing a plain string instead of a string containing CSS text. This caused fonts with names containing special characters (e.g.
O'Reilly Sans) to produce invalid CSS and break the fonts or more CSS on the site.How?
createCssString()— New function that produces a properly quoted, Unicode-escaped CSS string, neutralizing characters problematic in CSS and WordPress HTML contexts.CSSStyleSheetinstead ofFontFaceAPI — Font faces are now inserted as@font-facerules into managedadoptedStyleSheets, giving full control over CSS output.loadFontFaceInBrowseris now synchronous.FontFileMetadatatype carries the rawfontDisplayName(for UI/slugs) separately from the escapedfontFamily(for CSS).The
FontFaceAPI was very problematic because of its inconsistent behavior across browsers. It also expects a "plain"font-familystring, which made it more difficult to use with the CSSfont-familythat the rest of the WordPress Font API expects to use. Thefamilyof aFontFaceinstance has inconsistent normalization and is not a CSS string, making it very difficult to compart correctly when attempting to unload a font face.An E2E test is added with a problematic font name
"Ephesis" font with <special \> {chars} & things, ya'know?to confirm it's working well.Testing Instructions
/wp-admin/site-editor.php?p=%2Fstyles§ion=%2Ftypography)test/e2e/assets/Ephesis-modified-name.ttf. It has more and different special characters.Check for any regressions:
Fira CodeandManropein twentytwentyfive.Known issues
I discovered some issues with font display name and font
-familythat are edge cases that should not impact the font's behavior. These are related to some sanitization that was introduced in #58636.The expected
font-familyis incorrectly sanitized by the REST API (these are the decoded values):This is incorrect, although it may be harmless. It appears to be the result of
sanitize_text_field()stripping "extra" whitespace. This could be addressed in this PR by Unicode escaping carriage returns, line feeds, horizontal tabs, and spaces but that seems excessive. The collapsing of multiple spaces to a single space should maintain correct Unicode escapes, but it means that the following whitespace disappears because it is consumed by the Unicode escape:\3E following textbecomes\3E following textwhich is decoded as>following textinstead of the expected> following text. Because the font-family mostly just needs to match, this issue should not be critical although it is worth addressing in follow-up work.Font display names are also sanitized in a destructive way.
Font families are stored in a
wp_font_familypost type, where the font family name is stored in the post title. JSON is stored in the post content.The family name is sanitized, so a family name is changed:
The family name could be stored without additional sanitization in the JSON instead of the post title.
Use of AI Tools
Claude Code (Claude Opus 4.6) was used to perform some development tasks.
Various AI tools were used for PR feedback.