From 54eccd3893af0a25390c7af236466ee337d96709 Mon Sep 17 00:00:00 2001
From: ibaker <ibaker@google.com>
Date: Wed, 24 Jun 2020 11:42:59 +0100
Subject: [PATCH] Change WebViewSubtitleOutput to use em not % for line offsets

The existing code moves a multi-line cue box by multiples of the height
of the whole cue box (incorrect), rather than multiples of the first
line of text (correct). These two are equivalent for single-line cues,
which is why I didn't initially spot the problem.

PiperOrigin-RevId: 318036793
---
 .../exoplayer2/ui/WebViewSubtitleOutput.java  | 44 ++++++++++---------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java
index 70ab6bc9c9..835a4df43a 100644
--- a/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java
+++ b/library/ui/src/main/java/com/google/android/exoplayer2/ui/WebViewSubtitleOutput.java
@@ -45,6 +45,12 @@ import java.util.List;
  */
 /* package */ final class WebViewSubtitleOutput extends FrameLayout implements SubtitleView.Output {
 
+  /**
+   * A hard-coded value for the line-height attribute, so we can use it to move text up and down by
+   * one line-height. Most browsers default 'normal' (CSS default) to 1.2 for most font families.
+   */
+  private static final float CSS_LINE_HEIGHT = 1.2f;
+
   /**
    * A {@link CanvasSubtitleOutput} used for displaying bitmap cues.
    *
@@ -165,10 +171,12 @@ import java.util.List;
                 + "right:0;"
                 + "color:%s;"
                 + "font-size:%s;"
+                + "line-height:%.2fem;"
                 + "text-shadow:%s;"
                 + "'>",
             HtmlUtils.toCssRgba(style.foregroundColor),
             convertTextSizeToCss(defaultTextSizeType, defaultTextSize),
+            CSS_LINE_HEIGHT,
             convertCaptionStyleToCssTextShadow(style)));
 
     String backgroundColorCss = HtmlUtils.toCssRgba(style.backgroundColor);
@@ -178,35 +186,31 @@ import java.util.List;
       float positionPercent = (cue.position != Cue.DIMEN_UNSET) ? (cue.position * 100) : 50;
       int positionAnchorTranslatePercent = anchorTypeToTranslatePercent(cue.positionAnchor);
 
-      float linePercent;
-      int lineTranslatePercent;
-      int lineAnchorTranslatePercent;
+      String lineValue;
+      boolean lineMeasuredFromEnd = false;
+      int lineAnchorTranslatePercent = 0;
       if (cue.line != Cue.DIMEN_UNSET) {
         switch (cue.lineType) {
           case Cue.LINE_TYPE_NUMBER:
             if (cue.line >= 0) {
-              linePercent = 0;
-              lineTranslatePercent = Math.round(cue.line) * 100;
-              lineAnchorTranslatePercent = 0;
+              lineValue = Util.formatInvariant("%.2fem", cue.line * CSS_LINE_HEIGHT);
             } else {
-              linePercent = 100;
-              lineTranslatePercent = Math.round(cue.line + 1) * 100;
-              lineAnchorTranslatePercent = -100;
+              lineValue = Util.formatInvariant("%.2fem", (-cue.line - 1) * CSS_LINE_HEIGHT);
+              lineMeasuredFromEnd = true;
             }
             break;
           case Cue.LINE_TYPE_FRACTION:
           case Cue.TYPE_UNSET:
           default:
-            linePercent = cue.line * 100;
-            lineTranslatePercent = 0;
+            lineValue = Util.formatInvariant("%.2f%%", cue.line * 100);
+
             lineAnchorTranslatePercent =
                 cue.verticalType == Cue.VERTICAL_TYPE_RL
                     ? -anchorTypeToTranslatePercent(cue.lineAnchor)
                     : anchorTypeToTranslatePercent(cue.lineAnchor);
         }
       } else {
-        linePercent = (1.0f - bottomPaddingFraction) * 100;
-        lineTranslatePercent = 0;
+        lineValue = Util.formatInvariant("%.2f%%", (1.0f - bottomPaddingFraction) * 100);
         lineAnchorTranslatePercent = -100;
       }
 
@@ -225,16 +229,16 @@ import java.util.List;
       String lineProperty;
       switch (cue.verticalType) {
         case Cue.VERTICAL_TYPE_LR:
-          lineProperty = "left";
+          lineProperty = lineMeasuredFromEnd ? "right" : "left";
           positionProperty = "top";
           break;
         case Cue.VERTICAL_TYPE_RL:
-          lineProperty = "right";
+          lineProperty = lineMeasuredFromEnd ? "left" : "right";
           positionProperty = "top";
           break;
         case Cue.TYPE_UNSET:
         default:
-          lineProperty = "top";
+          lineProperty = lineMeasuredFromEnd ? "bottom" : "top";
           positionProperty = "left";
       }
 
@@ -243,12 +247,12 @@ import java.util.List;
       int verticalTranslatePercent;
       if (cue.verticalType == Cue.VERTICAL_TYPE_LR || cue.verticalType == Cue.VERTICAL_TYPE_RL) {
         sizeProperty = "height";
-        horizontalTranslatePercent = lineTranslatePercent + lineAnchorTranslatePercent;
+        horizontalTranslatePercent = lineAnchorTranslatePercent;
         verticalTranslatePercent = positionAnchorTranslatePercent;
       } else {
         sizeProperty = "width";
         horizontalTranslatePercent = positionAnchorTranslatePercent;
-        verticalTranslatePercent = lineTranslatePercent + lineAnchorTranslatePercent;
+        verticalTranslatePercent = lineAnchorTranslatePercent;
       }
 
       html.append(
@@ -256,7 +260,7 @@ import java.util.List;
                   "<div style='"
                       + "position:absolute;"
                       + "%s:%.2f%%;"
-                      + "%s:%.2f%%;"
+                      + "%s:%s;"
                       + "%s:%s;"
                       + "text-align:%s;"
                       + "writing-mode:%s;"
@@ -267,7 +271,7 @@ import java.util.List;
                   positionProperty,
                   positionPercent,
                   lineProperty,
-                  linePercent,
+                  lineValue,
                   sizeProperty,
                   size,
                   textAlign,