Merge lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters into lp:widelands
- bug-1610507-disallowed-filename-characters
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 8722 |
Proposed branch: | lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters |
Merge into: | lp:widelands |
Diff against target: |
473 lines (+148/-56) 16 files modified
src/editor/ui_menus/main_menu_save_map.cc (+5/-2) src/editor/ui_menus/main_menu_save_map.h (+1/-0) src/editor/ui_menus/main_menu_save_map_make_directory.cc (+6/-3) src/editor/ui_menus/main_menu_save_map_make_directory.h (+1/-0) src/graphic/text_layout.cc (+7/-0) src/graphic/text_layout.h (+4/-0) src/io/CMakeLists.txt (+0/-21) src/io/filesystem/CMakeLists.txt (+23/-0) src/io/filesystem/filesystem.cc (+70/-0) src/io/filesystem/filesystem.h (+4/-0) src/io/filesystem/layered_filesystem.cc (+0/-16) src/io/filesystem/layered_filesystem.h (+1/-2) src/ui_basic/panel.cc (+20/-9) src/ui_basic/panel.h (+1/-1) src/wui/game_main_menu_save_game.cc (+4/-2) src/wui/game_main_menu_save_game.h (+1/-0) |
To merge this branch: | bzr merge lp:~widelands-dev/widelands/bug-1610507-disallowed-filename-characters |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Notabilis | Approve | ||
Review via email: mp+346442@code.launchpad.net |
Commit message
Added tooltip to file save and make directory edit boxes if illegal filename is being entered. Also, some refactoring:
- New function as_listitem in text layout
- Fixed tooltip drawing for modal panels
- Some code style tweaks to Panel
- Filename checks now live directly in Filesystem
Description of the change
bunnybot (widelandsofficial) wrote : | # |
bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 3540. State: passed. Details: https:/
Appveyor build 3344. State: success. Details: https:/
Notabilis (notabilis27) wrote : | # |
Nice change, I like the tooltip! Works as intended and code is looking good.
Some remarks:
- Maybe add the tooltip to the OK button as well? At least for me that would be the more likely place where I would notice it.
- Is the smaller font for the list in the tooltip intentional?
- Maybe calculate the tooltip text only once and store it as static string in the file system class?
Not really related: I noticed that in the editor->make directory dialog the OK button is initially disabled, even though it should (?) be enabled. This is done intentionally by calling set_enabled(false) when creating the dialog, even though the directory name is valid and can be used. Is there a reason for disabling it?
GunChleoc (gunchleoc) wrote : | # |
It is disabled because the text is always empty at the start, and empty names aren't allowed.
I like your suggestions and we should implement them before merging this.
The smaller text in the list is intentional - I have another branch for the editor info tool that uses the new listitem function too, and I wanted a consistent look.
Notabilis (notabilis27) wrote : | # |
In the case of the "make directory" dialog the default value of the edit box is "unnamed". This is a valid name (changing and changing it back is accepted) but the OK button is initially disabled anyway.
GunChleoc (gunchleoc) wrote : | # |
OK, that's a bug then.
GunChleoc (gunchleoc) wrote : | # |
OK button is fixed. Thanks for the review and testing :)
@bunnybot merge
Preview Diff
1 | === modified file 'src/editor/ui_menus/main_menu_save_map.cc' | |||
2 | --- src/editor/ui_menus/main_menu_save_map.cc 2018-05-22 06:54:12 +0000 | |||
3 | +++ src/editor/ui_menus/main_menu_save_map.cc 2018-05-31 17:12:19 +0000 | |||
4 | @@ -68,7 +68,8 @@ | |||
5 | 68 | UI::ButtonStyle::kWuiPrimary, | 68 | UI::ButtonStyle::kWuiPrimary, |
6 | 69 | _("Map Options")), | 69 | _("Map Options")), |
7 | 70 | editbox_label_( | 70 | editbox_label_( |
9 | 71 | this, padding_, tabley_ + tableh_ + 3 * padding_, butw_, buth_, _("Filename:")) { | 71 | this, padding_, tabley_ + tableh_ + 3 * padding_, butw_, buth_, _("Filename:")), |
10 | 72 | illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) { | ||
11 | 72 | set_current_directory(curdir_); | 73 | set_current_directory(curdir_); |
12 | 73 | 74 | ||
13 | 74 | // Make room for edit_options_ button | 75 | // Make room for edit_options_ button |
14 | @@ -220,7 +221,9 @@ | |||
15 | 220 | */ | 221 | */ |
16 | 221 | void MainMenuSaveMap::edit_box_changed() { | 222 | void MainMenuSaveMap::edit_box_changed() { |
17 | 222 | // Prevent the user from creating nonsense file names, like e.g. ".." or "...". | 223 | // Prevent the user from creating nonsense file names, like e.g. ".." or "...". |
19 | 223 | ok_.set_enabled(LayeredFileSystem::is_legal_filename(editbox_->text())); | 224 | const bool is_legal_filename = FileSystem::is_legal_filename(editbox_->text()); |
20 | 225 | ok_.set_enabled(is_legal_filename); | ||
21 | 226 | editbox_->set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_); | ||
22 | 224 | } | 227 | } |
23 | 225 | 228 | ||
24 | 226 | void MainMenuSaveMap::reset_editbox_or_die(const std::string& current_filename) { | 229 | void MainMenuSaveMap::reset_editbox_or_die(const std::string& current_filename) { |
25 | 227 | 230 | ||
26 | === modified file 'src/editor/ui_menus/main_menu_save_map.h' | |||
27 | --- src/editor/ui_menus/main_menu_save_map.h 2018-05-10 15:57:43 +0000 | |||
28 | +++ src/editor/ui_menus/main_menu_save_map.h 2018-05-31 17:12:19 +0000 | |||
29 | @@ -55,6 +55,7 @@ | |||
30 | 55 | 55 | ||
31 | 56 | UI::Textarea editbox_label_; | 56 | UI::Textarea editbox_label_; |
32 | 57 | UI::EditBox* editbox_; | 57 | UI::EditBox* editbox_; |
33 | 58 | const std::string illegal_filename_tooltip_; | ||
34 | 58 | }; | 59 | }; |
35 | 59 | 60 | ||
36 | 60 | #endif // end of include guard: WL_EDITOR_UI_MENUS_MAIN_MENU_SAVE_MAP_H | 61 | #endif // end of include guard: WL_EDITOR_UI_MENUS_MAIN_MENU_SAVE_MAP_H |
37 | 61 | 62 | ||
38 | === modified file 'src/editor/ui_menus/main_menu_save_map_make_directory.cc' | |||
39 | --- src/editor/ui_menus/main_menu_save_map_make_directory.cc 2018-04-27 06:11:05 +0000 | |||
40 | +++ src/editor/ui_menus/main_menu_save_map_make_directory.cc 2018-05-31 17:12:19 +0000 | |||
41 | @@ -54,7 +54,8 @@ | |||
42 | 54 | butw_, | 54 | butw_, |
43 | 55 | buth_, | 55 | buth_, |
44 | 56 | UI::ButtonStyle::kWuiSecondary, | 56 | UI::ButtonStyle::kWuiSecondary, |
46 | 57 | _("Cancel")) { | 57 | _("Cancel")), |
47 | 58 | illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) { | ||
48 | 58 | 59 | ||
49 | 59 | vbox_.add(&label_); | 60 | vbox_.add(&label_); |
50 | 60 | vbox_.add_space(padding_); | 61 | vbox_.add_space(padding_); |
51 | @@ -66,7 +67,7 @@ | |||
52 | 66 | ok_button_.sigclicked.connect( | 67 | ok_button_.sigclicked.connect( |
53 | 67 | boost::bind(&MainMenuSaveMapMakeDirectory::end_modal<UI::Panel::Returncodes>, | 68 | boost::bind(&MainMenuSaveMapMakeDirectory::end_modal<UI::Panel::Returncodes>, |
54 | 68 | boost::ref(*this), UI::Panel::Returncodes::kOk)); | 69 | boost::ref(*this), UI::Panel::Returncodes::kOk)); |
56 | 69 | ok_button_.set_enabled(false); | 70 | ok_button_.set_enabled(!dirname_.empty()); |
57 | 70 | cancel_button_.sigclicked.connect( | 71 | cancel_button_.sigclicked.connect( |
58 | 71 | boost::bind(&MainMenuSaveMapMakeDirectory::end_modal<UI::Panel::Returncodes>, | 72 | boost::bind(&MainMenuSaveMapMakeDirectory::end_modal<UI::Panel::Returncodes>, |
59 | 72 | boost::ref(*this), UI::Panel::Returncodes::kBack)); | 73 | boost::ref(*this), UI::Panel::Returncodes::kBack)); |
60 | @@ -80,6 +81,8 @@ | |||
61 | 80 | void MainMenuSaveMapMakeDirectory::edit_changed() { | 81 | void MainMenuSaveMapMakeDirectory::edit_changed() { |
62 | 81 | const std::string& text = edit_.text(); | 82 | const std::string& text = edit_.text(); |
63 | 82 | // Prevent the user from creating nonsense directory names, like e.g. ".." or "...". | 83 | // Prevent the user from creating nonsense directory names, like e.g. ".." or "...". |
65 | 83 | ok_button_.set_enabled(LayeredFileSystem::is_legal_filename(text)); | 84 | const bool is_legal_filename = FileSystem::is_legal_filename(text); |
66 | 85 | ok_button_.set_enabled(is_legal_filename); | ||
67 | 86 | edit_.set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_); | ||
68 | 84 | dirname_ = text; | 87 | dirname_ = text; |
69 | 85 | } | 88 | } |
70 | 86 | 89 | ||
71 | === modified file 'src/editor/ui_menus/main_menu_save_map_make_directory.h' | |||
72 | --- src/editor/ui_menus/main_menu_save_map_make_directory.h 2018-04-07 16:59:00 +0000 | |||
73 | +++ src/editor/ui_menus/main_menu_save_map_make_directory.h 2018-05-31 17:12:19 +0000 | |||
74 | @@ -50,6 +50,7 @@ | |||
75 | 50 | UI::EditBox edit_; | 50 | UI::EditBox edit_; |
76 | 51 | UI::Button ok_button_; | 51 | UI::Button ok_button_; |
77 | 52 | UI::Button cancel_button_; | 52 | UI::Button cancel_button_; |
78 | 53 | const std::string illegal_filename_tooltip_; | ||
79 | 53 | void edit_changed(); | 54 | void edit_changed(); |
80 | 54 | }; | 55 | }; |
81 | 55 | 56 | ||
82 | 56 | 57 | ||
83 | === modified file 'src/graphic/text_layout.cc' | |||
84 | --- src/graphic/text_layout.cc 2018-05-05 17:10:37 +0000 | |||
85 | +++ src/graphic/text_layout.cc 2018-05-31 17:12:19 +0000 | |||
86 | @@ -141,6 +141,13 @@ | |||
87 | 141 | return f.str(); | 141 | return f.str(); |
88 | 142 | } | 142 | } |
89 | 143 | 143 | ||
90 | 144 | /// Bullet list item | ||
91 | 145 | std::string as_listitem(const std::string& txt, int ptsize, const RGBColor& clr) { | ||
92 | 146 | static boost::format f("<div width=100%%><div><p><font size=%d color=%s>•</font></p></div><div><p><space gap=6></p></div><div width=*><p><font size=%d color=%s>%s<vspace gap=6></font></p></div></div>"); | ||
93 | 147 | f % ptsize % clr.hex_value() % ptsize % clr.hex_value() % txt; | ||
94 | 148 | return f.str(); | ||
95 | 149 | } | ||
96 | 150 | |||
97 | 144 | std::string as_richtext(const std::string& txt) { | 151 | std::string as_richtext(const std::string& txt) { |
98 | 145 | static boost::format f("<rt>%s</rt>"); | 152 | static boost::format f("<rt>%s</rt>"); |
99 | 146 | f % txt; | 153 | f % txt; |
100 | 147 | 154 | ||
101 | === modified file 'src/graphic/text_layout.h' | |||
102 | --- src/graphic/text_layout.h 2018-04-07 16:59:00 +0000 | |||
103 | +++ src/graphic/text_layout.h 2018-05-31 17:12:19 +0000 | |||
104 | @@ -84,6 +84,10 @@ | |||
105 | 84 | const RGBColor& clr = UI_FONT_CLR_FG, | 84 | const RGBColor& clr = UI_FONT_CLR_FG, |
106 | 85 | UI::FontSet::Face face = UI::FontSet::Face::kSans); | 85 | UI::FontSet::Face face = UI::FontSet::Face::kSans); |
107 | 86 | 86 | ||
108 | 87 | std::string as_listitem(const std::string&, | ||
109 | 88 | int ptsize = UI_FONT_SIZE_SMALL, | ||
110 | 89 | const RGBColor& clr = UI_FONT_CLR_FG); | ||
111 | 90 | |||
112 | 87 | std::string as_richtext(const std::string&); | 91 | std::string as_richtext(const std::string&); |
113 | 88 | std::string as_tooltip(const std::string&); | 92 | std::string as_tooltip(const std::string&); |
114 | 89 | std::string as_waresinfo(const std::string&); | 93 | std::string as_waresinfo(const std::string&); |
115 | 90 | 94 | ||
116 | === modified file 'src/io/CMakeLists.txt' | |||
117 | --- src/io/CMakeLists.txt 2016-02-06 11:11:24 +0000 | |||
118 | +++ src/io/CMakeLists.txt 2018-05-31 17:12:19 +0000 | |||
119 | @@ -23,24 +23,3 @@ | |||
120 | 23 | io_filesystem | 23 | io_filesystem |
121 | 24 | io_stream | 24 | io_stream |
122 | 25 | ) | 25 | ) |
123 | 26 | |||
124 | 27 | wl_library(io_filesystem | ||
125 | 28 | SRCS | ||
126 | 29 | filesystem/disk_filesystem.cc | ||
127 | 30 | filesystem/disk_filesystem.h | ||
128 | 31 | filesystem/filesystem.cc | ||
129 | 32 | filesystem/filesystem.h | ||
130 | 33 | filesystem/filesystem_exceptions.h | ||
131 | 34 | filesystem/layered_filesystem.cc | ||
132 | 35 | filesystem/layered_filesystem.h | ||
133 | 36 | filesystem/zip_exceptions.h | ||
134 | 37 | filesystem/zip_filesystem.cc | ||
135 | 38 | filesystem/zip_filesystem.h | ||
136 | 39 | DEPENDS | ||
137 | 40 | base_exceptions | ||
138 | 41 | base_log | ||
139 | 42 | base_macros | ||
140 | 43 | io_fileread | ||
141 | 44 | io_stream | ||
142 | 45 | third_party_minizip | ||
143 | 46 | ) | ||
144 | 47 | 26 | ||
145 | === modified file 'src/io/filesystem/CMakeLists.txt' | |||
146 | --- src/io/filesystem/CMakeLists.txt 2012-02-15 21:25:34 +0000 | |||
147 | +++ src/io/filesystem/CMakeLists.txt 2018-05-31 17:12:19 +0000 | |||
148 | @@ -1,1 +1,24 @@ | |||
149 | 1 | add_subdirectory(test) | 1 | add_subdirectory(test) |
150 | 2 | |||
151 | 3 | wl_library(io_filesystem | ||
152 | 4 | SRCS | ||
153 | 5 | disk_filesystem.cc | ||
154 | 6 | disk_filesystem.h | ||
155 | 7 | filesystem.cc | ||
156 | 8 | filesystem.h | ||
157 | 9 | filesystem_exceptions.h | ||
158 | 10 | layered_filesystem.cc | ||
159 | 11 | layered_filesystem.h | ||
160 | 12 | zip_exceptions.h | ||
161 | 13 | zip_filesystem.cc | ||
162 | 14 | zip_filesystem.h | ||
163 | 15 | DEPENDS | ||
164 | 16 | base_exceptions | ||
165 | 17 | base_i18n | ||
166 | 18 | base_log | ||
167 | 19 | base_macros | ||
168 | 20 | graphic_text_layout | ||
169 | 21 | io_fileread | ||
170 | 22 | io_stream | ||
171 | 23 | third_party_minizip | ||
172 | 24 | ) | ||
173 | 2 | 25 | ||
174 | === modified file 'src/io/filesystem/filesystem.cc' | |||
175 | --- src/io/filesystem/filesystem.cc 2018-05-12 04:17:53 +0000 | |||
176 | +++ src/io/filesystem/filesystem.cc 2018-05-31 17:12:19 +0000 | |||
177 | @@ -32,6 +32,9 @@ | |||
178 | 32 | #include <string> | 32 | #include <string> |
179 | 33 | #include <vector> | 33 | #include <vector> |
180 | 34 | 34 | ||
181 | 35 | // We have to add Boost to this block to make codecheck happy | ||
182 | 36 | #include <boost/algorithm/string/predicate.hpp> | ||
183 | 37 | #include <boost/format.hpp> | ||
184 | 35 | #ifdef _WIN32 | 38 | #ifdef _WIN32 |
185 | 36 | #include <direct.h> | 39 | #include <direct.h> |
186 | 37 | #include <io.h> | 40 | #include <io.h> |
187 | @@ -43,8 +46,10 @@ | |||
188 | 43 | #include <sys/stat.h> | 46 | #include <sys/stat.h> |
189 | 44 | #include <unistd.h> | 47 | #include <unistd.h> |
190 | 45 | 48 | ||
191 | 49 | #include "base/i18n.h" | ||
192 | 46 | #include "base/log.h" | 50 | #include "base/log.h" |
193 | 47 | #include "config.h" | 51 | #include "config.h" |
194 | 52 | #include "graphic/text_layout.h" | ||
195 | 48 | #include "io/filesystem/disk_filesystem.h" | 53 | #include "io/filesystem/disk_filesystem.h" |
196 | 49 | #include "io/filesystem/layered_filesystem.h" | 54 | #include "io/filesystem/layered_filesystem.h" |
197 | 50 | #include "io/filesystem/zip_exceptions.h" | 55 | #include "io/filesystem/zip_exceptions.h" |
198 | @@ -60,6 +65,34 @@ | |||
199 | 60 | #define PATH_MAX MAX_PATH | 65 | #define PATH_MAX MAX_PATH |
200 | 61 | #endif | 66 | #endif |
201 | 62 | 67 | ||
202 | 68 | |||
203 | 69 | namespace { | ||
204 | 70 | // Characters that are allowed in filenames, but not at the beginning | ||
205 | 71 | const std::vector<std::string> illegal_filename_starting_characters{ | ||
206 | 72 | ".", | ||
207 | 73 | "-", | ||
208 | 74 | " ", // Keep the blank last | ||
209 | 75 | }; | ||
210 | 76 | |||
211 | 77 | // Characters that are disallowed anywhere in a filename | ||
212 | 78 | // No potential file separators or other potentially illegal characters | ||
213 | 79 | // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx | ||
214 | 80 | // http://www.linfo.org/file_name.html | ||
215 | 81 | // https://support.apple.com/en-us/HT202808 | ||
216 | 82 | // We can't just regex for word & digit characters here because of non-Latin scripts. | ||
217 | 83 | const std::vector<std::string> illegal_filename_characters{ | ||
218 | 84 | "<", | ||
219 | 85 | ">", | ||
220 | 86 | ":", | ||
221 | 87 | "\"", | ||
222 | 88 | "|", | ||
223 | 89 | "?", | ||
224 | 90 | "*", | ||
225 | 91 | "/", | ||
226 | 92 | "\\", | ||
227 | 93 | }; | ||
228 | 94 | } // namespace | ||
229 | 95 | |||
230 | 63 | /** | 96 | /** |
231 | 64 | * \param path A file or directory name | 97 | * \param path A file or directory name |
232 | 65 | * \return True if ref path is absolute and within this FileSystem, false otherwise | 98 | * \return True if ref path is absolute and within this FileSystem, false otherwise |
233 | @@ -148,6 +181,43 @@ | |||
234 | 148 | #endif | 181 | #endif |
235 | 149 | } | 182 | } |
236 | 150 | 183 | ||
237 | 184 | bool FileSystem::is_legal_filename(const std::string& filename) { | ||
238 | 185 | if (filename.empty()) { | ||
239 | 186 | return false; | ||
240 | 187 | } | ||
241 | 188 | for (const std::string& illegal_start : illegal_filename_starting_characters) { | ||
242 | 189 | if (boost::starts_with(filename, illegal_start)) { | ||
243 | 190 | return false; | ||
244 | 191 | } | ||
245 | 192 | } | ||
246 | 193 | for (const std::string& illegal_char : illegal_filename_characters) { | ||
247 | 194 | if (boost::contains(filename, illegal_char)) { | ||
248 | 195 | return false; | ||
249 | 196 | } | ||
250 | 197 | } | ||
251 | 198 | return true; | ||
252 | 199 | } | ||
253 | 200 | |||
254 | 201 | std::string FileSystem::illegal_filename_tooltip() { | ||
255 | 202 | std::vector<std::string> starting_characters; | ||
256 | 203 | for (const std::string& character : illegal_filename_starting_characters) { | ||
257 | 204 | if (character == " ") { | ||
258 | 205 | /** TRANSLATORS: Part of tooltip entry for characters in illegal filenames. replaces tha blank space in a list of illegal characters */ | ||
259 | 206 | starting_characters.push_back(pgettext("illegal_filename_characters", "blank space")); | ||
260 | 207 | } else { | ||
261 | 208 | starting_characters.push_back(character); | ||
262 | 209 | } | ||
263 | 210 | } | ||
264 | 211 | /** TRANSLATORS: Tooltip entry for characters in illegal filenames. %s is a list of illegal characters */ | ||
265 | 212 | const std::string illegal_start(as_listitem((boost::format(pgettext("illegal_filename_characters", "%s at the start of the filename")) % richtext_escape(i18n::localize_list(starting_characters, i18n::ConcatenateWith::OR))).str(), UI_FONT_SIZE_MESSAGE)); | ||
266 | 213 | |||
267 | 214 | /** TRANSLATORS: Tooltip entry for characters in illegal filenames. %s is a list of illegal characters */ | ||
268 | 215 | const std::string illegal(as_listitem((boost::format(pgettext("illegal_filename_characters", "%s anywhere in the filename")) % richtext_escape(i18n::localize_list(illegal_filename_characters, i18n::ConcatenateWith::OR))).str(), UI_FONT_SIZE_MESSAGE)); | ||
269 | 216 | |||
270 | 217 | /** TRANSLATORS: Tooltip header for characters in illegal filenames. This is followed by a list of bullet points */ | ||
271 | 218 | return (boost::format("%s%s%s") % pgettext("illegal_filename_characters", "The following characters are not allowed:") % illegal_start % illegal).str(); | ||
272 | 219 | } | ||
273 | 220 | |||
274 | 151 | // TODO(unknown): Write homedir detection for non-getenv-systems | 221 | // TODO(unknown): Write homedir detection for non-getenv-systems |
275 | 152 | std::string FileSystem::get_homedir() { | 222 | std::string FileSystem::get_homedir() { |
276 | 153 | std::string homedir; | 223 | std::string homedir; |
277 | 154 | 224 | ||
278 | === modified file 'src/io/filesystem/filesystem.h' | |||
279 | --- src/io/filesystem/filesystem.h 2018-04-07 16:59:00 +0000 | |||
280 | +++ src/io/filesystem/filesystem.h 2018-05-31 17:12:19 +0000 | |||
281 | @@ -108,6 +108,10 @@ | |||
282 | 108 | std::string canonicalize_name(std::string path) const; | 108 | std::string canonicalize_name(std::string path) const; |
283 | 109 | bool is_path_absolute(const std::string& path) const; | 109 | bool is_path_absolute(const std::string& path) const; |
284 | 110 | 110 | ||
285 | 111 | /// Returns true if the filename is legal in all operating systems | ||
286 | 112 | static bool is_legal_filename(const std::string& filename); | ||
287 | 113 | static std::string illegal_filename_tooltip(); | ||
288 | 114 | |||
289 | 111 | // Returns the path separator, i.e. \ on windows and / everywhere else. | 115 | // Returns the path separator, i.e. \ on windows and / everywhere else. |
290 | 112 | static char file_separator(); | 116 | static char file_separator(); |
291 | 113 | 117 | ||
292 | 114 | 118 | ||
293 | === modified file 'src/io/filesystem/layered_filesystem.cc' | |||
294 | --- src/io/filesystem/layered_filesystem.cc 2018-04-07 16:59:00 +0000 | |||
295 | +++ src/io/filesystem/layered_filesystem.cc 2018-05-31 17:12:19 +0000 | |||
296 | @@ -22,16 +22,12 @@ | |||
297 | 22 | #include <cstdio> | 22 | #include <cstdio> |
298 | 23 | #include <memory> | 23 | #include <memory> |
299 | 24 | 24 | ||
300 | 25 | #include <boost/algorithm/string/predicate.hpp> | ||
301 | 26 | #include <boost/regex.hpp> | ||
302 | 27 | |||
303 | 28 | #include "base/log.h" | 25 | #include "base/log.h" |
304 | 29 | #include "base/wexception.h" | 26 | #include "base/wexception.h" |
305 | 30 | #include "io/fileread.h" | 27 | #include "io/fileread.h" |
306 | 31 | #include "io/streamread.h" | 28 | #include "io/streamread.h" |
307 | 32 | 29 | ||
308 | 33 | LayeredFileSystem* g_fs; | 30 | LayeredFileSystem* g_fs; |
309 | 34 | |||
310 | 35 | LayeredFileSystem::LayeredFileSystem() : home_(nullptr) { | 31 | LayeredFileSystem::LayeredFileSystem() : home_(nullptr) { |
311 | 36 | } | 32 | } |
312 | 37 | 33 | ||
313 | @@ -41,18 +37,6 @@ | |||
314 | 41 | LayeredFileSystem::~LayeredFileSystem() { | 37 | LayeredFileSystem::~LayeredFileSystem() { |
315 | 42 | } | 38 | } |
316 | 43 | 39 | ||
317 | 44 | bool LayeredFileSystem::is_legal_filename(const std::string& filename) { | ||
318 | 45 | // No potential file separators or other potentially illegal characters | ||
319 | 46 | // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx | ||
320 | 47 | // http://www.linfo.org/file_name.html | ||
321 | 48 | // https://support.apple.com/en-us/HT202808 | ||
322 | 49 | // We can't just regex for word & digit characters here because of non-Latin scripts. | ||
323 | 50 | boost::regex re(".*[<>:\"|?*/\\\\].*"); | ||
324 | 51 | return !filename.empty() && !boost::starts_with(filename, ".") && | ||
325 | 52 | !boost::starts_with(filename, " ") && !boost::starts_with(filename, "-") && | ||
326 | 53 | !boost::regex_match(filename, re); | ||
327 | 54 | } | ||
328 | 55 | |||
329 | 56 | /** | 40 | /** |
330 | 57 | * Just assume that at least one of our child FSs is writable | 41 | * Just assume that at least one of our child FSs is writable |
331 | 58 | */ | 42 | */ |
332 | 59 | 43 | ||
333 | === modified file 'src/io/filesystem/layered_filesystem.h' | |||
334 | --- src/io/filesystem/layered_filesystem.h 2018-04-07 16:59:00 +0000 | |||
335 | +++ src/io/filesystem/layered_filesystem.h 2018-05-31 17:12:19 +0000 | |||
336 | @@ -58,8 +58,7 @@ | |||
337 | 58 | 58 | ||
338 | 59 | std::set<std::string> list_directory(const std::string& path) override; | 59 | std::set<std::string> list_directory(const std::string& path) override; |
339 | 60 | 60 | ||
342 | 61 | /// Returns true if the filename is legal in all operating systems | 61 | |
341 | 62 | static bool is_legal_filename(const std::string& filename); | ||
343 | 63 | bool is_writable() const override; | 62 | bool is_writable() const override; |
344 | 64 | bool file_exists(const std::string& path) override; | 63 | bool file_exists(const std::string& path) override; |
345 | 65 | bool is_directory(const std::string& path) override; | 64 | bool is_directory(const std::string& path) override; |
346 | 66 | 65 | ||
347 | === modified file 'src/ui_basic/panel.cc' | |||
348 | --- src/ui_basic/panel.cc 2018-05-14 06:59:39 +0000 | |||
349 | +++ src/ui_basic/panel.cc 2018-05-31 17:12:19 +0000 | |||
350 | @@ -143,8 +143,9 @@ | |||
351 | 143 | app->set_mouse_lock(false); // more paranoia :-) | 143 | app->set_mouse_lock(false); // more paranoia :-) |
352 | 144 | 144 | ||
353 | 145 | Panel* forefather = this; | 145 | Panel* forefather = this; |
356 | 146 | while (Panel* const p = forefather->parent_) | 146 | while (forefather->parent_ != nullptr) { |
357 | 147 | forefather = p; | 147 | forefather = forefather->parent_; |
358 | 148 | } | ||
359 | 148 | 149 | ||
360 | 149 | default_cursor_ = g_gr->images().get("images/ui_basic/cursor.png"); | 150 | default_cursor_ = g_gr->images().get("images/ui_basic/cursor.png"); |
361 | 150 | default_cursor_click_ = g_gr->images().get("images/ui_basic/cursor_click.png"); | 151 | default_cursor_click_ = g_gr->images().get("images/ui_basic/cursor_click.png"); |
362 | @@ -175,13 +176,16 @@ | |||
363 | 175 | app->handle_input(&input_callback); | 176 | app->handle_input(&input_callback); |
364 | 176 | 177 | ||
365 | 177 | if (start_time >= next_think_time) { | 178 | if (start_time >= next_think_time) { |
367 | 178 | if (app->should_die()) | 179 | if (app->should_die()) { |
368 | 179 | end_modal<Returncodes>(Returncodes::kBack); | 180 | end_modal<Returncodes>(Returncodes::kBack); |
369 | 181 | } | ||
370 | 180 | 182 | ||
371 | 181 | do_think(); | 183 | do_think(); |
372 | 182 | 184 | ||
374 | 183 | if (flags_ & pf_child_die) | 185 | if (flags_ & pf_child_die) { |
375 | 184 | check_child_death(); | 186 | check_child_death(); |
376 | 187 | } | ||
377 | 188 | |||
378 | 185 | next_think_time = start_time + kGameLogicDelay; | 189 | next_think_time = start_time + kGameLogicDelay; |
379 | 186 | } | 190 | } |
380 | 187 | 191 | ||
381 | @@ -190,7 +194,13 @@ | |||
382 | 190 | forefather->do_draw(rt); | 194 | forefather->do_draw(rt); |
383 | 191 | rt.blit((app->get_mouse_position() - Vector2i(3, 7)), | 195 | rt.blit((app->get_mouse_position() - Vector2i(3, 7)), |
384 | 192 | app->is_mouse_pressed() ? default_cursor_click_ : default_cursor_); | 196 | app->is_mouse_pressed() ? default_cursor_click_ : default_cursor_); |
386 | 193 | forefather->do_tooltip(); | 197 | |
387 | 198 | if (is_modal()) { | ||
388 | 199 | do_tooltip(); | ||
389 | 200 | } else { | ||
390 | 201 | forefather->do_tooltip(); | ||
391 | 202 | } | ||
392 | 203 | |||
393 | 194 | g_gr->refresh(); | 204 | g_gr->refresh(); |
394 | 195 | next_draw_time = start_time + draw_delay; | 205 | next_draw_time = start_time + draw_delay; |
395 | 196 | } | 206 | } |
396 | @@ -594,8 +604,7 @@ | |||
397 | 594 | * false otherwise. | 604 | * false otherwise. |
398 | 595 | */ | 605 | */ |
399 | 596 | bool Panel::handle_tooltip() { | 606 | bool Panel::handle_tooltip() { |
402 | 597 | RenderTarget& rt = *g_gr->get_render_target(); | 607 | return draw_tooltip(tooltip()); |
401 | 598 | return draw_tooltip(rt, tooltip()); | ||
403 | 599 | } | 608 | } |
404 | 600 | 609 | ||
405 | 601 | /** | 610 | /** |
406 | @@ -1060,13 +1069,15 @@ | |||
407 | 1060 | /** | 1069 | /** |
408 | 1061 | * Draw the tooltip. Return true on success | 1070 | * Draw the tooltip. Return true on success |
409 | 1062 | */ | 1071 | */ |
411 | 1063 | bool Panel::draw_tooltip(RenderTarget& dst, const std::string& text) { | 1072 | bool Panel::draw_tooltip(const std::string& text) { |
412 | 1064 | if (text.empty()) { | 1073 | if (text.empty()) { |
413 | 1065 | return false; | 1074 | return false; |
414 | 1066 | } | 1075 | } |
415 | 1076 | |||
416 | 1077 | RenderTarget& dst = *g_gr->get_render_target(); | ||
417 | 1067 | std::string text_to_render = text; | 1078 | std::string text_to_render = text; |
418 | 1068 | if (!is_richtext(text_to_render)) { | 1079 | if (!is_richtext(text_to_render)) { |
420 | 1069 | text_to_render = as_tooltip(text); | 1080 | text_to_render = as_tooltip(text_to_render); |
421 | 1070 | } | 1081 | } |
422 | 1071 | 1082 | ||
423 | 1072 | constexpr uint32_t kTipWidthMax = 360; | 1083 | constexpr uint32_t kTipWidthMax = 360; |
424 | 1073 | 1084 | ||
425 | === modified file 'src/ui_basic/panel.h' | |||
426 | --- src/ui_basic/panel.h 2018-04-27 06:11:05 +0000 | |||
427 | +++ src/ui_basic/panel.h 2018-05-31 17:12:19 +0000 | |||
428 | @@ -311,7 +311,7 @@ | |||
429 | 311 | static void play_new_chat_member(); | 311 | static void play_new_chat_member(); |
430 | 312 | static void play_new_chat_message(); | 312 | static void play_new_chat_message(); |
431 | 313 | 313 | ||
433 | 314 | static bool draw_tooltip(RenderTarget&, const std::string& text); | 314 | static bool draw_tooltip(const std::string& text); |
434 | 315 | void draw_background(RenderTarget& dst, const UI::PanelStyleInfo&); | 315 | void draw_background(RenderTarget& dst, const UI::PanelStyleInfo&); |
435 | 316 | void draw_background(RenderTarget& dst, Recti rect, const UI::PanelStyleInfo&); | 316 | void draw_background(RenderTarget& dst, Recti rect, const UI::PanelStyleInfo&); |
436 | 317 | 317 | ||
437 | 318 | 318 | ||
438 | === modified file 'src/wui/game_main_menu_save_game.cc' | |||
439 | --- src/wui/game_main_menu_save_game.cc 2018-05-22 06:54:12 +0000 | |||
440 | +++ src/wui/game_main_menu_save_game.cc 2018-05-31 17:12:19 +0000 | |||
441 | @@ -65,7 +65,8 @@ | |||
442 | 65 | cancel_(&buttons_box_, "cancel", 0, 0, 0, 0, UI::ButtonStyle::kWuiSecondary, _("Cancel")), | 65 | cancel_(&buttons_box_, "cancel", 0, 0, 0, 0, UI::ButtonStyle::kWuiSecondary, _("Cancel")), |
443 | 66 | ok_(&buttons_box_, "ok", 0, 0, 0, 0, UI::ButtonStyle::kWuiPrimary, _("OK")), | 66 | ok_(&buttons_box_, "ok", 0, 0, 0, 0, UI::ButtonStyle::kWuiPrimary, _("OK")), |
444 | 67 | 67 | ||
446 | 68 | curdir_(kSaveDir) { | 68 | curdir_(kSaveDir), |
447 | 69 | illegal_filename_tooltip_(FileSystem::illegal_filename_tooltip()) { | ||
448 | 69 | 70 | ||
449 | 70 | layout(); | 71 | layout(); |
450 | 71 | 72 | ||
451 | @@ -133,8 +134,9 @@ | |||
452 | 133 | 134 | ||
453 | 134 | void GameMainMenuSaveGame::edit_box_changed() { | 135 | void GameMainMenuSaveGame::edit_box_changed() { |
454 | 135 | // Prevent the user from creating nonsense directory names, like e.g. ".." or "...". | 136 | // Prevent the user from creating nonsense directory names, like e.g. ".." or "...". |
456 | 136 | const bool is_legal_filename = LayeredFileSystem::is_legal_filename(filename_editbox_.text()); | 137 | const bool is_legal_filename = FileSystem::is_legal_filename(filename_editbox_.text()); |
457 | 137 | ok_.set_enabled(is_legal_filename); | 138 | ok_.set_enabled(is_legal_filename); |
458 | 139 | filename_editbox_.set_tooltip(is_legal_filename ? "" : illegal_filename_tooltip_); | ||
459 | 138 | load_or_save_.delete_button()->set_enabled(false); | 140 | load_or_save_.delete_button()->set_enabled(false); |
460 | 139 | load_or_save_.clear_selections(); | 141 | load_or_save_.clear_selections(); |
461 | 140 | } | 142 | } |
462 | 141 | 143 | ||
463 | === modified file 'src/wui/game_main_menu_save_game.h' | |||
464 | --- src/wui/game_main_menu_save_game.h 2018-05-10 15:57:43 +0000 | |||
465 | +++ src/wui/game_main_menu_save_game.h 2018-05-31 17:12:19 +0000 | |||
466 | @@ -79,6 +79,7 @@ | |||
467 | 79 | UI::Button cancel_, ok_; | 79 | UI::Button cancel_, ok_; |
468 | 80 | 80 | ||
469 | 81 | std::string curdir_; | 81 | std::string curdir_; |
470 | 82 | const std::string illegal_filename_tooltip_; | ||
471 | 82 | }; | 83 | }; |
472 | 83 | 84 | ||
473 | 84 | #endif // end of include guard: WL_WUI_GAME_MAIN_MENU_SAVE_GAME_H | 85 | #endif // end of include guard: WL_WUI_GAME_MAIN_MENU_SAVE_GAME_H |
Continuous integration builds have changed state:
Travis build 3534. State: failed. Details: https:/ /travis- ci.org/ widelands/ widelands/ builds/ 382014340. /ci.appveyor. com/project/ widelands- dev/widelands/ build/_ widelands_ dev_widelands_ bug_1610507_ disallowed_ filename_ characters- 3338.
Appveyor build 3338. State: success. Details: https:/