From c0e8cef73162f76ce0d7e216aa4c45d58356cfe7 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Fri, 4 Feb 2022 11:39:44 -0700
Subject: [PATCH] Fix bug #986 to better handle a both a correctly behaving
 lxml, as well as a badly behaving lxml

---
 openlp/plugins/songs/lib/openlyricsxml.py     |  3 +-
 .../songs/test_openlyricsxml.py               | 39 +++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/openlp/plugins/songs/lib/openlyricsxml.py b/openlp/plugins/songs/lib/openlyricsxml.py
index d3388b236..b31e2ad3a 100644
--- a/openlp/plugins/songs/lib/openlyricsxml.py
+++ b/openlp/plugins/songs/lib/openlyricsxml.py
@@ -683,8 +683,7 @@ class OpenLyrics(object):
         string_lines = etree.tostring(lines)
         # lxml 4.6.x or some recent version of libxml2 seems to add the rest of the song to the string, so split on the
         # closing verse tag, and then add it back in
-        string_lines, _ = string_lines.split(b'</lines>', 1)
-        string_lines += b'</lines>'
+        string_lines = string_lines.split(b'</lines>', 1)[0] + b'</lines>'
         element = etree.XML(string_lines)
 
         # OpenLyrics 0.8 uses <br/> for new lines. Append text from "lines" element to verse text.
diff --git a/tests/openlp_plugins/songs/test_openlyricsxml.py b/tests/openlp_plugins/songs/test_openlyricsxml.py
index 0efab5226..ee12950c1 100644
--- a/tests/openlp_plugins/songs/test_openlyricsxml.py
+++ b/tests/openlp_plugins/songs/test_openlyricsxml.py
@@ -37,6 +37,14 @@ RESULT_TAGS = [{"temporary": False, "protected": False, "desc": "z", "start tag"
                {"temporary": False, "end tag": "{/c}", "desc": "c", "start tag": "{c}",
                 "start html": "<span class=\"chord\" style=\"display:none\"><strong>", "end html": "</strong></span>",
                 "protected": False}]
+VERSE_LINES_07_XML = '<lines>\
+                        <line>Amazing grace, how sweet the sound</line>\
+                        <line>That saved a wretch like me</line>\
+                      </lines>'
+VERSE_LINES_08_XML = '<lines>\
+                        Amazing grace, how sweet the sound<br/>\
+                        That saved a wretch like me\
+                      </lines>'
 AUTHOR_XML = '<properties>\
                   <authors>\
                       <author type="words">Test Author1</author>\
@@ -65,6 +73,37 @@ def test_songxml_get_verses_invalid_xml():
     assert result == []
 
 
+def test_process_verse_lines_v07():
+    """
+    Test that the _process_verse_lines method correctly processes the verse lines with v0.7 OpenLyrics
+    """
+    # GIVEN: Some lyrics XML and version 0.7 of OpenLyrics
+    open_lyrics = OpenLyrics(MagicMock())
+    lines = objectify.fromstring(VERSE_LINES_07_XML)
+
+    # WHEN: The lyrics of a verse are processed
+    result = open_lyrics._process_verse_lines(lines, '0.7')
+
+    # THEN: The results should be correct
+    assert result == 'Amazing grace, how sweet the sound\nThat saved a wretch like me'
+
+
+def test_process_verse_lines_v08():
+    """
+    Test that the _process_verse_lines method correctly processes the verse lines with v0.8 OpenLyrics
+    """
+    # GIVEN: Some lyrics XML and version 0.8 of OpenLyrics
+    open_lyrics = OpenLyrics(MagicMock())
+    lines = objectify.fromstring(VERSE_LINES_08_XML)
+
+    # WHEN: The lyrics of a verse are processed
+    result = open_lyrics._process_verse_lines(lines, '0.8')
+
+    # THEN: The results should be correct
+    assert result == '                        Amazing grace, how sweet the sound'\
+                     '                        That saved a wretch like me                      '
+
+
 def test_process_formatting_tags(settings):
     """
     Test that _process_formatting_tags works