openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #21809
[Merge] lp:~phill-ridout/openlp/bug1209515 into lp:openlp
Phill has proposed merging lp:~phill-ridout/openlp/bug1209515 into lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Related bugs:
Bug #1209515 in OpenLP: "getFileNames corrupts file names that use "special chars""
https://bugs.launchpad.net/openlp/+bug/1209515
For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/bug1209515/+merge/185683
Fixes Bug 1209515 by sub classing QFileDialog and reimplementing getOpenFileNames and attempting to urlunquote and file paths which do not exist
--
https://code.launchpad.net/~phill-ridout/openlp/bug1209515/+merge/185683
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py 2013-08-31 18:17:38 +0000
+++ openlp/core/lib/__init__.py 2013-09-15 19:44:15 +0000
@@ -376,6 +376,7 @@
from .registry import Registry
from .uistrings import UiStrings
+from .filedialog import FileDialog
from .screen import ScreenList
from .settings import Settings
from .listwidgetwithdnd import ListWidgetWithDnD
@@ -392,4 +393,3 @@
from .imagemanager import ImageManager
from .renderer import Renderer
from .mediamanageritem import MediaManagerItem
-
=== added file 'openlp/core/lib/filedialog.py'
--- openlp/core/lib/filedialog.py 1970-01-01 00:00:00 +0000
+++ openlp/core/lib/filedialog.py 2013-09-15 19:44:15 +0000
@@ -0,0 +1,66 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2013 Raoul Snyman #
+# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan #
+# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, #
+# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. #
+# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, #
+# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, #
+# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, #
+# Frode Woldsund, Martin Zibricky #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it #
+# under the terms of the GNU General Public License as published by the Free #
+# Software Foundation; version 2 of the License. #
+# #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
+# more details. #
+# #
+# You should have received a copy of the GNU General Public License along #
+# with this program; if not, write to the Free Software Foundation, Inc., 59 #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
+###############################################################################
+
+"""
+Provide a work around for a bug in QFileDialog <https://bugs.launchpad.net/openlp/+bug/1209515>
+"""
+import logging
+import os
+from urllib import parse
+
+from PyQt4 import QtGui
+
+from openlp.core.lib import UiStrings
+
+log = logging.getLogger(__name__)
+
+class FileDialog(QtGui.QFileDialog):
+ """
+ Subclass QFileDialog to work round a bug
+ """
+ @staticmethod
+ def getOpenFileNames(parent, *args, **kwargs):
+ """
+ Reimplement getOpenFileNames to fix the way it returns some file names that url encoded when selecting multiple
+ files
+ """
+ files = QtGui.QFileDialog.getOpenFileNames(parent, *args, **kwargs)
+ file_list = []
+ for file in files:
+ if not os.path.exists(file):
+ log.info('File %s not found. Attempting to unquote.')
+ file = parse.unquote(file)
+ if not os.path.exists(file):
+ log.error('File %s not found.' % file)
+ QtGui.QMessageBox.information(parent, UiStrings().FileNotFound,
+ UiStrings().FileNotFoundMessage % file)
+ continue
+ log.info('File %s found.')
+ file_list.append(file)
+ return file_list
\ No newline at end of file
=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py 2013-08-31 18:17:38 +0000
+++ openlp/core/lib/mediamanageritem.py 2013-09-15 19:44:15 +0000
@@ -35,7 +35,7 @@
from PyQt4 import QtCore, QtGui
-from openlp.core.lib import OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \
+from openlp.core.lib import FileDialog, OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \
ServiceItemContext, Settings, Registry, UiStrings, translate
from openlp.core.lib.searchedit import SearchEdit
from openlp.core.lib.ui import create_widget_action, critical_error_message_box
@@ -305,7 +305,7 @@
"""
Add a file to the list widget to make it available for showing
"""
- files = QtGui.QFileDialog.getOpenFileNames(self, self.on_new_prompt,
+ files = FileDialog.getOpenFileNames(self, self.on_new_prompt,
Settings().value(self.settings_section + '/last directory'), self.on_new_file_masks)
log.info('New files(s) %s', files)
if files:
=== modified file 'openlp/core/lib/uistrings.py'
--- openlp/core/lib/uistrings.py 2013-04-15 21:31:04 +0000
+++ openlp/core/lib/uistrings.py 2013-09-15 19:44:15 +0000
@@ -83,6 +83,8 @@
self.Error = translate('OpenLP.Ui', 'Error')
self.Export = translate('OpenLP.Ui', 'Export')
self.File = translate('OpenLP.Ui', 'File')
+ self.FileNotFound = translate('OpenLP.Ui', 'File Not Found')
+ self.FileNotFoundMessage = translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.')
self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit')
self.Help = translate('OpenLP.Ui', 'Help')
self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours')
=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py 2013-08-31 18:17:38 +0000
+++ openlp/core/ui/thememanager.py 2013-09-15 19:44:15 +0000
@@ -38,8 +38,9 @@
from xml.etree.ElementTree import ElementTree, XML
from PyQt4 import QtCore, QtGui
-from openlp.core.lib import ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, get_text_file_string, \
- build_icon, translate, check_item_selected, check_directory_exists, create_thumb, validate_thumb
+from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, \
+ get_text_file_string, build_icon, translate, check_item_selected, check_directory_exists, create_thumb, \
+ validate_thumb
from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, BackgroundGradientType
from openlp.core.lib.ui import critical_error_message_box, create_widget_action
from openlp.core.theme import Theme
@@ -374,7 +375,7 @@
Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from
those files. This process will load both OpenLP version 1 and version 2 themes.
"""
- files = QtGui.QFileDialog.getOpenFileNames(self,
+ files = FileDialog.getOpenFileNames(self,
translate('OpenLP.ThemeManager', 'Select Theme Import File'),
Settings().value(self.settings_section + '/last directory import'),
translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)'))
=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py 2013-08-31 18:17:38 +0000
+++ openlp/plugins/songs/forms/editsongform.py 2013-09-15 19:44:15 +0000
@@ -38,8 +38,8 @@
from PyQt4 import QtCore, QtGui
-from openlp.core.lib import Registry, PluginStatus, MediaType, UiStrings, translate, create_separated_list, \
- check_directory_exists
+from openlp.core.lib import FileDialog, Registry, PluginStatus, MediaType, UiStrings, translate, \
+ create_separated_list, check_directory_exists
from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box
from openlp.core.utils import AppLocation
from openlp.plugins.songs.lib import VerseType, clean_song
@@ -760,7 +760,7 @@
Loads file(s) from the filesystem.
"""
filters = '%s (*)' % UiStrings().AllFiles
- filenames = QtGui.QFileDialog.getOpenFileNames(self,
+ filenames = FileDialog.getOpenFileNames(self,
translate('SongsPlugin.EditSongForm', 'Open File(s)'), '', filters)
for filename in filenames:
item = QtGui.QListWidgetItem(os.path.split(str(filename))[1])
=== modified file 'openlp/plugins/songs/forms/songimportform.py'
--- openlp/plugins/songs/forms/songimportform.py 2013-08-31 18:17:38 +0000
+++ openlp/plugins/songs/forms/songimportform.py 2013-09-15 19:44:15 +0000
@@ -35,7 +35,7 @@
from PyQt4 import QtCore, QtGui
-from openlp.core.lib import Registry, Settings, UiStrings, translate
+from openlp.core.lib import FileDialog, Registry, Settings, UiStrings, translate
from openlp.core.lib.ui import critical_error_message_box
from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
from openlp.plugins.songs.lib.importer import SongFormat, SongFormatSelect
@@ -244,7 +244,7 @@
if filters:
filters += ';;'
filters += '%s (*)' % UiStrings().AllFiles
- filenames = QtGui.QFileDialog.getOpenFileNames(self, title,
+ filenames = FileDialog.getOpenFileNames(self, title,
Settings().value(self.plugin.settings_section + '/last directory import'), filters)
if filenames:
listbox.addItems(filenames)
=== added file 'tests/functional/openlp_core_lib/test_file_dialog.py'
--- tests/functional/openlp_core_lib/test_file_dialog.py 1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_core_lib/test_file_dialog.py 2013-09-15 19:44:15 +0000
@@ -0,0 +1,71 @@
+"""
+Package to test the openlp.core.lib.filedialog package.
+"""
+import copy
+from unittest import TestCase
+from mock import MagicMock, call, patch
+
+from openlp.core.lib.filedialog import FileDialog
+from openlp.core.lib.uistrings import UiStrings
+
+class TestFileDialog(TestCase):
+ """
+ Test the functions in the :mod:`filedialog` module.
+ """
+ def setUp(self):
+ self.os_patcher = patch('openlp.core.lib.filedialog.os')
+ self.qt_gui_patcher = patch('openlp.core.lib.filedialog.QtGui')
+ self.ui_strings_patcher = patch('openlp.core.lib.filedialog.UiStrings')
+ self.mocked_os = self.os_patcher.start()
+ self.mocked_qt_gui = self.qt_gui_patcher.start()
+ self.mocked_ui_strings = self.ui_strings_patcher.start()
+ self.mocked_parent = MagicMock()
+
+ def tearDown(self):
+ self.os_patcher.stop()
+ self.qt_gui_patcher.stop()
+ self.ui_strings_patcher.stop()
+
+ def get_open_file_names_canceled_test(self):
+ """
+ Test that FileDialog.getOpenFileNames() returns and empty QStringList when QFileDialog is canceled
+ (returns an empty QStringList)
+ """
+ self.mocked_os.reset()
+
+ # GIVEN: An empty QStringList as a return value from QFileDialog.getOpenFileNames
+ self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = []
+
+ # WHEN: FileDialog.getOpenFileNames is called
+ result = FileDialog.getOpenFileNames(self.mocked_parent)
+
+ # THEN: The returned value should be an empty QStringList and os.path.exists should not have been called
+ assert not self.mocked_os.path.exists.called
+ self.assertEqual(result, [],
+ 'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled')
+
+ def returned_file_list_test(self):
+ """
+ Test that FileDialog.getOpenFileNames handles a list of files properly when QFileList.getOpenFileNames
+ returns a good file name, a urlencoded file name and a non-existing file
+ """
+ self.mocked_os.rest()
+ self.mocked_qt_gui.reset()
+
+ # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNames and a list of valid
+ # file names.
+ self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [
+ '/Valid File', '/url%20encoded%20file%20%231', '/non-existing']
+ self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [
+ '/Valid File', '/url encoded file #1']
+
+ # WHEN: FileDialog.getOpenFileNames is called
+ result = FileDialog.getOpenFileNames(self.mocked_parent)
+
+ # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been
+ # called. The returned result should corrilate with the input.
+ self.mocked_os.path.exists.assert_has_calls([call('/Valid File'), call('/url%20encoded%20file%20%231'),
+ call('/url encoded file #1'), call('/non-existing'), call('/non-existing')])
+ self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound,
+ UiStrings().FileNotFoundMessage % '/non-existing')
+ self.assertEqual(result, ['/Valid File', '/url encoded file #1'], 'The returned file list is incorrect')
\ No newline at end of file
Follow ups