From 726e8f54dbddd294dec7d850bf08ac91b04c80bd Mon Sep 17 00:00:00 2001 From: Harry Terkelsen Date: Mon, 11 Nov 2019 16:54:28 -0800 Subject: [PATCH] Add Helvetica and sans-serif as fallback font families (#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 --- lib/web_ui/lib/src/engine/text/paragraph.dart | 10 ++--- lib/web_ui/lib/src/engine/text/ruler.dart | 4 +- lib/web_ui/lib/src/engine/util.dart | 16 ++++++-- lib/web_ui/test/text_test.dart | 41 +++++++++++++++++-- 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text/paragraph.dart b/lib/web_ui/lib/src/engine/text/paragraph.dart index 2aca66ff2..7b6faaf3e 100644 --- a/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -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) { @@ -1106,7 +1106,7 @@ void _applyParagraphStyleToElement({ : null; } if (style._fontFamily != previousStyle._fontFamily) { - cssStyle.fontFamily = quoteFontFamily(style._fontFamily); + cssStyle.fontFamily = canonicalizeFontFamily(style._fontFamily); } } } @@ -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) { @@ -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'; diff --git a/lib/web_ui/lib/src/engine/text/ruler.dart b/lib/web_ui/lib/src/engine/text/ruler.dart index 41cbdf0dc..e8c1944af 100644 --- a/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/lib/web_ui/lib/src/engine/text/ruler.dart @@ -86,7 +86,7 @@ class ParagraphGeometricStyle { result.write(DomRenderer.defaultFontSize); } result.write(' '); - result.write(quoteFontFamily(effectiveFontFamily)); + result.write(canonicalizeFontFamily(effectiveFontFamily)); return result.toString(); } @@ -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 diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 715d6a30e..087136408 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -258,10 +258,20 @@ const Set _genericFontFamilies = { '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'; } diff --git a/lib/web_ui/test/text_test.dart b/lib/web_ui/test/text_test.dart index eecebd711..6889e890f 100644 --- a/lib/web_ui/test/text_test.dart +++ b/lib/web_ui/test/text_test.dart @@ -241,9 +241,43 @@ void main() async { expect(paragraph.plainText, isNull); final List 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', () { @@ -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); -- GitLab