Merge lp:~phill-ridout/openlp/bug1209515 into lp:openlp
- bug1209515
- Merge into trunk
Proposed by
Phill
Status: | Superseded |
---|---|
Proposed branch: | lp:~phill-ridout/openlp/bug1209515 |
Merge into: | lp:openlp |
Diff against target: |
282 lines (+154/-14) 8 files modified
openlp/core/lib/__init__.py (+4/-4) openlp/core/lib/filedialog.py (+66/-0) openlp/core/lib/mediamanageritem.py (+2/-2) openlp/core/lib/uistrings.py (+2/-0) openlp/core/ui/thememanager.py (+4/-3) openlp/plugins/songs/forms/editsongform.py (+3/-3) openlp/plugins/songs/forms/songimportform.py (+2/-2) tests/functional/openlp_core_lib/test_file_dialog.py (+71/-0) |
To merge this branch: | bzr merge lp:~phill-ridout/openlp/bug1209515 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Bentley | Needs Resubmitting | ||
Review via email: mp+181654@code.launchpad.net |
This proposal has been superseded by a proposal from 2013-09-15.
Commit message
Description of the change
Fixes Bug 1209515 by sub classing QFileDialog and reimplementing getOpenFileNames and attempting to urlunquote and file paths which do not exist
To post a comment you must log in.
Unmerged revisions
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'openlp/core/lib/__init__.py' |
2 | --- openlp/core/lib/__init__.py 2013-08-31 18:17:38 +0000 |
3 | +++ openlp/core/lib/__init__.py 2013-09-15 19:28:36 +0000 |
4 | @@ -370,12 +370,13 @@ |
5 | 'Locale list separator: end') % (stringlist[-2], stringlist[-1]) |
6 | for index in reversed(list(range(1, len(stringlist) - 2))): |
7 | merged = translate('OpenLP.core.lib', '%s, %s', |
8 | - 'Locale list separator: middle') % (stringlist[index], merged) |
9 | - return translate('OpenLP.core.lib', '%s, %s', 'Locale list separator: start') % (stringlist[0], merged) |
10 | + u'Locale list separator: middle') % (stringlist[index], merged) |
11 | + return translate('OpenLP.core.lib', '%s, %s', u'Locale list separator: start') % (stringlist[0], merged) |
12 | |
13 | |
14 | from .registry import Registry |
15 | from .uistrings import UiStrings |
16 | +from .filedialog import FileDialog |
17 | from .screen import ScreenList |
18 | from .settings import Settings |
19 | from .listwidgetwithdnd import ListWidgetWithDnD |
20 | @@ -391,5 +392,4 @@ |
21 | from .dockwidget import OpenLPDockWidget |
22 | from .imagemanager import ImageManager |
23 | from .renderer import Renderer |
24 | -from .mediamanageritem import MediaManagerItem |
25 | - |
26 | +from .mediamanageritem import MediaManagerItem |
27 | \ No newline at end of file |
28 | |
29 | === added file 'openlp/core/lib/filedialog.py' |
30 | --- openlp/core/lib/filedialog.py 1970-01-01 00:00:00 +0000 |
31 | +++ openlp/core/lib/filedialog.py 2013-09-15 19:28:36 +0000 |
32 | @@ -0,0 +1,66 @@ |
33 | +# -*- coding: utf-8 -*- |
34 | +# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4 |
35 | + |
36 | +############################################################################### |
37 | +# OpenLP - Open Source Lyrics Projection # |
38 | +# --------------------------------------------------------------------------- # |
39 | +# Copyright (c) 2008-2013 Raoul Snyman # |
40 | +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # |
41 | +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # |
42 | +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # |
43 | +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # |
44 | +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # |
45 | +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # |
46 | +# Frode Woldsund, Martin Zibricky # |
47 | +# --------------------------------------------------------------------------- # |
48 | +# This program is free software; you can redistribute it and/or modify it # |
49 | +# under the terms of the GNU General Public License as published by the Free # |
50 | +# Software Foundation; version 2 of the License. # |
51 | +# # |
52 | +# This program is distributed in the hope that it will be useful, but WITHOUT # |
53 | +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # |
54 | +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # |
55 | +# more details. # |
56 | +# # |
57 | +# You should have received a copy of the GNU General Public License along # |
58 | +# with this program; if not, write to the Free Software Foundation, Inc., 59 # |
59 | +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # |
60 | +############################################################################### |
61 | + |
62 | +""" |
63 | +Provide a work around for a bug in QFileDialog <https://bugs.launchpad.net/openlp/+bug/1209515> |
64 | +""" |
65 | +import logging |
66 | +import os |
67 | +from urllib import parse |
68 | + |
69 | +from PyQt4 import QtGui |
70 | + |
71 | +from openlp.core.lib import UiStrings |
72 | + |
73 | +log = logging.getLogger(__name__) |
74 | + |
75 | +class FileDialog(QtGui.QFileDialog): |
76 | + """ |
77 | + Subclass QFileDialog to work round a bug |
78 | + """ |
79 | + @staticmethod |
80 | + def getOpenFileNames(parent, *args, **kwargs): |
81 | + """ |
82 | + Reimplement getOpenFileNames to fix the way it returns some file names that url encoded when selecting multiple |
83 | + files |
84 | + """ |
85 | + files = QtGui.QFileDialog.getOpenFileNames(parent, *args, **kwargs) |
86 | + file_list = [] |
87 | + for file in files: |
88 | + if not os.path.exists(file): |
89 | + log.info('File %s not found. Attempting to unquote.') |
90 | + file = parse.unquote(file) |
91 | + if not os.path.exists(file): |
92 | + log.error('File %s not found.' % file) |
93 | + QtGui.QMessageBox.information(parent, UiStrings().FileNotFound, |
94 | + UiStrings().FileNotFoundMessage % file) |
95 | + continue |
96 | + log.info('File %s found.') |
97 | + file_list.append(file) |
98 | + return file_list |
99 | \ No newline at end of file |
100 | |
101 | === modified file 'openlp/core/lib/mediamanageritem.py' |
102 | --- openlp/core/lib/mediamanageritem.py 2013-08-31 18:17:38 +0000 |
103 | +++ openlp/core/lib/mediamanageritem.py 2013-09-15 19:28:36 +0000 |
104 | @@ -35,7 +35,7 @@ |
105 | |
106 | from PyQt4 import QtCore, QtGui |
107 | |
108 | -from openlp.core.lib import OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \ |
109 | +from openlp.core.lib import FileDialog, OpenLPToolbar, ServiceItem, StringContent, ListWidgetWithDnD, \ |
110 | ServiceItemContext, Settings, Registry, UiStrings, translate |
111 | from openlp.core.lib.searchedit import SearchEdit |
112 | from openlp.core.lib.ui import create_widget_action, critical_error_message_box |
113 | @@ -305,7 +305,7 @@ |
114 | """ |
115 | Add a file to the list widget to make it available for showing |
116 | """ |
117 | - files = QtGui.QFileDialog.getOpenFileNames(self, self.on_new_prompt, |
118 | + files = FileDialog.getOpenFileNames(self, self.on_new_prompt, |
119 | Settings().value(self.settings_section + '/last directory'), self.on_new_file_masks) |
120 | log.info('New files(s) %s', files) |
121 | if files: |
122 | |
123 | === modified file 'openlp/core/lib/uistrings.py' |
124 | --- openlp/core/lib/uistrings.py 2013-04-15 21:31:04 +0000 |
125 | +++ openlp/core/lib/uistrings.py 2013-09-15 19:28:36 +0000 |
126 | @@ -83,6 +83,8 @@ |
127 | self.Error = translate('OpenLP.Ui', 'Error') |
128 | self.Export = translate('OpenLP.Ui', 'Export') |
129 | self.File = translate('OpenLP.Ui', 'File') |
130 | + self.FileNotFound = translate('OpenLP.Ui', 'File Not Found') |
131 | + self.FileNotFoundMessage = translate('OpenLP.Ui', 'File %s not found.\nPlease try selecting it individually.') |
132 | self.FontSizePtUnit = translate('OpenLP.Ui', 'pt', 'Abbreviated font pointsize unit') |
133 | self.Help = translate('OpenLP.Ui', 'Help') |
134 | self.Hours = translate('OpenLP.Ui', 'h', 'The abbreviated unit for hours') |
135 | |
136 | === modified file 'openlp/core/ui/thememanager.py' |
137 | --- openlp/core/ui/thememanager.py 2013-08-31 18:17:38 +0000 |
138 | +++ openlp/core/ui/thememanager.py 2013-09-15 19:28:36 +0000 |
139 | @@ -38,8 +38,9 @@ |
140 | from xml.etree.ElementTree import ElementTree, XML |
141 | from PyQt4 import QtCore, QtGui |
142 | |
143 | -from openlp.core.lib import ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, get_text_file_string, \ |
144 | - build_icon, translate, check_item_selected, check_directory_exists, create_thumb, validate_thumb |
145 | +from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, Registry, Settings, UiStrings, \ |
146 | + get_text_file_string, build_icon, translate, check_item_selected, check_directory_exists, create_thumb, \ |
147 | + validate_thumb |
148 | from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, BackgroundGradientType |
149 | from openlp.core.lib.ui import critical_error_message_box, create_widget_action |
150 | from openlp.core.theme import Theme |
151 | @@ -374,7 +375,7 @@ |
152 | Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from |
153 | those files. This process will load both OpenLP version 1 and version 2 themes. |
154 | """ |
155 | - files = QtGui.QFileDialog.getOpenFileNames(self, |
156 | + files = FileDialog.getOpenFileNames(self, |
157 | translate('OpenLP.ThemeManager', 'Select Theme Import File'), |
158 | Settings().value(self.settings_section + '/last directory import'), |
159 | translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)')) |
160 | |
161 | === modified file 'openlp/plugins/songs/forms/editsongform.py' |
162 | --- openlp/plugins/songs/forms/editsongform.py 2013-08-31 18:17:38 +0000 |
163 | +++ openlp/plugins/songs/forms/editsongform.py 2013-09-15 19:28:36 +0000 |
164 | @@ -38,8 +38,8 @@ |
165 | |
166 | from PyQt4 import QtCore, QtGui |
167 | |
168 | -from openlp.core.lib import Registry, PluginStatus, MediaType, UiStrings, translate, create_separated_list, \ |
169 | - check_directory_exists |
170 | +from openlp.core.lib import FileDialog, Registry, PluginStatus, MediaType, UiStrings, translate, \ |
171 | + create_separated_list, check_directory_exists |
172 | from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box |
173 | from openlp.core.utils import AppLocation |
174 | from openlp.plugins.songs.lib import VerseType, clean_song |
175 | @@ -760,7 +760,7 @@ |
176 | Loads file(s) from the filesystem. |
177 | """ |
178 | filters = '%s (*)' % UiStrings().AllFiles |
179 | - filenames = QtGui.QFileDialog.getOpenFileNames(self, |
180 | + filenames = FileDialog.getOpenFileNames(self, |
181 | translate('SongsPlugin.EditSongForm', 'Open File(s)'), '', filters) |
182 | for filename in filenames: |
183 | item = QtGui.QListWidgetItem(os.path.split(str(filename))[1]) |
184 | |
185 | === modified file 'openlp/plugins/songs/forms/songimportform.py' |
186 | --- openlp/plugins/songs/forms/songimportform.py 2013-08-31 18:17:38 +0000 |
187 | +++ openlp/plugins/songs/forms/songimportform.py 2013-09-15 19:28:36 +0000 |
188 | @@ -35,7 +35,7 @@ |
189 | |
190 | from PyQt4 import QtCore, QtGui |
191 | |
192 | -from openlp.core.lib import Registry, Settings, UiStrings, translate |
193 | +from openlp.core.lib import FileDialog, Registry, Settings, UiStrings, translate |
194 | from openlp.core.lib.ui import critical_error_message_box |
195 | from openlp.core.ui.wizard import OpenLPWizard, WizardStrings |
196 | from openlp.plugins.songs.lib.importer import SongFormat, SongFormatSelect |
197 | @@ -244,7 +244,7 @@ |
198 | if filters: |
199 | filters += ';;' |
200 | filters += '%s (*)' % UiStrings().AllFiles |
201 | - filenames = QtGui.QFileDialog.getOpenFileNames(self, title, |
202 | + filenames = FileDialog.getOpenFileNames(self, title, |
203 | Settings().value(self.plugin.settings_section + '/last directory import'), filters) |
204 | if filenames: |
205 | listbox.addItems(filenames) |
206 | |
207 | === added file 'tests/functional/openlp_core_lib/test_file_dialog.py' |
208 | --- tests/functional/openlp_core_lib/test_file_dialog.py 1970-01-01 00:00:00 +0000 |
209 | +++ tests/functional/openlp_core_lib/test_file_dialog.py 2013-09-15 19:28:36 +0000 |
210 | @@ -0,0 +1,71 @@ |
211 | +""" |
212 | +Package to test the openlp.core.lib.formattingtags package. |
213 | +""" |
214 | +import copy |
215 | +from unittest import TestCase |
216 | +from mock import MagicMock, call, patch |
217 | + |
218 | +from openlp.core.lib.filedialog import FileDialog |
219 | +from openlp.core.lib.uistrings import UiStrings |
220 | + |
221 | +class TestFileDialog(TestCase): |
222 | + """ |
223 | + Test the functions in the :mod:`filedialog` module. |
224 | + """ |
225 | + def setUp(self): |
226 | + self.os_patcher = patch(u'openlp.core.lib.filedialog.os') |
227 | + self.qt_gui_patcher = patch(u'openlp.core.lib.filedialog.QtGui') |
228 | + self.ui_strings_patcher = patch(u'openlp.core.lib.filedialog.UiStrings') |
229 | + self.mocked_os = self.os_patcher.start() |
230 | + self.mocked_qt_gui = self.qt_gui_patcher.start() |
231 | + self.mocked_ui_strings = self.ui_strings_patcher.start() |
232 | + self.mocked_parent = MagicMock() |
233 | + |
234 | + def tearDown(self): |
235 | + self.os_patcher.stop() |
236 | + self.qt_gui_patcher.stop() |
237 | + self.ui_strings_patcher.stop() |
238 | + |
239 | + def get_open_file_names_canceled_test(self): |
240 | + """ |
241 | + Test that FileDialog.getOpenFileNames() returns and empty QStringList when QFileDialog is canceled |
242 | + (returns an empty QStringList) |
243 | + """ |
244 | + self.mocked_os.reset() |
245 | + |
246 | + # GIVEN: An empty QStringList as a return value from QFileDialog.getOpenFileNames |
247 | + self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [] |
248 | + |
249 | + # WHEN: FileDialog.getOpenFileNames is called |
250 | + result = FileDialog.getOpenFileNames(self.mocked_parent) |
251 | + |
252 | + # THEN: The returned value should be an empty QStiingList and os.path.exists should not have been called |
253 | + assert not self.mocked_os.path.exists.called |
254 | + self.assertEqual(result, [], |
255 | + u'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames is canceled') |
256 | + |
257 | + def returned_file_list_test(self): |
258 | + """ |
259 | + Test that FileDialog.getOpenFileNames handles a list of files properly when QFileList.getOpenFileNames |
260 | + returns a good file name, a urlencoded file name and a non-existing file |
261 | + """ |
262 | + self.mocked_os.rest() |
263 | + self.mocked_qt_gui.reset() |
264 | + |
265 | + # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNamesand a list of valid |
266 | + # file names. |
267 | + self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = [ |
268 | + u'/Valid File', u'/url%20encoded%20file%20%231', u'/non-existing'] |
269 | + self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [ |
270 | + u'/Valid File', u'/url encoded file #1'] |
271 | + |
272 | + # WHEN: FileDialog.getOpenFileNames is called |
273 | + result = FileDialog.getOpenFileNames(self.mocked_parent) |
274 | + |
275 | + # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been |
276 | + # called. The returned result should corrilate with the input. |
277 | + self.mocked_os.path.exists.assert_has_calls([call(u'/Valid File'), call(u'/url%20encoded%20file%20%231'), |
278 | + call(u'/url encoded file #1'), call(u'/non-existing'), call(u'/non-existing')]) |
279 | + self.mocked_qt_gui.QmessageBox.information.called_with(self.mocked_parent, UiStrings().FileNotFound, |
280 | + UiStrings().FileNotFoundMessage % u'/non-existing') |
281 | + self.assertEqual(result, [u'/Valid File', u'/url encoded file #1'], u'The returned file list is incorrect') |
282 | \ No newline at end of file |
Please resubmit due to the age of this request and additionally it will need converting to Python 3 as on 1/9/2013 truck will be converted.