Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Make sure to include all split fonts #56733

Merged
auto-submit merged 3 commits into from
Nov 21, 2024
Merged

[web] Make sure to include all split fonts #56733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a971263
[web] Make sure to include all split fonts
mdebbar Nov 20, 2024
99176e8
More restrictive regex
mdebbar Nov 20, 2024
568868d
fix test
mdebbar Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ deps = {
'packages': [
{
'package': 'flutter/flutter_font_fallbacks',
'version': '8a753fd2150c398a5777a7fdb24fc9d4d5fe8015088c5237b61cf0ff26653fd0'
'version': '44bd38be0bc8c189a397ca6dd6f737746a9e0c6117b96a8f84f1edf6acd1206b'
}
],
'dep_type': 'cipd',
Expand Down
20 changes: 5 additions & 15 deletions lib/web_ui/dev/roll_fallback_fonts.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import 'dart:typed_data';
import 'package:args/command_runner.dart';
import 'package:convert/convert.dart';
import 'package:crypto/crypto.dart' as crypto;
import 'package:csslib/parser.dart' as csslib;
import 'package:csslib/visitor.dart' show StyleSheet, UriTerm, Visitor;
import 'package:http/http.dart' as http;
import 'package:path/path.dart' as path;

Expand Down Expand Up @@ -346,13 +344,14 @@ OTHER DEALINGS IN THE FONT SOFTWARE.
'Chrome/112.0.0.0 Safari/537.36'
});
final String cssString = response.body;
final StyleSheet stylesheet = csslib.parse(cssString);
final UriCollector uriCollector = UriCollector();
stylesheet.visit(uriCollector);
// Match the patterns that look like:
// `src: url(...some url...)`
final r = RegExp(r'src:\s*url\((https?://[^)]+?\.woff2)\)');
int familyCount = 0;
// Give each font shard a unique family name.
for (final Uri uri in uriCollector.uris) {
for (final match in r.allMatches(cssString)) {
Copy link
Member

@ditman ditman Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check that the match is a proper font URI? (like ending in known extensions or something?)

Copy link
Contributor Author

@mdebbar mdebbar Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing that in the if, I'll make the regex more restrictive (i.e. starts with https?:// and ends with \.woff2).

Copy link
Member

@ditman ditman Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM! Something I like to do when I write a complex RegEx is to create it here:

And save it (they support "Github login") with some Tests there. It's pretty fancy and IMO makes the regex explanation very legible!

(For example: https://regexr.com/88q3g)

final String family = '$font $familyCount';
final Uri uri = Uri.parse(match.group(1)!);
processedFonts.add((
family: family,
uri: uri,
Expand All @@ -364,15 +363,6 @@ OTHER DEALINGS IN THE FONT SOFTWARE.
}
}

class UriCollector extends Visitor {
final List<Uri> uris = <Uri>[];

@override
void visitUriTerm(UriTerm uriTerm) {
uris.add(Uri.parse(uriTerm.value as String));
}
}

/// Fonts that should be downloaded directly from the Google Fonts API.
const List<String> apiFallbackFonts = <String>[
'Noto Sans',
Expand Down
Loading