Phill has proposed merging lp:~phill-ridout/openlp/bug1209515 into lp:openlp.
Requested reviews: OpenLP Core (openlp-core) 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/181475 Fixes Bug 1209515 by reimplementing QFileDialof.getFileNames and attempting to urldecode file paths that do not exisit -- https://code.launchpad.net/~phill-ridout/openlp/bug1209515/+merge/181475 Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/bug1209515 into lp:openlp.
=== modified file 'openlp/core/lib/__init__.py' --- openlp/core/lib/__init__.py 2013-06-03 17:19:42 +0000 +++ openlp/core/lib/__init__.py 2013-08-22 06:43:35 +0000 @@ -377,6 +377,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 === 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-08-22 06:43:35 +0000 @@ -0,0 +1,67 @@ +# -*- 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 +import urllib + +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: + file = unicode(file) + if not os.path.exists(file): + log.info(u'File %s not found. Attempting to unquote.') + file = urllib.unquote(unicode(file)) + if not os.path.exists(file): + log.error(u'File %s not found.' % file) + QtGui.QMessageBox.information(parent, UiStrings().FileNotFound, + UiStrings().FileNotFoundMessage % file) + continue + log.info(u'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-06-24 16:54:23 +0000 +++ openlp/core/lib/mediamanageritem.py 2013-08-22 06:43:35 +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 + u'/last directory'), self.on_new_file_masks) log.info(u'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-08-22 06:43:35 +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 = unicode(translate('OpenLP.Ui', 'File Not Found')) + self.FileNotFoundMessage = unicode(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-06-24 16:54:23 +0000 +++ openlp/core/ui/thememanager.py 2013-08-22 06:43:35 +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 @@ -373,7 +374,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 + u'/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-07-22 20:27:51 +0000 +++ openlp/plugins/songs/forms/editsongform.py 2013-08-22 06:43:35 +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 @@ -755,7 +755,7 @@ Loads file(s) from the filesystem. """ filters = u'%s (*)' % UiStrings().AllFiles - filenames = QtGui.QFileDialog.getOpenFileNames(self, + filenames = FileDialog().getOpenFileNames(self, translate('SongsPlugin.EditSongForm', 'Open File(s)'), u'', filters) for filename in filenames: item = QtGui.QListWidgetItem(os.path.split(unicode(filename))[1]) === modified file 'openlp/plugins/songs/forms/songimportform.py' --- openlp/plugins/songs/forms/songimportform.py 2013-06-27 08:30:34 +0000 +++ openlp/plugins/songs/forms/songimportform.py 2013-08-22 06:43:35 +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 += u';;' filters += u'%s (*)' % UiStrings().AllFiles - filenames = QtGui.QFileDialog.getOpenFileNames(self, title, + filenames = FileDialog().getOpenFileNames(self, title, Settings().value(self.plugin.settings_section + u'/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-08-22 06:43:35 +0000 @@ -0,0 +1,71 @@ +""" +Package to test the openlp.core.lib.formattingtags 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(u'openlp.core.lib.filedialog.os') + self.qt_gui_patcher = patch(u'openlp.core.lib.filedialog.QtGui') + self.ui_strings_patcher = patch(u'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 QStiingList and os.path.exists should not have been called + assert not self.mocked_os.path.exists.called + self.assertEqual(result, [], + u'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.getOpenFileNamesand a list of valid + # file names. + self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [ + u'/Valid File', u'/url%20encoded%20file%20%231', u'/non-existing'] + self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [ + u'/Valid File', u'/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(u'/Valid File'), call(u'/url%20encoded%20file%20%231'), + call(u'/url encoded file #1'), call(u'/non-existing'), call(u'/non-existing')]) + self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FNFT, + UiStrings().FNF % u'/non-existing') + self.assertEqual(result, [u'/Valid File', u'/url encoded file #1'], u'The returned file list is incorrect') \ No newline at end of file
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
