From cd297aec21ffedce7940ebd58c7512dc2c636038 Mon Sep 17 00:00:00 2001
From: Mouad Debbar <mdebbar@google.com>
Date: Tue, 29 Mar 2022 10:55:03 -0400
Subject: [PATCH] [web] Correct text baseline when rendering to DOM
 (flutter/engine#32298)

---
 .../lib/src/engine/text/canvas_paragraph.dart |  4 +-
 .../web_ui/lib/src/engine/text/paragraph.dart |  3 --
 .../text/canvas_paragraph_builder_test.dart   | 37 +++++++++++--------
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart
index b291549ecb3..f502f6cf8a6 100644
--- a/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart
+++ b/engine/src/flutter/lib/web_ui/lib/src/engine/text/canvas_paragraph.dart
@@ -258,7 +258,9 @@ void _positionSpanElement(html.Element element, EngineLineMetrics line, RangeBox
     ..top = '${boxRect.top}px'
     ..left = '${boxRect.left}px'
     // This is needed for space-only spans that are used to justify the paragraph.
-    ..width = '${boxRect.width}px';
+    ..width = '${boxRect.width}px'
+    // Makes sure the baseline of each span is positioned as expected.
+    ..lineHeight = '${boxRect.height}px';
 }
 
 /// A common interface for all types of spans that make up a paragraph.
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 0c34403a48c..9daef10cbd1 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
@@ -699,9 +699,6 @@ void applyTextStyleToElement({
   if (background != null) {
     cssStyle.backgroundColor = colorToCssString(background);
   }
-  if (style.height != null) {
-    cssStyle.lineHeight = '${style.height}';
-  }
   final double? fontSize = style.fontSize;
   if (fontSize != null) {
     cssStyle.fontSize = '${fontSize.floor()}px';
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 82536da0434..2154663f1bc 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
@@ -92,13 +92,14 @@ Future<void> testMain() async {
     expect(paragraph.spans, hasLength(1));
 
     paragraph.layout(const ParagraphConstraints(width: double.infinity));
-    expect(
-      paragraph.toDomElement().outerHtml,
+    expectOuterHtml(
+      paragraph,
       '<flt-paragraph style="${paragraphStyle()}">'
       '<flt-span style="${spanStyle(top: 0, left: 0, width: 14*5)}">'
       'Hello'
       '</flt-span>'
       '</flt-paragraph>',
+      ignorePositions: !isBlink,
     );
 
     final FlatTextSpan textSpan = paragraph.spans.single as FlatTextSpan;
@@ -116,13 +117,14 @@ Future<void> testMain() async {
     expect(paragraph.toPlainText(), 'Hello');
 
     paragraph.layout(const ParagraphConstraints(width: double.infinity));
-    expect(
-      paragraph.toDomElement().outerHtml,
+    expectOuterHtml(
+      paragraph,
       '<flt-paragraph style="${paragraphStyle()}">'
       '<flt-span style="${spanStyle(top: 0, left: 0, width: 14*5)}">'
       'Hello'
       '</flt-span>'
       '</flt-paragraph>',
+      ignorePositions: !isBlink,
     );
   });
 
@@ -137,13 +139,14 @@ Future<void> testMain() async {
     expect(paragraph.toPlainText(), 'HelloWorld');
 
     paragraph.layout(const ParagraphConstraints(width: 100.0));
-    expect(
-      paragraph.toDomElement().outerHtml,
+    expectOuterHtml(
+      paragraph,
       '<flt-paragraph style="${paragraphStyle()}">'
       '<flt-span style="${spanStyle(top: 0, left: 0, width: 14*4)}">'
       'Hell...'
       '</flt-span>'
       '</flt-paragraph>',
+      ignorePositions: !isBlink,
     );
   });
 
@@ -168,7 +171,7 @@ Future<void> testMain() async {
     expectOuterHtml(
       paragraph,
       '<flt-paragraph style="${paragraphStyle()}">'
-      '<flt-span style="${spanStyle(top: 0, left: 0, width: (9+2)*5, lineHeight: 1.5, fontSize: 9, fontWeight: 'bold', fontStyle: 'italic', letterSpacing: 2)}">'
+      '<flt-span style="${spanStyle(top: 0, left: 0, width: (9+2)*5, lineHeight: 1.5*9, fontSize: 9, fontWeight: 'bold', fontStyle: 'italic', letterSpacing: 2)}">'
       'Hello'
       '</flt-span>'
       '</flt-paragraph>',
@@ -280,7 +283,7 @@ Future<void> testMain() async {
     expectOuterHtml(
       paragraph,
       '<flt-paragraph style="${paragraphStyle()}">'
-      '<flt-span style="${spanStyle(top: 0, left: 0, width: 13*5, lineHeight: 2, fontSize: 13, fontWeight: 'bold')}">'
+      '<flt-span style="${spanStyle(top: 0, left: 0, width: 13*5, lineHeight: 2*13, fontSize: 13, fontWeight: 'bold')}">'
       'Hello'
       '</flt-span>'
       '<flt-span style="${spanStyle(top: 6, left: 65, width: 13*1, fontSize: 13, fontWeight: 'bold', fontStyle: 'italic')}">'
@@ -458,7 +461,6 @@ String spanStyle({
 }) {
   return <String>[
     'color: rgb(255, 0, 0);',
-    if (lineHeight != null) 'line-height: $lineHeight;',
     'font-size: ${fontSize}px;',
     if (fontWeight != null) 'font-weight: $fontWeight;',
     if (fontStyle != null) 'font-style: $fontStyle;',
@@ -468,6 +470,7 @@ String spanStyle({
     if (top != null) 'top: ${top}px;',
     if (left != null) 'left: ${left}px;',
     if (width != null) 'width: ${width}px;',
+    'line-height: ${lineHeight ?? fontSize}px;',
   ].join(' ');
 }
 
@@ -494,19 +497,23 @@ TextStyle styleWithDefaults({
 void expectOuterHtml(CanvasParagraph paragraph, String expected, {required bool ignorePositions}) {
   String outerHtml = paragraph.toDomElement().outerHtml!;
   if (ignorePositions) {
-    outerHtml = removePositionInfo(outerHtml);
-    expected = removePositionInfo(expected);
+    outerHtml = removeMeasurementInfo(outerHtml);
+    expected = removeMeasurementInfo(expected);
   }
 
   expect(outerHtml, expected);
 }
 
-/// Removes "top" and "left" CSS styles from the given html string.
+/// Removes CSS styles that are based on text measurement from the given html
+/// string.
 ///
-/// This is needed when the positioning information in the html output is
-/// unknown and could be different depending on browser and environment.
-String removePositionInfo(String outerHtml) {
+/// Examples: top, left, line-height, width.
+///
+/// This is needed when the measurement is unknown or could be different
+/// depending on browser and environment.
+String removeMeasurementInfo(String outerHtml) {
   return outerHtml
+      .replaceAll(RegExp(r'\s*line-height:\s*[\d\.]+px\s*;\s*'), '')
       .replaceAll(RegExp(r'\s*width:\s*[\d\.]+px\s*;\s*'), '')
       .replaceAll(RegExp(r'\s*top:\s*[\d\.]+px\s*;\s*'), '')
       .replaceAll(RegExp(r'\s*left:\s*[\d\.]+px\s*;\s*'), '');