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
_______________________________________________ Mailing list: https://launchpad.net/~openlp-core Post to : [email protected] Unsubscribe : https://launchpad.net/~openlp-core More help : https://help.launchpad.net/ListHelp
