Mark a custom slide edited from preview as coming from plugin. Fixes bug 1439671.

Use html.escape instead of the deprecated cgi.escape
Fix support for special characters in theme names. Fixes bug 1438563.
Fix another case of traceback when playing media with no players available/enabled (bug 1422761).

bzr-revno: 2526
Fixes: https://launchpad.net/bugs/1439671, https://launchpad.net/bugs/1438563, https://launchpad.net/bugs/1422761

openlp/core/common/__init__.py

@@ -66,8 +66,9 @@ def check_directory_exists(directory, do_not_log=False):
     try:
         if not os.path.exists(directory):
             os.makedirs(directory)
-    except IOError:
-        pass
+    except IOError as e:
+        if not do_not_log:
+            log.exception('failed to check if directory exists or create directory')
 
 
 def get_frozen_path(frozen_option, non_frozen_option):

openlp/core/lib/__init__.py

@@ -90,7 +90,7 @@ def get_text_file_string(text_file):
     file_handle = None
     content = None
     try:
-        file_handle = open(text_file, 'r')
+        file_handle = open(text_file, 'r', encoding='utf-8')
         if not file_handle.read(3) == '\xEF\xBB\xBF':
             # no BOM was found
             file_handle.seek(0)

openlp/core/ui/maindisplay.py

@@ -29,7 +29,7 @@ Some of the code for this form is based on the examples at:
 
 """
 
-import cgi
+import html
 import logging
 
 from PyQt4 import QtCore, QtGui, QtWebKit, QtOpenGL
@@ -273,7 +273,7 @@ class MainDisplay(OpenLPMixin, Display, RegistryProperties):
         """
         # First we convert <>& marks to html variants, then apply
         # formattingtags, finally we double all backslashes for JavaScript.
-        text_prepared = expand_tags(cgi.escape(text)).replace('\\', '\\\\').replace('\"', '\\\"')
+        text_prepared = expand_tags(html.escape(text)).replace('\\', '\\\\').replace('\"', '\\\"')
         if self.height() != self.screen['size'].height() or not self.isVisible():
             shrink = True
             js = 'show_alert("%s", "%s")' % (text_prepared, 'top')

openlp/core/ui/media/mediacontroller.py

@@ -523,6 +523,8 @@ class MediaController(RegistryMixin, OpenLPMixin, RegistryProperties):
         if controller.media_info.file_info.isFile():
             suffix = '*.%s' % controller.media_info.file_info.suffix().lower()
             for title in used_players:
+                if not title:
+                    continue
                 player = self.media_players[title]
                 if suffix in player.video_extensions_list:
                     if not controller.media_info.is_background or controller.media_info.is_background and \

openlp/core/ui/thememanager.py

@@ -354,8 +354,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
         delete_file(os.path.join(self.path, thumb))
         delete_file(os.path.join(self.thumb_path, thumb))
         try:
-            encoding = get_filesystem_encoding()
-            shutil.rmtree(os.path.join(self.path, theme).encode(encoding))
+            # Windows is always unicode, so no need to encode filenames
+            if is_win():
+                shutil.rmtree(os.path.join(self.path, theme))
+            else:
+                encoding = get_filesystem_encoding()
+                shutil.rmtree(os.path.join(self.path, theme).encode(encoding))
         except OSError as os_error:
             shutil.Error = os_error
             self.log_exception('Error deleting theme %s' % theme)
@@ -567,7 +571,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
                 check_directory_exists(os.path.dirname(full_name))
                 if os.path.splitext(name)[1].lower() == '.xml':
                     file_xml = str(theme_zip.read(name), 'utf-8')
-                    out_file = open(full_name, 'w')
+                    out_file = open(full_name, 'w', encoding='utf-8')
                     out_file.write(file_xml)
                 else:
                     out_file = open(full_name, 'wb')
@@ -645,8 +649,8 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R
             delete_file(self.old_background_image)
         out_file = None
         try:
