From 25f6fe0a594f8b9cbd3968c3f67d835e1609acf1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 20 Mar 2015 17:03:45 +0000 Subject: [PATCH 1/6] Fix another case of bug 1422761. Fixes: https://launchpad.net/bugs/1422761 --- openlp/core/ui/media/mediacontroller.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlp/core/ui/media/mediacontroller.py b/openlp/core/ui/media/mediacontroller.py index f91c46a7d..ebd647822 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 \ From 335c804e8f7054ee2818ad785736e42b821608da Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 2 Apr 2015 10:04:56 +0100 Subject: [PATCH 2/6] Fix support for special characters in theme names. Fixes bug 1438563. Fixes: https://launchpad.net/bugs/1438563 --- openlp/core/lib/__init__.py | 2 +- openlp/core/ui/thememanager.py | 14 +++++---- tests/functional/openlp_core_lib/test_lib.py | 2 +- .../openlp_core_ui/test_thememanager.py | 29 +++++++++++++++++++ 4 files changed, 40 insertions(+), 7 deletions(-) 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/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/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. From d2301e016ec61f7f1145a794c5e574dc40a3d8c4 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 2 Apr 2015 13:50:50 +0100 Subject: [PATCH 3/6] improved logging --- openlp/core/common/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index 0ae62e0a4..526c51878 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -66,8 +66,11 @@ 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 yo check/create if directury exists') + log.exception(e) + #pass def get_frozen_path(frozen_option, non_frozen_option): From f8b575065a6cb1c0ee389de4773d2e1512593422 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 2 Apr 2015 21:29:43 +0100 Subject: [PATCH 4/6] pep8 fix --- openlp/core/common/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index 526c51878..ccb982206 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -68,9 +68,7 @@ def check_directory_exists(directory, do_not_log=False): os.makedirs(directory) except IOError as e: if not do_not_log: - log.exception('failed yo check/create if directury exists') - log.exception(e) - #pass + log.exception('failed to check if directory exists or create directory') def get_frozen_path(frozen_option, non_frozen_option): From 589bc98bec9d8a8f233254314d37e8e9425d813c Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 2 Apr 2015 21:32:20 +0100 Subject: [PATCH 5/6] Use html.escape instead of the deprecated cgi.escape --- openlp/core/ui/maindisplay.py | 4 ++-- openlp/plugins/songs/lib/openlyricsxml.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) 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/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 = '' From da6f88a7ae967314cd9a0e0920496439062df54a Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 2 Apr 2015 21:49:19 +0100 Subject: [PATCH 6/6] Mark a custom slide edited from preview as coming from plugin. Fixes bug 1439671. Fixes: https://launchpad.net/bugs/1439671 --- openlp/plugins/custom/lib/mediaitem.py | 4 ++++ 1 file changed, 4 insertions(+) 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