From 42c2dc35ad779baf698c53cae6da14e604aee01c Mon Sep 17 00:00:00 2001 From: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com> Date: Thu, 23 Mar 2023 15:24:36 -0700 Subject: [PATCH] Fix default test font on web (flutter/engine#40479) Fix default test font on web --- .../lib/web_ui/lib/src/engine/canvaskit/text.dart | 2 +- .../lib/web_ui/lib/src/engine/text/paragraph.dart | 2 +- .../lib/web_ui/lib/src/engine/text/ruler.dart | 7 +++++-- .../flutter_tester_emulation_golden_test.dart | 2 +- .../flutter/lib/web_ui/test/canvaskit/text_test.dart | 2 +- .../lib/web_ui/test/html/paragraph/helper.dart | 2 +- .../test/text/canvas_paragraph_builder_test.dart | 8 ++++---- .../web_ui/test/text/layout_service_plain_test.dart | 12 ++++++------ .../web_ui/test/text/layout_service_rich_test.dart | 6 +++--- engine/src/flutter/lib/web_ui/test/text_test.dart | 12 ++++++------ 10 files changed, 29 insertions(+), 26 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart index e885bd7da4f..fd6f81e4e11 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/text.dart @@ -16,7 +16,7 @@ import 'skia_object_cache.dart'; import 'text_fragmenter.dart'; import 'util.dart'; -final List _testFonts = ['Ahem', 'FlutterTest']; +final List _testFonts = ['FlutterTest', 'Ahem']; String? _effectiveFontFamily(String? fontFamily) { return ui.debugEmulateFlutterTesterEnvironment && !_testFonts.contains(fontFamily) ? _testFonts.first diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart index 0ac773576ed..0b32f66661a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/paragraph.dart @@ -464,7 +464,7 @@ class EngineTextStyle implements ui.TextStyle { final ui.Paint? foreground; final List? shadows; - static final List _testFonts = ['Ahem', 'FlutterTest']; + static final List _testFonts = ['FlutterTest', 'Ahem']; String get effectiveFontFamily { final String fontFamily = this.fontFamily.isEmpty ? FlutterViewEmbedder.defaultFontFamily : this.fontFamily; // In the flutter tester environment, we use predictable-size test fonts. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/ruler.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/ruler.dart index bce0e6bbd08..b18ad73fb8b 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/ruler.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/ruler.dart @@ -119,8 +119,11 @@ class TextDimensions { ..fontFamily = canonicalizeFontFamily(fontFamily)!; final double? height = textHeightStyle.height; - if (height != null) { - style.lineHeight = height.toString(); + // Workaround the rounding introduced by https://github.com/flutter/flutter/issues/122066 + // in tests. + final double? effectiveLineHeight = height ?? (fontFamily == 'FlutterTest' ? 1.0 : null); + if (effectiveLineHeight != null) { + style.lineHeight = effectiveLineHeight.toString(); } _invalidateBoundsCache(); } diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/flutter_tester_emulation_golden_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/flutter_tester_emulation_golden_test.dart index add30b29ca9..81a2c340ff7 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/flutter_tester_emulation_golden_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/flutter_tester_emulation_golden_test.dart @@ -21,7 +21,7 @@ void testMain() { group('flutter_tester emulation', () { setUpCanvasKitTest(); - test('defaults to Ahem font family', + test('defaults to FlutterTest font family', () async { final CkPictureRecorder recorder = CkPictureRecorder(); final CkCanvas canvas = recorder.beginRecording(kDefaultRegion); diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart index e46b8511ae0..d14ad7b75e8 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/text_test.dart @@ -97,7 +97,7 @@ void testMain() { final bool resetValue = ui.debugEmulateFlutterTesterEnvironment; ui.debugEmulateFlutterTesterEnvironment = true; tearDownAll(() => ui.debugEmulateFlutterTesterEnvironment = resetValue); - const List testFonts = ['Ahem', 'FlutterTest']; + const List testFonts = ['FlutterTest', 'Ahem']; test('The default test font is used when a non-test fontFamily is specified', () { final String defaultTestFontFamily = testFonts.first; diff --git a/engine/src/flutter/lib/web_ui/test/html/paragraph/helper.dart b/engine/src/flutter/lib/web_ui/test/html/paragraph/helper.dart index 1497f7b4ab2..23d56fa27cc 100644 --- a/engine/src/flutter/lib/web_ui/test/html/paragraph/helper.dart +++ b/engine/src/flutter/lib/web_ui/test/html/paragraph/helper.dart @@ -37,7 +37,7 @@ const Color yellow = Color(0xFFFFEB3B); const Color lightPurple = Color(0xFFE1BEE7); final EngineParagraphStyle ahemStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, ); diff --git a/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_builder_test.dart b/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_builder_test.dart index 0e10fcb6c6c..0989d248bdb 100644 --- a/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_builder_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text/canvas_paragraph_builder_test.dart @@ -410,9 +410,9 @@ Future testMain() async { }); test('various font sizes', () { - // Paragraphs and spans force the Ahem font in test mode. We need to trick - // them into thinking they are not in test mode, so they use the provided - // font family. + // Paragraphs and spans force the FlutterTest font in test mode. We need to + // trick them into thinking they are not in test mode, so they use the + // provided font family. debugEmulateFlutterTesterEnvironment = false; final EngineParagraphStyle style = EngineParagraphStyle(fontSize: 12.0, fontFamily: 'first'); final CanvasParagraphBuilder builder = CanvasParagraphBuilder(style); @@ -491,7 +491,7 @@ Future testMain() async { }); } -const String defaultFontFamily = 'Ahem'; +const String defaultFontFamily = 'FlutterTest'; const num defaultFontSize = 14; String paragraphStyle() { diff --git a/engine/src/flutter/lib/web_ui/test/text/layout_service_plain_test.dart b/engine/src/flutter/lib/web_ui/test/text/layout_service_plain_test.dart index 567411d993b..96c3e047cec 100644 --- a/engine/src/flutter/lib/web_ui/test/text/layout_service_plain_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text/layout_service_plain_test.dart @@ -388,7 +388,7 @@ Future testMain() async { test('respects text overflow', () { final EngineParagraphStyle overflowStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, ellipsis: '...', ); @@ -458,7 +458,7 @@ Future testMain() async { test('respects max lines', () { final EngineParagraphStyle maxlinesStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, maxLines: 2, ); @@ -506,13 +506,13 @@ Future testMain() async { test('respects text overflow and max lines combined', () { final EngineParagraphStyle onelineStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, maxLines: 1, ellipsis: '...', ); final EngineParagraphStyle multilineStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, maxLines: 2, ellipsis: '...', @@ -591,7 +591,7 @@ Future testMain() async { EngineParagraphStyle createStyle(ui.TextAlign textAlign) { return EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, textAlign: textAlign, textDirection: ui.TextDirection.ltr, @@ -644,7 +644,7 @@ Future testMain() async { EngineParagraphStyle createStyle(ui.TextAlign textAlign) { return EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, textAlign: textAlign, textDirection: ui.TextDirection.rtl, diff --git a/engine/src/flutter/lib/web_ui/test/text/layout_service_rich_test.dart b/engine/src/flutter/lib/web_ui/test/text/layout_service_rich_test.dart index 9843d154bf8..381da5e2281 100644 --- a/engine/src/flutter/lib/web_ui/test/text/layout_service_rich_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text/layout_service_rich_test.dart @@ -143,7 +143,7 @@ Future testMain() async { test('should handle placeholder-only paragraphs', () { final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, textAlign: ui.TextAlign.center, ); @@ -161,7 +161,7 @@ Future testMain() async { test('correct maxIntrinsicWidth when paragraph ends with placeholder', () { final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, textAlign: ui.TextAlign.center, ); @@ -180,7 +180,7 @@ Future testMain() async { test('handles new line followed by a placeholder', () { final EngineParagraphStyle paragraphStyle = EngineParagraphStyle( - fontFamily: 'ahem', + fontFamily: 'Ahem', fontSize: 10, textAlign: ui.TextAlign.center, ); diff --git a/engine/src/flutter/lib/web_ui/test/text_test.dart b/engine/src/flutter/lib/web_ui/test/text_test.dart index 5dc319a2970..adf768bd650 100644 --- a/engine/src/flutter/lib/web_ui/test/text_test.dart +++ b/engine/src/flutter/lib/web_ui/test/text_test.dart @@ -227,15 +227,15 @@ Future testMain() async { expect(paragraph.plainText, 'abcdef'); final List spans = paragraph.toDomElement().querySelectorAll('flt-span').toList(); - expect(spans[0].style.fontFamily, 'Ahem, $fallback, sans-serif'); + expect(spans[0].style.fontFamily, 'FlutterTest, $fallback, sans-serif'); // The nested span here should not set it's family to default sans-serif. - expect(spans[1].style.fontFamily, 'Ahem, $fallback, sans-serif'); + expect(spans[1].style.fontFamily, 'FlutterTest, $fallback, sans-serif'); }, // TODO(mdebbar): https://github.com/flutter/flutter/issues/46638 skip: browserEngine == BrowserEngine.firefox); test('adds Arial and sans-serif as fallback fonts', () { - // Set this to false so it doesn't default to 'Ahem' font. + // Set this to false so it doesn't default to the test font. debugEmulateFlutterTesterEnvironment = false; final CanvasParagraph paragraph = plain(EngineParagraphStyle( @@ -253,7 +253,7 @@ Future testMain() async { skip: browserEngine == BrowserEngine.firefox); test('does not add fallback fonts to generic families', () { - // Set this to false so it doesn't default to 'Ahem' font. + // Set this to false so it doesn't default to the default test font. debugEmulateFlutterTesterEnvironment = false; final CanvasParagraph paragraph = plain(EngineParagraphStyle( @@ -268,7 +268,7 @@ Future testMain() async { }); test('can set font families that need to be quoted', () { - // Set this to false so it doesn't default to 'Ahem' font. + // Set this to false so it doesn't default to the default test font. debugEmulateFlutterTesterEnvironment = false; final CanvasParagraph paragraph = plain(EngineParagraphStyle( @@ -363,7 +363,7 @@ Future testMain() async { final bool resetValue = debugEmulateFlutterTesterEnvironment; debugEmulateFlutterTesterEnvironment = true; tearDownAll(() => debugEmulateFlutterTesterEnvironment = resetValue); - const List testFonts = ['Ahem', 'FlutterTest']; + const List testFonts = ['FlutterTest', 'Ahem']; test('The default test font is used when a non-test fontFamily is specified, or fontFamily is not specified', () { final String defaultTestFontFamily = testFonts.first;