-            out_file = open(theme_file, 'w')
-            out_file.write(theme_pretty_xml.decode('UTF-8'))
+            out_file = open(theme_file, 'w', encoding='utf-8')
+            out_file.write(theme_pretty_xml.decode('utf-8'))
         except IOError:
             self.log_exception('Saving theme to file failed')
         finally:

openlp/plugins/custom/lib/mediaitem.py

@@ -158,6 +158,10 @@ class CustomMediaItem(MediaManagerItem):
                 self.remote_triggered = None
                 self.remote_custom = 1
                 if item:
+                    if preview:
+                        # A custom slide can only be edited if it comes from plugin,
+                        # so we set it again for the new item.
+                        item.from_plugin = True
                     return item
         return None
 

openlp/plugins/songs/lib/openlyricsxml.py

@@ -55,7 +55,7 @@ The XML of an `OpenLyrics <http://openlyrics.info/>`_  song looks like this::
         </lyrics>
     </song>
 """
-import cgi
+import html
 import logging
 import re
 
@@ -318,7 +318,7 @@ class OpenLyrics(object):
             if 'lang' in verse[0]:
                 verse_element.set('lang', verse[0]['lang'])
             # Create a list with all "optional" verses.
-            optional_verses = cgi.escape(verse[1])
+            optional_verses = html.escape(verse[1])
             optional_verses = optional_verses.split('\n[---]\n')
             start_tags = ''
             end_tags = ''

tests/functional/openlp_core_lib/test_lib.py

@@ -176,7 +176,7 @@ class TestLib(TestCase):
 
             # THEN: None should be returned
             mocked_isfile.assert_called_with(filename)
-            mocked_open.assert_called_with(filename, 'r')
+            mocked_open.assert_called_with(filename, 'r', encoding='utf-8')
             self.assertIsNone(result, 'None should be returned if the file cannot be opened')
 
     def get_text_file_string_decode_error_test(self):

tests/functional/openlp_core_ui/test_thememanager.py

@@ -29,6 +29,7 @@ from unittest import TestCase
 from tempfile import mkdtemp
 
 from PyQt4 import QtGui
+from tempfile import mkdtemp
 
 from openlp.core.ui import ThemeManager
 from openlp.core.common import Registry
@@ -44,6 +45,13 @@ class TestThemeManager(TestCase):
         Set up the tests
         """
         Registry.create()
+        self.temp_folder = mkdtemp()
+
+    def tearDown(self):
+        """
+        Clean up
+        """
+        shutil.rmtree(self.temp_folder)
 
     def export_theme_test(self):
         """
@@ -131,6 +139,27 @@ class TestThemeManager(TestCase):
             # THEN: The mocked_copyfile should not have been called
             self.assertTrue(mocked_copyfile.called, 'shutil.copyfile should be called')
 
+    def write_theme_special_char_name_test(self):
+        """
+        Test that we can save themes with special characters in the name
+        """
+        # GIVEN: A new theme manager instance, with mocked theme and thememanager-attributes.
+        theme_manager = ThemeManager(None)
+        theme_manager.old_background_image = None
+        theme_manager.generate_and_save_image = MagicMock()
+        theme_manager.path = self.temp_folder
+        mocked_theme = MagicMock()
+        mocked_theme.theme_name = 'theme 愛 name'
+        mocked_theme.extract_formatted_xml = MagicMock()
+        mocked_theme.extract_formatted_xml.return_value = 'fake theme 愛 XML'.encode()
+
+        # WHEN: Calling _write_theme with a theme with a name with special characters in it
+        theme_manager._write_theme(mocked_theme, None, None)
+
+        # THEN: It should have been created
+        self.assertTrue(os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.xml')),
+                        'Theme with special characters should have been created!')
+
     def over_write_message_box_yes_test(self):
         """
         Test that theme_manager.over_write_message_box returns True when the user clicks yes.