? Success

User tests: Successful: Unsuccessful:

avatar roland-d roland-d - open - 2 Jun 2014
avatar hen345
hen345 - comment - 11 Jul 2014

@test
create map: 'Joomla\images\test test'
successful map 'test test' deleted in 'Media Manager'

avatar GraafG
GraafG - comment - 11 Jul 2014

@Test

Pre update:
Can delete folders with spaces. (reproduced)

Post update:
Cannot delete folders with spaces. (test ok)

Comment: does this eror help the user?
(Unable to delete: . Directory name must only contain alphanumeric characters and no spaces.)
The folder has to be removed using ftp/file manager/ssh but error does not inform user on the how.

avatar infograf768
infograf768 - comment - 20 Jul 2014

Not sure I understand here.
I think this PR is fine, but why do we prevent com_media from deleting stuff with spaces?
We should let com_media delete stuff with spaces and prevent indeed uploading anything with spaces imho.

This would mean also taking off the code in file.php and folder.php of type:

                if ($path !== JFile::makeSafe($path))
                {
                    $dirname = htmlspecialchars($path, ENT_COMPAT, 'UTF-8');
                    JError::raiseWarning(100, JText::sprintf('COM_MEDIA_ERROR_UNABLE_TO_DELETE_FOLDER_WARNDIRNAME', substr($dirname, strlen(COM_MEDIA_BASE))));
                    continue;
                }
avatar Bakual
Bakual - comment - 20 Jul 2014

I agree it would make most sense to allow managing/renaming/deleting files and folders with invalid names. But restrict the creation (or renaming to) of those folders/files.
That would be perfect.

avatar infograf768
infograf768 - comment - 21 Jul 2014

@roland-d
Could you update your PR by taking off the code I quoted above in the 2 files concerned?

avatar roland-d roland-d - change - 21 Jul 2014
Title
JFile::makeSafe() allows spaces but com_media assumes it doesn't
[#31549] JFile::makeSafe() allows spaces but com_media assumes it doesn't
avatar roland-d roland-d - change - 21 Jul 2014
Title
JFile::makeSafe() allows spaces but com_media assumes it doesn't
[#31549] JFile::makeSafe() allows spaces but com_media assumes it doesn't
avatar roland-d
roland-d - comment - 21 Jul 2014

@infograf768 PR has been updated. You also asked why to prevent deletion of stuff with spaces, I just didn't know this code was there :)

avatar infograf768
infograf768 - comment - 21 Jul 2014

Now, test again. You will see that modifying the regex for JFile::makeSafe does not check if files are safe anymore.
What happens in the case of com_media is that the space contained in the file name will just be deleted instead of getting an alert.
For example a file "myfile 22.jpg" will be uploaded as "myfile22.jpg"

That I guess is OK for com_media, but maybe not in Joomla generally.
We have to check this before merging.

avatar roland-d
roland-d - comment - 21 Jul 2014

Not sure what to do with this. Probably is better to close this thing until there is some decision on what needs to be done. A discussion on the mailing list did not reach any conclusion either (https://groups.google.com/forum/#!topic/joomla-dev-cms/l-5rNCOTudE).

My intention was only to create a PR based against staging not to pull this topic. As my personal opinion is that all non-supported characters should be changed to underscores.

avatar roland-d
roland-d - comment - 26 Jul 2014

As discussed at the Bug Cleanup Sprint there are 2 issues here and we concluded:

  1. Spaces are not going to be allowed to upload

  2. Deleting images/folders with spaces should be possible.

As this is current behavior we only need to update the language string.

avatar Bakual Bakual - change - 27 Jul 2014
Status New Closed
Closed_Date 0000-00-00 00:00:00 2014-07-27 17:03:51
avatar Bakual Bakual - close - 27 Jul 2014
avatar Bakual Bakual - close - 27 Jul 2014
avatar Sophist-UK Sophist-UK - reference | 47c0d1c - 7 Oct 14
avatar roland-d roland-d - head_ref_deleted - 6 May 2015

Add a Comment

Login with GitHub to post a comment