Don't insert incorrect <Effect> nodes into metadata (#2581).

Previously we would add assume Effect=none (i.e. force all subtitles
to have no effect) if neither of the legacy tags Border or Shadow
were present in the metadata.  In this case we should just leave
Effect as unset.

diff --git a/src/lib/text_content.cc b/src/lib/text_content.cc
index e4cbc601a0774716354538f354213fdfe354f93f..92a35b8220cbca3f41c7302d9d8f0536a0b22ab9 100644 (file)
--- a/src/lib/text_content.cc
+++ b/src/lib/text_content.cc
@@ -148,8 +148,6 @@ TextContent::TextContent (Content* parent, cxml::ConstNodePtr node, int version,
                _effect = dcp::Effect::BORDER;
        } else if (node->optional_bool_child("Shadow").get_value_or(false)) {
                _effect = dcp::Effect::SHADOW;
-       } else {
-               _effect = dcp::Effect::NONE;
        }
 
        auto effect = node->optional_string_child("Effect");

diff --git a/test/film_metadata_test.cc b/test/film_metadata_test.cc
index 9b855de5b5c355e56918a904424b39834d5798d6..ada943c804bf0beb0ae461cb2440d6194f9c4e6c 100644 (file)
--- a/test/film_metadata_test.cc
+++ b/test/film_metadata_test.cc
@@ -210,3 +210,24 @@ BOOST_AUTO_TEST_CASE (metadata_video_range_guessed_for_png)
        BOOST_REQUIRE(film->content()[0]->video);
        BOOST_CHECK(film->content()[0]->video->range() == VideoRange::FULL);
 }
+
+
+/* Bug #2581 */
+BOOST_AUTO_TEST_CASE(effect_node_not_inserted_incorrectly)
+{
+       auto sub = content_factory("test/data/15s.srt");
+       auto film = new_test_film2("effect_node_not_inserted_incorrectly", sub);
+       film->write_metadata();
+
+       namespace fs = boost::filesystem;
+       auto film2 = make_shared<Film>(fs::path("build/test/effect_node_not_inserted_incorrectly"));
+       film2->read_metadata();
+       film2->write_metadata();
+
+       cxml::Document doc("Metadata");
+       doc.read_file("build/test/effect_node_not_inserted_incorrectly/metadata.xml");
+
+       /* There should be no <Effect> node in the text, since we don't want to force the effect to "none" */
+       BOOST_CHECK(!doc.node_child("Playlist")->node_child("Content")->node_child("Text")->optional_node_child("Effect"));
+}
+