-
Notifications
You must be signed in to change notification settings - Fork 512
[ohos] Fix font fallback crash by using new text module API on API14+ #3238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add API version check using deviceInfo.sdkApiVersion - For API14+: use new text.getSystemFontFullNamesByType() and text.getFontDescriptorByFullName() APIs - Fallback to legacy font.getUIFontConfig() API if new API fails or on lower API versions - Add null checks for fontConfig and fallbackGroups - Fix paths.concat() bug (missing assignment) Related: #3232
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3238 +/- ##
==========================================
- Coverage 77.04% 77.02% -0.03%
==========================================
Files 413 413
Lines 21922 21922
Branches 6298 6298
==========================================
- Hits 16890 16885 -5
- Misses 3821 3825 +4
- Partials 1211 1212 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ohos/libpag/src/main/ets/PAGInit.ets
Outdated
| let fontDescriptor = await text.getFontDescriptorByFullName(fontName, text.SystemFontType.ALL); | ||
| if (fontDescriptor && fontDescriptor.path) { | ||
| paths.push(fontDescriptor.path); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
性能优化建议:当前串行 await 每个字体描述符,系统字体较多时初始化会较慢。建议使用 Promise.all() 并行获取:
let descriptors = await Promise.all(fontNames.map(fontName =>
text.getFontDescriptorByFullName(fontName, text.SystemFontType.ALL)
));
let paths = descriptors
.filter(d => d && d.path)
.map(d => d.path);
ohos/libpag/src/main/ets/PAGInit.ets
Outdated
| try { | ||
| let fontNames: string[] = await text.getSystemFontFullNamesByType(text.SystemFontType.ALL); | ||
| if (fontNames && fontNames.length > 0) { | ||
| let paths = new Array<string>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
代码简化建议:使用字面量初始化更简洁,如 let paths: string[] = [];
ohos/libpag/src/main/ets/PAGInit.ets
Outdated
| for (let fontName of fontNames) { | ||
| let fontDescriptor = await text.getFontDescriptorByFullName(fontName, text.SystemFontType.ALL); | ||
| if (fontDescriptor && fontDescriptor.path) { | ||
| paths.push(fontDescriptor.path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
潜在风险:同一字体路径可能被多次添加(如同一字体在不同 fallbackGroup 出现),建议考虑去重处理。
domchen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Related: #3232