feat: support project font persistence - #3362
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for project-specific font preferences and generic font embedding in SVGs, replacing the previous Scratch-specific implementation. It adds a new FontFamily model, updates SpxProject to manage and export font preferences, and implements a utility to inject referenced font files as base64 data URIs into SVGs. The code reviewer provided valuable feedback to improve performance and robustness: they suggested using the asynchronous FileReader.readAsDataURL API instead of a synchronous manual chunking loop to avoid blocking the main thread when encoding large font files, and recommended filtering out empty strings when parsing font preferences and extracting used font families to handle malformed inputs gracefully.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
1c6d62e to
1d9307a
Compare
There was a problem hiding this comment.
Code Review Summary
This PR cleanly adds a project-font persistence layer (FontFamily model, project.fonts / fontPreferences, config round-trip, legacy-project migration) with good persistence-side test coverage, and refactors the Scratch-specific SVG font injection into a generic injectFontsToSvgText(svgText, fontPreferences, fontFiles).
The main concern is that the new mechanism is not actually wired into the runtime rendering path, which makes the feature inert at render time and regresses the previous Scratch-font embedding. A few smaller robustness/perf items follow. Security review found no exploitable issues (CSS font-family values are escaped via JSON.stringify, style is injected through XMLSerializer, and faces[].path is only used for in-memory map lookups, not filesystem access).
Inline comments cover the concrete diff-line findings. Additional items that don't map to a single changed line:
- maintainability — duplicated quote-stripping:
stripQuotesinspx-gui/src/utils/svg-font.ts:9andstripFontPreferenceQuotesinspx-gui/src/models/spx/project.ts:40are byte-for-byte identical. Extract a single shared helper so preference parsing and SVG rule generation can't diverge on family-name semantics. - maintainability — migration condition coupling:
spx-gui/src/models/spx/project.ts:469triggers legacy migration only whenfontPreferences == null && fonts.length === 0. Thenullcheck alone is the real signal of a pre-feature project; the extrafonts.length === 0clause is subtle and undocumented. Consider keying migration onfontPreferences == nullonly, or add a comment explaining why both are required. - stale comment (outside this diff):
spx-gui/src/utils/file.ts:125still saysuseRenderableImageUrl"applies rendering-specific handling such as Scratch font injection for SVG files." Sincescratch-svg-font.tswas removed, this should read "font injection for SVG files." - licensing hygiene: the deleted
scratch/fonts vendored their license text (OFL.txt/LICENSE.txt), but the newbasic-chinese/README.mdonly links to the source repo. If policy is to vendor the OFL text alongside binary font assets, consider adding it. - test gap:
spx-gui/src/utils/img-rendering.test.tsfully mocksinjectFontsToSvgText, so no test asserts what arguments the real render path passes — which is why the wiring gap below is invisible to CI. Once wiring is added, assert thatgetRenderableImageUrlforwards the project's preferences and font files.
1d9307a to
1a0b50b
Compare
|
Review-summary follow-up:
|
fe2af2d to
37557ed
Compare
37557ed to
ce1482c
Compare
ce1482c to
9da2b15
Compare
9da2b15 to
3bcc08a
Compare
There was a problem hiding this comment.
把原有的 default-project.xbp 拆了(这样方便根据不同的 env 构造不同的 project 内容),所以多了个一个新的 spx-gui/src/components/project/default-project/ 目录
There was a problem hiding this comment.
svg-font.ts 是原来的 scratch-svg-font.ts 干掉 scratch 的特有逻辑后得到的,不过它的逻辑事实上没生效,只是先保留这份逻辑,后续再更新并启用
|
It appears that it is possible to create font configurations that Builder accepts but SPX rejects. For example:
Currently, Builder only rejects empty font family names, exact matches of the lowercase name For these cases, is runtime rejection the expected behavior, or does Builder apply an equivalent validation mechanism elsewhere? |
@CORCTON It's intended. Builder-side validation aims to enhance user experience by catching common errors and providing immediate feedback. While not replicating spx validation exactly, we focus on frequently encountered or easily verifiable mistakes to avoid unnecessary complexity. |
e875f55 to
435b65f
Compare
435b65f to
4089414
Compare
* Sync `main` to `dev` (#3398) * chore(deps): bump spx to 3.1.0 (#3397) * chore(deps): bump spx to 3.1.0 * fix(deps): add spx 3.1.0 module checksums * fix(deps): record spx transitive module requirement * fix(deps): tidy spx 3.1.0 module graph * feat: support project font persistence (#3362) * feat: support project font persistence * chore: use SPX preset font assets * fix: address project font review comments * docs: link deferred project font rendering issue * fix(ai): handle interaction termination consistently (#3399) Schedule history management from the interaction cleanup path so completed sequences are archived even when they return early. Run archive work in an owner-scoped coroutine so it survives caller completion without outliving the game. Report an error when an interaction exhausts its turn limit instead of silently treating an unfinished sequence as complete. Add regression coverage for archive lifecycle behavior and turn-limit failure reporting. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * feat(account): add Firefox browser hijacking (#3404) Support Firefox WebDriver BiDi alongside Chrome DevTools Protocol for local Account development. Preserve redirects for sign-in and identity provider callbacks across both browsers. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * fix(api): raise the course prompt limit to 12000 (#3408) A course prompt carries the scaffold code the learner starts from, the reference answer, and the rules that decide when the course is complete, so exercise and multi-sprite courses run several times longer than a plain lesson. The longest published course prompt is 8763 characters, past the 4000 the contract has allowed since #3279. Mirrors goplus/builder-backend#329, which relaxes the server-side check. Co-authored-by: Ethanlita <a1198537235@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(product): add product design for the new tutorial course system (#3402) * docs(product): add product design for the new tutorial course system Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: split tutorial v2 into product design and develop notes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(product): add English version of tutorial v2 design Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(spx-gui): timeout pre-run static checks (#3410) * fix(spx-gui): timeout pre-run static checks * fix(spx-gui): capture static check timeouts * fix(spx-gui): bound pre-run static checks * refactor(spx-gui): name pre-run static checks * refactor(spx-gui): simplify static check timeout handling * docs(spx-gui): explain monitor check cancellation * fix(spx-gui): include capture context in Sentry * fix(spx-gui): scope static check timeouts to LS calls * refactor(spx-gui): reuse static check timeout helper * refactor(spx-gui): reuse timeout helper for thumbnails * docs(spx-gui): explain Code Editor timeout * fix(spx-gui): refine Code Editor timeout handling * refactor(spx-gui): merge timeout signals in helper * fix(spx-gui): race external timeout signals * docs(spx-gui): explain pre-run check errors * docs(spx-gui): clarify pre-run error handling * fix(account): preserve client IP through trusted proxies (#3389) Trust only the jfcs-k8s-qa1 ingress controller addresses when restoring `X-Real-IP` before proxying Account API requests. This preserves the client address across the hosted sign-in facade for backend IP-based limits. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * fix(editor): avoid quadratic input slot filtering (#3415) Upgrade xgolsw to collect large input-slot sets efficiently and rely on its pairwise non-overlap guarantee. Remove the redundant editor-side containment pass so projects with many editable literals stay responsive. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> --------- Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> Co-authored-by: Aofei Sheng <aofei@aofeisheng.com> Co-authored-by: Ethanlita <65023676+Ethanlita@users.noreply.github.com> Co-authored-by: Ethanlita <a1198537235@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Sync `main` to `dev` (#3398) * chore(deps): bump spx to 3.1.0 (#3397) * chore(deps): bump spx to 3.1.0 * fix(deps): add spx 3.1.0 module checksums * fix(deps): record spx transitive module requirement * fix(deps): tidy spx 3.1.0 module graph * feat: support project font persistence (#3362) * feat: support project font persistence * chore: use SPX preset font assets * fix: address project font review comments * docs: link deferred project font rendering issue * fix(ai): handle interaction termination consistently (#3399) Schedule history management from the interaction cleanup path so completed sequences are archived even when they return early. Run archive work in an owner-scoped coroutine so it survives caller completion without outliving the game. Report an error when an interaction exhausts its turn limit instead of silently treating an unfinished sequence as complete. Add regression coverage for archive lifecycle behavior and turn-limit failure reporting. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * feat(account): add Firefox browser hijacking (#3404) Support Firefox WebDriver BiDi alongside Chrome DevTools Protocol for local Account development. Preserve redirects for sign-in and identity provider callbacks across both browsers. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * fix(api): raise the course prompt limit to 12000 (#3408) A course prompt carries the scaffold code the learner starts from, the reference answer, and the rules that decide when the course is complete, so exercise and multi-sprite courses run several times longer than a plain lesson. The longest published course prompt is 8763 characters, past the 4000 the contract has allowed since #3279. Mirrors goplus/builder-backend#329, which relaxes the server-side check. Co-authored-by: Ethanlita <a1198537235@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * docs(product): add product design for the new tutorial course system (#3402) * docs(product): add product design for the new tutorial course system Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: split tutorial v2 into product design and develop notes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(product): add English version of tutorial v2 design Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * fix(spx-gui): timeout pre-run static checks (#3410) * fix(spx-gui): timeout pre-run static checks * fix(spx-gui): capture static check timeouts * fix(spx-gui): bound pre-run static checks * refactor(spx-gui): name pre-run static checks * refactor(spx-gui): simplify static check timeout handling * docs(spx-gui): explain monitor check cancellation * fix(spx-gui): include capture context in Sentry * fix(spx-gui): scope static check timeouts to LS calls * refactor(spx-gui): reuse static check timeout helper * refactor(spx-gui): reuse timeout helper for thumbnails * docs(spx-gui): explain Code Editor timeout * fix(spx-gui): refine Code Editor timeout handling * refactor(spx-gui): merge timeout signals in helper * fix(spx-gui): race external timeout signals * docs(spx-gui): explain pre-run check errors * docs(spx-gui): clarify pre-run error handling * fix(account): preserve client IP through trusted proxies (#3389) Trust only the jfcs-k8s-qa1 ingress controller addresses when restoring `X-Real-IP` before proxying Account API requests. This preserves the client address across the hosted sign-in facade for backend IP-based limits. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> * fix(editor): avoid quadratic input slot filtering (#3415) Upgrade xgolsw to collect large input-slot sets efficiently and rely on its pairwise non-overlap guarantee. Remove the redundant editor-side containment pass so projects with many editable literals stay responsive. Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> --------- Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> Co-authored-by: Hanxing Yang <nighca@live.cn> Co-authored-by: Ethanlita <65023676+Ethanlita@users.noreply.github.com> Co-authored-by: Ethanlita <a1198537235@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Closes #3343
Adds Builder support for SPX project fonts:
assets/fonts/<family>/and thefontPreferencesarray inassets/index.json;.xbptemplate;Editor previews intentionally do not consume project fonts or preferences in this PR. That integration is tracked separately in #3366 and will be implemented by #3348 after its large-font SVG rendering performance issue is resolved.