From 364fde73c54926ef67c02ee813c5c08ac854d402 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Tue, 13 Aug 2013 19:02:04 +0100 Subject: [PATCH] Fix #120515 by reimplementing QFileDialog.getOpenFileNames --- openlp/core/lib/__init__.py | 1 + openlp/core/lib/filedialog.py | 66 +++++++++++++++++ openlp/core/lib/mediamanageritem.py | 4 +- openlp/core/lib/uistrings.py | 2 + openlp/core/ui/thememanager.py | 7 +- openlp/plugins/songs/forms/editsongform.py | 6 +- openlp/plugins/songs/forms/songimportform.py | 4 +- .../openlp_core_lib/test_file_dialog.py | 71 +++++++++++++++++++ 8 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 openlp/core/lib/filedialog.py create mode 100644 tests/functional/openlp_core_lib/test_file_dialog.py diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index d6c338271..61891c1ef 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -377,6 +377,7 @@ def create_separated_list(stringlist): from registry import Registry from uistrings import UiStrings +from filedialog import FileDialog from screen import ScreenList from settings import Settings from listwidgetwithdnd import ListWidgetWithDnD diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py new file mode 100644 index 000000000..b2159b511 --- /dev/null +++ b/openlp/core/lib/filedialog.py @@ -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 (#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): + """ + Inherit form QFileDialog + """ + @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().FNFT, UiStrings().FNF % file) + continue + log.info(u'File %s found.') + file_list.append(file) + return file_list \ No newline at end of file diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index 973d457bb..1aa622b1e 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -35,7 +35,7 @@ import re 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 @@ class MediaManagerItem(QtGui.QWidget): """ 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: diff --git a/openlp/core/lib/uistrings.py b/openlp/core/lib/uistrings.py index 6d4b4d250..26ad63b55 100644 --- a/openlp/core/lib/uistrings.py +++ b/openlp/core/lib/uistrings.py @@ -83,6 +83,8 @@ class UiStrings(object): self.Error = translate('OpenLP.Ui', 'Error') self.Export = translate('OpenLP.Ui', 'Export') self.File = translate('OpenLP.Ui', 'File') + self.FNFT = unicode(translate('OpenLP.Ui', 'File Not Found')) + self.FNF = 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') diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 58f29cab6..a1f761d8f 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -38,8 +38,9 @@ import re 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 @@ class ThemeManager(QtGui.QWidget): 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)')) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 24d0d3024..f73a4468c 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -38,8 +38,8 @@ import shutil 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 @@ -753,7 +753,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): 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]) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index c1aca92ad..bd9acc943 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -35,7 +35,7 @@ import os 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 @@ class SongImportForm(OpenLPWizard): 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) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py new file mode 100644 index 000000000..dc704eaef --- /dev/null +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -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