Clean up export filenames for Excel compatibility #22969
Conversation
mneudert
added
Bug
|
Tested the new function locally, can confirm that the new function behaves as expected. Was unable to replicate the wider issue on my Windows machine, my version of excel seems to handle these filenames correctly. |
| ["control\x00\x09\x0A\x7Fcharacters", 'controlcharacters'], | ||
| [' spaces are trimmed ', 'spaces are trimmed'], | ||
| ['unicode spaces', 'unicode spaces'], | ||
| ['unicode‒–—dashes', 'unicode---dashes'], |
There was a problem hiding this comment.
Do we need a couple more tests, such as a legal file name that isn't changed, and other special characters such as multi-byte unicode characters, like Chinese?
There was a problem hiding this comment.
Added some examples generated from the report export feature. Let me know if there are any more variants/special characters we should add.
core/Filesystem.php
Outdated
|
|
||
| // https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file | ||
| $filename = preg_replace('~[<>:"/\\\\|?*]~', '', $filename); | ||
| $filename = preg_replace('~\p{Cc}~u', '', $filename); |
There was a problem hiding this comment.
Perhaps another comment explaining what these Regex statements mean, more than just a link?
There was a problem hiding this comment.
More links and comments added 👍
Description:
The current (CSV) exports can contain unicode characters like
&thsp;("thin space"), preventing them from being openend in some programs like Excel.A replacement of "special dashes" and "special whitespace" with their ASCII representation should fix that problem. Additionaly some other reserved characters like
<>are now being removed from both report exports and scheduled report email attachment filenames.Refs DEV-18832
Review