From add5ddd86cf8049e02b25ad5de73673974ee8147 Mon Sep 17 00:00:00 2001
From: Mouad Debbar <mouad.debbar@gmail.com>
Date: Thu, 14 Nov 2019 00:12:56 -0800
Subject: [PATCH] [web] Fix selectable text rendering (flutter/engine#13802)

---
 .../web_ui/lib/src/engine/text/paragraph.dart |  6 ++--
 .../test/golden_tests/engine/scuba.dart       | 31 ++++++++++++++-----
 .../engine/text_style_golden_test.dart        | 20 ++++++++++++
 3 files changed, 48 insertions(+), 9 deletions(-)

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 6117d2a5f6a..17b1041a5e2 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
@@ -362,7 +362,9 @@ class EngineParagraphStyle implements ui.ParagraphStyle {
   }
 
   double get _lineHeight {
-    if (_strutStyle == null || _strutStyle._height == null) {
+    // TODO(mdebbar): Implement proper support for strut styles.
+    // https://github.com/flutter/flutter/issues/32243
+    if (_strutStyle == null || _strutStyle._height == null || _strutStyle._height == 0) {
       // When there's no strut height, always use paragraph style height.
       return _height;
     }
@@ -1093,7 +1095,7 @@ void _applyParagraphStyleToElement({
       cssStyle.textAlign = textAlignToCssValue(
           style._textAlign, style._textDirection ?? ui.TextDirection.ltr);
     }
-    if (style._lineHeight != style._lineHeight) {
+    if (style._lineHeight != previousStyle._lineHeight) {
       cssStyle.lineHeight = '${style._lineHeight}';
     }
     if (style._textDirection != previousStyle._textDirection) {
diff --git a/engine/src/flutter/lib/web_ui/test/golden_tests/engine/scuba.dart b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/scuba.dart
index 18dff26fd7a..a43afdbb59d 100644
--- a/engine/src/flutter/lib/web_ui/test/golden_tests/engine/scuba.dart
+++ b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/scuba.dart
@@ -39,18 +39,31 @@ class EngineScubaTester {
     return EngineScubaTester(viewportSize);
   }
 
-  Future<void> diffScreenshot(String fileName, {double maxDiffRate}) async {
-    await matchGoldenFile('$fileName.png',
-        region: ui.Rect.fromLTWH(0, 0, viewportSize.width, viewportSize.height),
-        maxDiffRate: maxDiffRate);
+  ui.Rect get viewportRegion =>
+      ui.Rect.fromLTWH(0, 0, viewportSize.width, viewportSize.height);
+
+  Future<void> diffScreenshot(
+    String fileName, {
+    ui.Rect region,
+    double maxDiffRate,
+  }) async {
+    await matchGoldenFile(
+      '$fileName.png',
+      region: region ?? viewportRegion,
+      maxDiffRate: maxDiffRate,
+    );
   }
 
   /// Prepares the DOM and inserts all the necessary nodes, then invokes scuba's
   /// screenshot diffing.
   ///
   /// It also cleans up the DOM after itself.
-  Future<void> diffCanvasScreenshot(EngineCanvas canvas, String fileName,
-      {double maxDiffRate}) async {
+  Future<void> diffCanvasScreenshot(
+    EngineCanvas canvas,
+    String fileName, {
+    ui.Rect region,
+    double maxDiffRate,
+  }) async {
     // Wrap in <flt-scene> so that our CSS selectors kick in.
     final html.Element sceneElement = html.Element.tag('flt-scene');
     try {
@@ -60,7 +73,11 @@ class EngineScubaTester {
       if (TextMeasurementService.enableExperimentalCanvasImplementation) {
         screenshotName += '+canvas_measurement';
       }
-      await diffScreenshot(screenshotName, maxDiffRate: maxDiffRate);
+      await diffScreenshot(
+        screenshotName,
+        region: region,
+        maxDiffRate: maxDiffRate,
+      );
     } finally {
       // The page is reused across tests, so remove the element after taking the
       // Scuba screenshot.
diff --git a/engine/src/flutter/lib/web_ui/test/golden_tests/engine/text_style_golden_test.dart b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/text_style_golden_test.dart
index 04742ad3f92..3017dade360 100644
--- a/engine/src/flutter/lib/web_ui/test/golden_tests/engine/text_style_golden_test.dart
+++ b/engine/src/flutter/lib/web_ui/test/golden_tests/engine/text_style_golden_test.dart
@@ -218,4 +218,24 @@ void main() async {
     drawTextWithShadow(canvas);
     return scuba.diffCanvasScreenshot(canvas, 'text_shadow', maxDiffRate: 0.2);
   }, bSkipHoudini: true);
+
+  testEachCanvas('Handles disabled strut style', (EngineCanvas canvas) {
+    // Flutter uses [StrutStyle.disabled] for the [SelectableText] widget. This
+    // translates into a strut style with a [height] of 0, which wasn't being
+    // handled correctly by the web engine.
+    final StrutStyle disabled = StrutStyle(height: 0, leading: 0);
+    canvas.drawParagraph(
+      paragraph(
+        'Hello\nWorld',
+        paragraphStyle: ParagraphStyle(strutStyle: disabled),
+      ),
+      Offset.zero,
+    );
+    return scuba.diffCanvasScreenshot(
+      canvas,
+      'text_strut_style_disabled',
+      region: Rect.fromLTRB(0, 0, 100, 100),
+      maxDiffRate: 0.9 / 100, // 0.9%
+    );
+  });
 }