From e9df8fb10c3752cb833e89c8ac793241497e29b6 Mon Sep 17 00:00:00 2001
From: Theodore Abshire <theodab@google.com>
Date: Mon, 24 Jan 2022 23:59:33 -0800
Subject: [PATCH] fix(text): Inherit alignment from regions.

The recent changes to TTML parsing, to not inherit regions,
inadvertently ended up breaking text alignment in situations
where a region with alignment was on the p or div above a span.
Previously, we only inherited the text and display alignment
from a region on leaf nodes... which was a problem, since we
also didn't apply any styles to text nodes.

Change-Id: I62ac155bc4310a5f7da52c10ca2dd434f8015c97
---
 lib/text/ttml_text_parser.js       |  8 +++---
 test/text/ttml_text_parser_unit.js | 41 +++++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/lib/text/ttml_text_parser.js b/lib/text/ttml_text_parser.js
index 5f8b50de1c..fc8cbfddfb 100644
--- a/lib/text/ttml_text_parser.js
+++ b/lib/text/ttml_text_parser.js
@@ -379,7 +379,6 @@ shaka.text.TtmlTextParser = class {
               parentCueElement, 'region', regionElements, /* prefix= */ '')[0];
     }
 
-
     shaka.text.TtmlTextParser.addStyle_(
         cue,
         cueElement,
@@ -527,14 +526,13 @@ shaka.text.TtmlTextParser = class {
     }
 
     const align = TtmlTextParser.getStyleAttribute_(
-        cueElement, region, styles, 'textAlign', shouldInheritRegionStyles);
+        cueElement, region, styles, 'textAlign', true);
     if (align) {
       cue.positionAlign = TtmlTextParser.textAlignToPositionAlign_[align];
       cue.lineAlign = TtmlTextParser.textAlignToLineAlign_[align];
 
       goog.asserts.assert(align.toUpperCase() in Cue.textAlign,
-          align.toUpperCase() +
-                          ' Should be in Cue.textAlign values!');
+          align.toUpperCase() + ' Should be in Cue.textAlign values!');
 
       cue.textAlign = Cue.textAlign[align.toUpperCase()];
     } else {
@@ -543,7 +541,7 @@ shaka.text.TtmlTextParser = class {
     }
 
     const displayAlign = TtmlTextParser.getStyleAttribute_(
-        cueElement, region, styles, 'displayAlign', shouldInheritRegionStyles);
+        cueElement, region, styles, 'displayAlign', true);
     if (displayAlign) {
       goog.asserts.assert(displayAlign.toUpperCase() in Cue.displayAlign,
           displayAlign.toUpperCase() +
diff --git a/test/text/ttml_text_parser_unit.js b/test/text/ttml_text_parser_unit.js
index 5355ffd104..da1c70bf42 100644
--- a/test/text/ttml_text_parser_unit.js
+++ b/test/text/ttml_text_parser_unit.js
@@ -1827,8 +1827,8 @@ describe('TtmlTextParser', () => {
             // Styles from regionStyle should apply only to the nested cue.
             backgroundColor: '',
             color: '',
-            displayAlign: Cue.displayAlign.AFTER, // displayAlign default value.
-            textAlign: Cue.textAlign.START, // textAlign default value.
+            displayAlign: Cue.displayAlign.CENTER,
+            textAlign: Cue.textAlign.CENTER,
 
             nestedCues: [
               {
@@ -1853,7 +1853,7 @@ describe('TtmlTextParser', () => {
                 // Styles inherited from backgroundStyle via regionStyle via
                 // spanStyle
                 displayAlign: Cue.displayAlign.CENTER,
-                textAlign: Cue.textAlign.CENTER,
+                textAlign: Cue.textAlign.END,
               },
             ],
           },
@@ -1867,7 +1867,7 @@ describe('TtmlTextParser', () => {
         // spanStyle inherits attributes from regionStyle
         '    <style xml:id="pStyle" tts:fontSize="15px" />' +
         '    <style xml:id="spanStyle" style="regionStyle" ' +
-        '           tts:backgroundColor="white" />' +
+        '           tts:backgroundColor="white" tts:textAlign="end" />' +
         // regionStyle inherits attributes from backgroundStyle
         '    <style xml:id="regionStyle" style="backgroundStyle" ' +
         '           tts:backgroundColor="transparent" tts:color="blue" />' +
@@ -1886,6 +1886,39 @@ describe('TtmlTextParser', () => {
         {startTime: 0, endTime: 60});
   });
 
+  it('inherits alignment from parent regions', () => {
+    verifyHelper(
+        [
+          {
+            startTime: 0,
+            endTime: 60,
+            payload: '',
+            fontSize: '',
+            textAlign: Cue.textAlign.END,
+            displayAlign: Cue.displayAlign.CENTER,
+            nestedCues: [
+              {
+                startTime: 0,
+                endTime: 60,
+                payload: 'Hello!',
+                textAlign: Cue.textAlign.END,
+                displayAlign: Cue.displayAlign.CENTER,
+              },
+            ],
+          },
+        ],
+        '<tt xmlns:tts="http://www.w3.org/ns/ttml#styling">' +
+        '<head><layout>' +
+        '<region xml:id="r1" tts:textAlign="end" tts:displayAlign="center" />' +
+        '</layout></head>' +
+        '<body><div><p begin="00:00" end="01:00" region="r1">' +
+        '<span>Hello!</span>' +
+        '</p></div></body></tt>',
+        {periodStart: 0, segmentStart: 0, segmentEnd: 0},
+        {startTime: 0, endTime: 60},
+    );
+  });
+
   // Regression test for https://github.com/google/shaka-player/issues/3743
   it('inherits styles from other styles on nestedCues', () => {
     verifyHelper(