Escape entities just before Pango rendering (#2382).

Previously, text coming out of the player would have things like
& escaped to &.  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.

diff --git a/src/lib/render_text.cc b/src/lib/render_text.cc
index d3db4ac1940c245987d16c4f055e632d82f2cec7..2338bde97abe449adf0170bcccb79014181f3565 100644 (file)
--- 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 4b521ede75449ff2e2cb7df792e8190f2cf9b806..750deb9b30602b952e88876ca27c20f1683d1178 100644 (file)
--- 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, "&", "&");
-       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 20fbe5f1a243b7ad034d1013e14b844ed8675603..fe95cbbfc13da7c4b079f6ef27619851c3953d4b 100644 (file)
--- 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>");
                }
        }
 }