Skip to content

Commit

Permalink
Add Helvetica and sans-serif as fallback font families (flutter#13784)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Harry Terkelsen committed Nov 12, 2019
1 parent 164df8e commit 726e8f5
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 13 deletions.
10 changes: 5 additions & 5 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
Expand Up @@ -1080,7 +1080,7 @@ void _applyParagraphStyleToElement({
style._fontStyle == ui.FontStyle.normal ? 'normal' : 'italic';
}
if (style._effectiveFontFamily != null) {
cssStyle.fontFamily = quoteFontFamily(style._effectiveFontFamily);
cssStyle.fontFamily = canonicalizeFontFamily(style._effectiveFontFamily);
}
} else {
if (style._textAlign != previousStyle._textAlign) {
Expand All @@ -1106,7 +1106,7 @@ void _applyParagraphStyleToElement({
: null;
}
if (style._fontFamily != previousStyle._fontFamily) {
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
}
}
}
Expand Down Expand Up @@ -1146,11 +1146,11 @@ void _applyTextStyleToElement({
// consistently use Ahem font.
if (isSpan && !ui.debugEmulateFlutterTesterEnvironment) {
if (style._fontFamily != null) {
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
}
} else {
if (style._effectiveFontFamily != null) {
cssStyle.fontFamily = quoteFontFamily(style._effectiveFontFamily);
cssStyle.fontFamily = canonicalizeFontFamily(style._effectiveFontFamily);
}
}
if (style._letterSpacing != null) {
Expand Down Expand Up @@ -1184,7 +1184,7 @@ void _applyTextStyleToElement({
: null;
}
if (style._fontFamily != previousStyle._fontFamily) {
cssStyle.fontFamily = quoteFontFamily(style._fontFamily);
cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily);
}
if (style._letterSpacing != previousStyle._letterSpacing) {
cssStyle.letterSpacing = '${style._letterSpacing}px';
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/text/ruler.dart
Expand Up @@ -86,7 +86,7 @@ class ParagraphGeometricStyle {
result.write(DomRenderer.defaultFontSize);
}
result.write(' ');
result.write(quoteFontFamily(effectiveFontFamily));
result.write(canonicalizeFontFamily(effectiveFontFamily));

return result.toString();
}
Expand Down Expand Up @@ -227,7 +227,7 @@ class TextDimensions {
void applyStyle(ParagraphGeometricStyle style) {
_element.style
..fontSize = style.fontSize != null ? '${style.fontSize.floor()}px' : null
..fontFamily = quoteFontFamily(style.effectiveFontFamily)
..fontFamily = canonicalizeFontFamily(style.effectiveFontFamily)
..fontWeight =
style.fontWeight != null ? fontWeightToCss(style.fontWeight) : null
..fontStyle = style.fontStyle != null
Expand Down
16 changes: 13 additions & 3 deletions lib/web_ui/lib/src/engine/util.dart
Expand Up @@ -258,10 +258,20 @@ const Set<String> _genericFontFamilies = <String>{
'fangsong',
};

/// Wraps a font family in quotes unless it is a generic font family.
String quoteFontFamily(String fontFamily) {
/// A default fallback font family in case an unloaded font has been requested.
///
/// For iOS, default to Helvetica, where it should be available, otherwise
/// default to Arial.
final String _fallbackFontFamily =
operatingSystem == OperatingSystem.iOs ? 'Helvetica' : 'Arial';

/// Create a font-family string appropriate for CSS.
///
/// If the given [fontFamily] is a generic font-family, then just return it.
/// Otherwise, wrap the family name in quotes and add a fallback font family.
String canonicalizeFontFamily(String fontFamily) {
if (_genericFontFamilies.contains(fontFamily)) {
return fontFamily;
}
return '"$fontFamily"';
return '"$fontFamily", $_fallbackFontFamily, sans-serif';
}
41 changes: 38 additions & 3 deletions lib/web_ui/test/text_test.dart
Expand Up @@ -241,9 +241,43 @@ void main() async {
expect(paragraph.plainText, isNull);
final List<SpanElement> spans =
paragraph.paragraphElement.querySelectorAll('span');
expect(spans[0].style.fontFamily, 'Ahem');
expect(spans[0].style.fontFamily, 'Ahem, Arial, sans-serif');
// The nested span here should not set it's family to default sans-serif.
expect(spans[1].style.fontFamily, 'Ahem');
expect(spans[1].style.fontFamily, 'Ahem, Arial, sans-serif');
});

test('adds Arial and sans-serif as fallback fonts', () {
// Set this to false so it doesn't default to 'Ahem' font.
debugEmulateFlutterTesterEnvironment = false;

final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
fontFamily: 'SomeFont',
fontSize: 12.0,
));

builder.addText('Hello');

final EngineParagraph paragraph = builder.build();
expect(paragraph.paragraphElement.style.fontFamily, 'SomeFont, Arial, sans-serif');

debugEmulateFlutterTesterEnvironment = true;
});

test('does not add fallback fonts to generic families', () {
// Set this to false so it doesn't default to 'Ahem' font.
debugEmulateFlutterTesterEnvironment = false;

final ParagraphBuilder builder = ParagraphBuilder(ParagraphStyle(
fontFamily: 'serif',
fontSize: 12.0,
));

builder.addText('Hello');

final EngineParagraph paragraph = builder.build();
expect(paragraph.paragraphElement.style.fontFamily, 'serif');

debugEmulateFlutterTesterEnvironment = true;
});

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

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

debugEmulateFlutterTesterEnvironment = true;
});

group('TextRange', () {
test('empty ranges are correct', () {
const TextRange range = TextRange(start: -1, end: -1);
Expand Down

0 comments on commit 726e8f5

Please sign in to comment.