diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index 0ae62e0a4..ccb982206 100644 --- a/openlp/core/common/__init__.py +++ b/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): diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 0439fc156..fe7cef4b2 100644 --- a/openlp/core/lib/__init__.py +++ b/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) diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index 7085626ef..68d5d6d3d 100644 --- a/openlp/core/ui/maindisplay.py +++ b/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') diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index 494bd54c3..158c44e71 100644 --- a/openlp/core/ui/media/mediacontroller.py +++ b/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 \ diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 0db4b12a7..9f8d0bfe0 100644 --- a/openlp/core/ui/thememanager.py +++ b/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: diff --git a/openlp/plugins/custom/lib/mediaitem.py b/openlp/plugins/custom/lib/mediaitem.py index 8667292ec..dc0411ad8 100644 --- a/openlp/plugins/custom/lib/mediaitem.py +++ b/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 diff --git a/openlp/plugins/songs/lib/openlyricsxml.py b/openlp/plugins/songs/lib/openlyricsxml.py index b9f6b6bd1..e6b01cfb8 100644 --- a/openlp/plugins/songs/lib/openlyricsxml.py +++ b/openlp/plugins/songs/lib/openlyricsxml.py @@ -55,7 +55,7 @@ The XML of an `OpenLyrics `_ song looks like this:: """ -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 = '' diff --git a/tests/functional/openlp_core_lib/test_lib.py b/tests/functional/openlp_core_lib/test_lib.py index 98c31d7be..9fc1fa3ff 100644 --- a/tests/functional/openlp_core_lib/test_lib.py +++ b/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): diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 85d470e4f..3dc168f77 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/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.