Skip to content

Commit 726e8f5

Browse files
author
Harry Terkelsen
authored
Add Helvetica and sans-serif as fallback font families (flutter#13784)
* Add Helvetica and sans-serif as fallback font families This prevents us from using an ugly serif default font when the requested font isn't available. * Use Arial when not on iOS
1 parent 164df8e commit 726e8f5

File tree

4 files changed

+58
-13
lines changed

4 files changed

+58
-13
lines changed

lib/web_ui/lib/src/engine/text/paragraph.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,7 +1080,7 @@ void _applyParagraphStyleToElement({
10801080
style._fontStyle == ui.FontStyle.normal ? 'normal' : 'italic';
10811081
}
10821082
if (style._effectiveFontFamily != null) {
1083-
cssStyle.fontFamily = quoteFontFamily(style._effectiveFontFamily);
1083+
cssStyle.fontFamily = canonicalizeFontFamily(style._effectiveFontFamily);
10841084
}
10851085
} else {
10861086
if (style._textAlign != previousStyle._textAlign) {
@@ -1106,7 +1106,7 @@ void _applyParagraphStyleToElement({
11061106
: null;
11071107
}
11081108
if (style._fontFamily != previousStyle._fontFamily) {
1109-
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
1109+
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
11101110
}
11111111
}
11121112
}
@@ -1146,11 +1146,11 @@ void _applyTextStyleToElement({
11461146
// consistently use Ahem font.
11471147
if (isSpan && !ui.debugEmulateFlutterTesterEnvironment) {
11481148
if (style._fontFamily != null) {
1149-
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
1149+
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
11501150
}
11511151
} else {
11521152
if (style._effectiveFontFamily != null) {
1153-
cssStyle.fontFamily = quoteFontFamily(style._effectiveFontFamily);
1153+
cssStyle.fontFamily = canonicalizeFontFamily(style._effectiveFontFamily);
11541154
}
11551155
}
11561156
if (style._letterSpacing != null) {
@@ -1184,7 +1184,7 @@ void _applyTextStyleToElement({
11841184
: null;
11851185
}
11861186
if (style._fontFamily != previousStyle._fontFamily) {
1187-
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
1187+
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
11881188
}
11891189
if (style._letterSpacing != previousStyle._letterSpacing) {
11901190
cssStyle.letterSpacing = '${style._letterSpacing}px';

lib/web_ui/lib/src/engine/text/ruler.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class ParagraphGeometricStyle {
8686
result.write(DomRenderer.defaultFontSize);
8787
}
8888
result.write(' ');
89-
result.write(quoteFontFamily(effectiveFontFamily));
89+
result.write(canonicalizeFontFamily(effectiveFontFamily));
9090

9191
return result.toString();
9292
}
@@ -227,7 +227,7 @@ class TextDimensions {
227227
void applyStyle(ParagraphGeometricStyle style) {
228228
_element.style
229229
..fontSize = style.fontSize != null ? '${style.fontSize.floor()}px' : null
230-
..fontFamily = quoteFontFamily(style.effectiveFontFamily)
230+
..fontFamily = canonicalizeFontFamily(style.effectiveFontFamily)
231231
..fontWeight =
232232
style.fontWeight != null ? fontWeightToCss(style.fontWeight) : null
233233
..fontStyle = style.fontStyle != null

lib/web_ui/lib/src/engine/util.dart

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,20 @@ const Set<String> _genericFontFamilies = <String>{
258258
'fangsong',
259259
};
260260

261-
/// Wraps a font family in quotes unless it is a generic font family.
262-
String quoteFontFamily(String fontFamily) {
261+
/// A default fallback font family in case an unloaded font has been requested.
262+
///
263+
/// For iOS, default to Helvetica, where it should be available, otherwise
264+
/// default to Arial.
265+
final String _fallbackFontFamily =
266+
operatingSystem == OperatingSystem.iOs ? 'Helvetica' : 'Arial';
267+
268+
/// Create a font-family string appropriate for CSS.
269+
///
270+
/// If the given [fontFamily] is a generic font-family, then just return it.
271+
/// Otherwise, wrap the family name in quotes and add a fallback font family.
272+
String canonicalizeFontFamily(String fontFamily) {
263273
if (_genericFontFamilies.contains(fontFamily)) {
264274
return fontFamily;
265275
}
266-
return '"$fontFamily"';
276+
return '"$fontFamily", $_fallbackFontFamily, sans-serif';
267277
}

lib/web_ui/test/text_test.dart

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,43 @@ void main() async {
241241
expect(paragraph.plainText, isNull);
242242
final List<SpanElement> spans =
243243
paragraph.paragraphElement.querySelectorAll('span');
244-
expect(spans[0].style.fontFamily, 'Ahem');
244+
expect(spans[0].style.fontFamily, 'Ahem, Arial, sans-serif');
245245
// The nested span here should not set it's family to default sans-serif.
246-
expect(spans[1].style.fontFamily, 'Ahem');
246+
expect(spans[1].style.fontFamily, 'Ahem, Arial, sans-serif');
247+
});
248+
249+
test('adds Arial and sans-serif as fallback fonts', () {
250+
// Set this to false so it doesn't default to 'Ahem' font.
251+
debugEmulateFlutterTesterEnvironment = false;
252+
253+
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
254+
fontFamily: 'SomeFont',
255+
fontSize: 12.0,
256+
));
257+
258+
builder.addText('Hello');
259+
260+
final EngineParagraph paragraph = builder.build();
261+
expect(paragraph.paragraphElement.style.fontFamily, 'SomeFont, Arial, sans-serif');
262+
263+
debugEmulateFlutterTesterEnvironment = true;
264+
});
265+
266+
test('does not add fallback fonts to generic families', () {
267+
// Set this to false so it doesn't default to 'Ahem' font.
268+
debugEmulateFlutterTesterEnvironment = false;
269+
270+
final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
271+
fontFamily: 'serif',
272+
fontSize: 12.0,
273+
));
274+
275+
builder.addText('Hello');
276+
277+
final EngineParagraph paragraph = builder.build();
278+
expect(paragraph.paragraphElement.style.fontFamily, 'serif');
279+
280+
debugEmulateFlutterTesterEnvironment = true;
247281
});
248282

249283
test('can set font families that need to be quoted', () {
@@ -258,10 +292,11 @@ void main() async {
258292
builder.addText('Hello');
259293

260294
final EngineParagraph paragraph = builder.build();
261-
expect(paragraph.paragraphElement.style.fontFamily, '"MyFont 2000"');
295+
expect(paragraph.paragraphElement.style.fontFamily, '"MyFont 2000", Arial, sans-serif');
262296

263297
debugEmulateFlutterTesterEnvironment = true;
264298
});
299+
265300
group('TextRange', () {
266301
test('empty ranges are correct', () {
267302
const TextRange range = TextRange(start: -1, end: -1);

0 commit comments

Comments
 (0)