Skip to content

Commit 3ad8c42

Browse files
nturgutnturgut
authored
do not wrap font family name (flutter#12801)
* do not wrap font family name in webkit otherwise icons not show on safari 13 (both IOS and desktop) * Changing the font loading to work in all browsers. * Documentation, renaming, gramatical/spelling error related PR comments addressed. Regexp will be addressed in the next commit. * Changing regular expressions to look simpler. Adding more unit tests.
1 parent 2d4f4bf commit 3ad8c42

File tree

2 files changed

+208
-20
lines changed

2 files changed

+208
-20
lines changed

‎lib/web_ui/lib/src/engine/text/font_collection.dart‎

Lines changed: 68 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ const String _robotoFontUrl = 'packages/ui/assets/Roboto-Regular.ttf';
1616
/// font manifest. If test fonts are enabled, then call
1717
/// [registerTestFonts] as well.
1818
class FontCollection {
19-
_FontManager _assetFontManager;
20-
_FontManager _testFontManager;
19+
FontManager _assetFontManager;
20+
FontManager _testFontManager;
2121

2222
/// Reads the font manifest using the [assetManager] and registers all of the
2323
/// fonts declared within.
@@ -49,7 +49,7 @@ class FontCollection {
4949
}
5050

5151
if (supportsFontLoadingApi) {
52-
_assetFontManager = _FontManager();
52+
_assetFontManager = FontManager();
5353
} else {
5454
_assetFontManager = _PolyfillFontManager();
5555
}
@@ -75,7 +75,7 @@ class FontCollection {
7575

7676
/// Registers fonts that are used by tests.
7777
void debugRegisterTestFonts() {
78-
_testFontManager = _FontManager();
78+
_testFontManager = FontManager();
7979
_testFontManager.registerAsset(
8080
_ahemFontFamily, 'url($_ahemFontUrl)', const <String, String>{});
8181
_testFontManager.registerAsset(
@@ -100,40 +100,88 @@ class FontCollection {
100100
}
101101

102102
/// Manages a collection of fonts and ensures they are loaded.
103-
class _FontManager {
103+
class FontManager {
104104
final List<Future<void>> _fontLoadingFutures = <Future<void>>[];
105105

106-
factory _FontManager() {
106+
// Regular expression to detect a string with no punctuations.
107+
// For example font family 'Ahem!' does not fall into this category
108+
// so the family name will be wrapped in quotes.
109+
static final RegExp notPunctuation =
110+
RegExp(r"[a-z0-9\s]+", caseSensitive: false);
111+
// Regular expression to detect tokens starting with a digit.
112+
// For example font family 'Goudy Bookletter 1911' falls into this
113+
// category.
114+
static final RegExp startWithDigit = RegExp(r"\b\d");
115+
116+
factory FontManager() {
107117
if (supportsFontLoadingApi) {
108-
return _FontManager._();
118+
return FontManager._();
109119
} else {
110120
return _PolyfillFontManager();
111121
}
112122
}
113123

114-
_FontManager._();
115-
124+
FontManager._();
125+
126+
/// Registers assets to Flutter Web Engine.
127+
///
128+
/// Browsers and browsers versions differ significantly on how a valid font
129+
/// family name should be formatted. Notable issues are:
130+
///
131+
/// Safari 12 and Firefox crash if you create a [html.FontFace] with a font
132+
/// family that is not correct CSS syntax. Font family names with invalid
133+
/// characters are accepted accepted on these browsers, when wrapped it in
134+
/// quotes.
135+
///
136+
/// Additionally, for Safari 12 to work [html.FontFace] name should be
137+
/// loaded correctly on the first try.
138+
///
139+
/// A font in Chrome is not usable other than inside a '<p>' tag, if a
140+
/// [html.FontFace] is loaded wrapped with quotes. Unlike Safari 12 if a
141+
/// valid version of the font is also loaded afterwards it will show
142+
/// that font normally.
143+
///
144+
/// In Safari 13 the [html.FontFace] should be loaded with unquoted family
145+
/// names.
146+
///
147+
/// In order to avoid all these browser compatibility issues this method:
148+
/// * Detects the family names that might cause a conflict.
149+
/// * Loads it with the quotes.
150+
/// * Loads it again without the quotes.
151+
/// * For all the other family names [html.FontFace] is loaded only once.
152+
///
153+
/// See also:
154+
///
155+
/// * https://developer.mozilla.org/en-US/docs/Web/CSS/font-family#Valid_family_names
156+
/// * https://drafts.csswg.org/css-fonts-3/#font-family-prop
116157
void registerAsset(
117158
String family,
118159
String asset,
119160
Map<String, String> descriptors,
120161
) {
121-
// Safari and Firefox crash if you create a [html.FontFace] with a font
122-
// family that is not correct CSS syntax. To ensure the font family is
123-
// accepted on these browsers, wrap it in quotes.
124-
// See: https://drafts.csswg.org/css-fonts-3/#font-family-prop
125-
if (browserEngine == BrowserEngine.webkit || browserEngine == BrowserEngine.firefox) {
126-
family = "'$family'";
162+
if (startWithDigit.hasMatch(family) ||
163+
notPunctuation.stringMatch(family) != family) {
164+
// Load a font family name with special characters once here wrapped in
165+
// quotes.
166+
_loadFontFace('\'$family\'', asset, descriptors);
127167
}
168+
// Load all fonts, without quoted family names.
169+
_loadFontFace(family, asset, descriptors);
170+
}
171+
172+
void _loadFontFace(
173+
String family,
174+
String asset,
175+
Map<String, String> descriptors,
176+
) {
128177
// try/catch because `new FontFace` can crash with an improper font family.
129178
try {
130179
final html.FontFace fontFace = html.FontFace(family, asset, descriptors);
131-
_fontLoadingFutures.add(fontFace
132-
.load()
133-
.then((_) => html.document.fonts.add(fontFace), onError: (dynamic e) {
180+
_fontLoadingFutures.add(fontFace.load().then((_) {
181+
html.document.fonts.add(fontFace);
182+
}, onError: (dynamic e) {
134183
html.window.console
135184
.warn('Error while trying to load font family "$family":\n$e');
136-
return null;
137185
}));
138186
} catch (e) {
139187
html.window.console
@@ -153,7 +201,7 @@ class _FontManager {
153201
/// The CSS Font Loading API is not implemented in IE 11 or Edge. To tell if a
154202
/// font is loaded, we continuously measure some text using that font until the
155203
/// width changes.
156-
class _PolyfillFontManager extends _FontManager {
204+
class _PolyfillFontManager extends FontManager {
157205
_PolyfillFontManager() : super._();
158206

159207
/// A String containing characters whose width varies greatly between fonts.

‎lib/web_ui/test/text/font_collection_test.dart‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'dart:html' as html;
6+
7+
import 'package:ui/src/engine.dart';
8+
9+
import 'package:test/test.dart';
10+
11+
void main() {
12+
group('$FontManager', () {
13+
FontManager fontManager;
14+
const String _testFontUrl = 'packages/ui/assets/ahem.ttf';
15+
16+
setUp(() {
17+
fontManager = FontManager();
18+
});
19+
20+
tearDown(() {
21+
html.document.fonts.clear();
22+
});
23+
24+
test('Register Asset with no special characters', () async {
25+
final String _testFontFamily = "Ahem";
26+
final List<String> fontFamilyList = List<String>();
27+
28+
fontManager.registerAsset(
29+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
30+
await fontManager.ensureFontsLoaded();
31+
html.document.fonts
32+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
33+
fontFamilyList.add(f.family);
34+
});
35+
36+
expect(fontFamilyList.length, equals(1));
37+
expect(fontFamilyList.first, 'Ahem');
38+
});
39+
40+
test('Register Asset with white space in the family name', () async {
41+
final String _testFontFamily = "Ahem ahem ahem";
42+
final List<String> fontFamilyList = List<String>();
43+
44+
fontManager.registerAsset(
45+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
46+
await fontManager.ensureFontsLoaded();
47+
html.document.fonts
48+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
49+
fontFamilyList.add(f.family);
50+
});
51+
52+
expect(fontFamilyList.length, equals(1));
53+
expect(fontFamilyList.first, 'Ahem ahem ahem');
54+
});
55+
56+
test('Register Asset with capital case letters', () async {
57+
final String _testFontFamily = "AhEm";
58+
final List<String> fontFamilyList = List<String>();
59+
60+
fontManager.registerAsset(
61+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
62+
await fontManager.ensureFontsLoaded();
63+
html.document.fonts
64+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
65+
fontFamilyList.add(f.family);
66+
});
67+
68+
expect(fontFamilyList.length, equals(1));
69+
expect(fontFamilyList.first, 'AhEm');
70+
});
71+
72+
test('Register Asset twice with special character slash', () async {
73+
final String _testFontFamily = '/Ahem';
74+
final List<String> fontFamilyList = List<String>();
75+
76+
fontManager.registerAsset(
77+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
78+
await fontManager.ensureFontsLoaded();
79+
html.document.fonts
80+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
81+
fontFamilyList.add(f.family);
82+
});
83+
84+
expect(fontFamilyList.length, equals(2));
85+
expect(fontFamilyList, contains('\'/Ahem\''));
86+
expect(fontFamilyList, contains('/Ahem'));
87+
});
88+
89+
test('Register Asset twice with exclamation mark', () async {
90+
final String _testFontFamily = 'Ahem!!ahem';
91+
final List<String> fontFamilyList = List<String>();
92+
93+
fontManager.registerAsset(
94+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
95+
await fontManager.ensureFontsLoaded();
96+
html.document.fonts
97+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
98+
fontFamilyList.add(f.family);
99+
});
100+
101+
expect(fontFamilyList.length, equals(2));
102+
expect(fontFamilyList, contains('\'Ahem!!ahem\''));
103+
expect(fontFamilyList, contains('Ahem!!ahem'));
104+
});
105+
106+
test('Register Asset twice with coma', () async {
107+
final String _testFontFamily = 'Ahem ,ahem';
108+
final List<String> fontFamilyList = List<String>();
109+
110+
fontManager.registerAsset(
111+
_testFontFamily, 'url($_testFontUrl)', const <String, String>{});
112+
await fontManager.ensureFontsLoaded();
113+
html.document.fonts
114+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
115+
fontFamilyList.add(f.family);
116+
});
117+
118+
expect(fontFamilyList.length, equals(2));
119+
expect(fontFamilyList, contains('\'Ahem ,ahem\''));
120+
expect(fontFamilyList, contains('Ahem ,ahem'));
121+
});
122+
123+
test('Register Asset twice with a digit at the start of a token', () async {
124+
final String testFontFamily = 'Ahem 1998';
125+
final List<String> fontFamilyList = List<String>();
126+
127+
fontManager.registerAsset(
128+
testFontFamily, 'url($_testFontUrl)', const <String, String>{});
129+
await fontManager.ensureFontsLoaded();
130+
html.document.fonts
131+
.forEach((html.FontFace f, html.FontFace f2, html.FontFaceSet s) {
132+
fontFamilyList.add(f.family);
133+
});
134+
135+
expect(fontFamilyList.length, equals(2));
136+
expect(fontFamilyList, contains('Ahem 1998'));
137+
expect(fontFamilyList, contains('\'Ahem 1998\''));
138+
});
139+
});
140+
}

0 commit comments

Comments
 (0)