From 99cb0937e54340fa20c594aaa501276b8321cbf0 Mon Sep 17 00:00:00 2001
From: Carl Hetherington <cth@carlh.net>
Date: Wed, 30 Nov 2022 22:08:00 +0100
Subject: Escape entities just before Pango rendering (#2382).

Previously, text coming out of the player would have things like
& escaped to &amp;.  This escaping is also done by libxml++ when
writing XML, so doing it in the player would mean it was done
twice.

We do, however, need to escape things before passing them to Pango
as otherwise it gives errors and renders nothing for the line.

Here we move the escaping to just before the rendering, meaning
that in the reset of DoM we should pass unescaped strings around.
---
 src/lib/render_text.cc    |  5 +++++
 src/lib/text_decoder.cc   | 18 ++----------------
 test/dcp_subtitle_test.cc |  3 ++-
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc
index d3db4ac19..2338bde97 100644
--- a/src/lib/render_text.cc
+++ b/src/lib/render_text.cc
@@ -103,6 +103,11 @@ marked_up (list<StringText> subtitles, int target_height, float fade_factor, str
 			span += " " + extra_attribute;
 		}
 		span += ">";
+
+		boost::algorithm::replace_all(text, "&", "&amp;");
+		boost::algorithm::replace_all(text, "<", "&lt;");
+		boost::algorithm::replace_all(text, ">", "&gt;");
+
 		span += text;
 		span += "</span>";
 		return span;
diff --git a/src/lib/text_decoder.cc b/src/lib/text_decoder.cc
index 4b521ede7..750deb9b3 100644
--- a/src/lib/text_decoder.cc
+++ b/src/lib/text_decoder.cc
@@ -65,20 +65,6 @@ TextDecoder::emit_bitmap_start (ContentBitmapText const& bitmap)
 }
 
 
-static
-string
-escape_text (string text)
-{
-	/* We must escape some things, otherwise they might confuse our subtitle
-	   renderer (which uses entities and some HTML-esque markup to do bold/italic etc.)
-	*/
-	boost::algorithm::replace_all(text, "&", "&amp;");
-	boost::algorithm::replace_all(text, "<", "&lt;");
-	boost::algorithm::replace_all(text, ">", "&gt;");
-	return text;
-}
-
-
 static
 void
 set_forced_appearance(shared_ptr<const TextContent> content, StringText& subtitle)
@@ -113,7 +99,7 @@ TextDecoder::emit_plain_start (ContentTime from, vector<dcp::SubtitleString> sub
 			content()->get_font(subtitle.font().get_value_or("")),
 			valign_standard
 			);
-		string_text.set_text(escape_text(string_text.text()));
+		string_text.set_text(string_text.text());
 		set_forced_appearance(content(), string_text);
 		string_texts.push_back(string_text);
 	}
@@ -257,7 +243,7 @@ TextDecoder::emit_plain_start (ContentTime from, sub::Subtitle const & sub_subti
 				v_align,
 				0,
 				dcp::Direction::LTR,
-				escape_text(block.text),
+				block.text,
 				dcp::Effect::NONE,
 				block.effect_colour.get_value_or(sub::Colour(0, 0, 0)).dcp(),
 				/* Hack: we should use subtitle.fade_up and subtitle.fade_down here
diff --git a/test/dcp_subtitle_test.cc b/test/dcp_subtitle_test.cc
index 20fbe5f1a..fe95cbbfc 100644
--- a/test/dcp_subtitle_test.cc
+++ b/test/dcp_subtitle_test.cc
@@ -140,7 +140,8 @@ BOOST_AUTO_TEST_CASE (dcp_subtitle_test2)
 	stored = optional<ContentStringText> ();
 	while (!decoder->pass()) {
 		if (stored && stored->from() == ContentTime(0)) {
-			BOOST_CHECK_EQUAL (stored->subs.front().text(), "&lt;b&gt;Hello world!&lt;/b&gt;");
+			/* Text passed around by the player should be unescaped */
+			BOOST_CHECK_EQUAL(stored->subs.front().text(), "<b>Hello world!</b>");
 		}
 	}
 }
-- 
cgit v1.2.3