User tests: Successful: Unsuccessful:
Currently when uploading a file through the Media Manager with a space in its name an error is thrown. All other unwanted chars are already removed by JFile::makeSafe() but this method explicitly allows spaces.
Errors tend to confuse inexperienced users, so this PR ensures that spaces are replaced with dashes instead of causing a failed upload.
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Media Manager |
I have tested this item successfully on a7709ed
Able to reproduce then #9608 works as described - Thanks
Status | Pending | ⇒ | Ready to Commit |
Maybe we need to update the error message as well
COM_MEDIA_ERROR_WARNFILENAME="File name must only contain alphanumeric characters and no spaces."
Labels |
Added:
?
|
The error message is triggered by JHelperMedia::canUpload() so I think the message would still be accurate if someone used that method directly without sanitizing the filename first.
This reminds me this issue #7841 ( @brianteeman still open but could be closed as issue seems already fixed, even if i don't know when, and in which PR...) and my comment about spaces in file upload : #7841 (comment)
So i agree to accept space on upload , but i prefer dash -
as replacement.
In Google’s search engine algorithm, the underscore isn’t a recognized word separator.
For example:
my-new-black-kitten.jpg will be seen as 4 (key)words my
new
black
kitten
my_new_black_kitten.jpg will be seen as 1 word mynewblackkitten
Another link with simple and clear explanation : http://searchengineland.com/9-seo-quirks-you-should-be-aware-of-146465
So... dash or underscore ?
I actually prefer the dash myself but went with underscore as I (mistakenly) thought that was the more accepted word separator. Happy to change it.
I thought it was an underscore as well
After many search since months, underscore seems to be used when it's an icon, a design element... in fact when you don't want and need to list this image in search engines.
Unfortunately, nothing definitely stated as a norm.
As image is included in a url (<img src="url-to-image"/>
, or <a href="url-to-image">
, metadata img
...),
i am personally for a url safe output...
But this would mean to replace this:
$file['name'] = JFile::makeSafe($file['name']);
By this (the first 3 lines. Last ones are what i use in my own extension for empty sanitized $filename (eg. Asian characters filename)):
$filename = JFilterOutput::stringURLSafe(JFile::stripExt($file['name']));
$ext = strtolower(JFile::getExt($file['name']));
$file['name'] = $filename . '.' . $ext;
// If filename empty (for example, if you have a filename like this 代替品.jpg, it is currently returning the same error as with spaces in current J version), filename based on current date/time
if ( ! $filename)
{
$file['name'] = JFactory::getDate()->format("YmdHis") . '.' . $ext;
}
This will return a lowcase dash separated string, the same way alias are generated.
For example, Spread the Joomla! Love.JPG will be spread-the-joomla-love.jpg
(today, with this PR, it would be Spread_the_Joomla_Love.JPG
, not the best url SEO-friendly filename...)
More discussion here (and not having a clear position from all...): #7841 (comment)
In fact, a decision from PLT and translators, would help to decide the good way to go!
Why are underscore not URL safe?
On 27 Mar 2016 2:26 pm, "Cyril Rezé" notifications@github.com wrote:
After many search since months, underscore seems to be used when it's an
icon, a design element... in fact when you don't want and need to list this
image in search engines.
Unfortunately, nothing definitely stated as a norm.As image is included in a url, i am personally for a url safe output...
But this would mean to replace this:
$file['name'] = JFile::makeSafe($file['name']);By this (the first 3 lines. Last ones are what i use in my own extension
for empty sanitized $filename (eg. Asian characters filename)):$filename = JFilterOutput::stringURLSafe(JFile::stripExt($file['name']));
$ext = strtolower(JFile::getExt($file['name']));
$file['name'] = $filename . '.' . $ext;// If filename empty (for example, if you have a filename like this 代替品.jpg, it is currently returning the same error as with spaces in current J version), filename based on current date/time
if ( ! $filename)
{
$file['name'] = JFactory::getDate()->format("YmdHis") . '.' . $ext;
}This will return a lowcase dash separated string, the same way alias are
generated.
For example, Spread the Joomla! Love.JPG will be
spread-the-joomla-love.jpg
(today, with this PR, it would be Spread_the_Joomla_Love.JPG, not the
best url safe filename...)More discussion here (and not having a clear position from all...): #7841
(comment)
#7841 (comment)In fact, a decision from PLT and translators, would help to decide the
good way to go!—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9608 (comment)
@brianteeman they are safe, but not SEO friendly.
@brianteeman direct link to video of Matt Cutts, in case you didn't watch it already in one of the previous links i posted ;-)
https://www.youtube.com/watch?v=AQcSFsQyct8
I was only querying this statement of yours
(today, with this PR, it would be Spread_the_Joomla_Love.JPG, not the
best url safe filename...)
On 27 March 2016 at 15:23, Cyril Rezé notifications@github.com wrote:
@brianteeman https://github.com/brianteeman direct link to video of
Matt Cutts, in case you didn't watch it already in one of the previous
links i posted ;-)
https://www.youtube.com/watch?v=AQcSFsQyct8—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9608 (comment)
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
@brianteeman Yes, an error, and i understood after your comment why, and so i just edited my message ;-)
@JoomliC The additional sanitation you suggest is probably beyond the scope of this PR. I suggest opening a new one referencing issue #7841.
I will note that filenames based entirely on a datetime could be problematic, as one can upload multiple files at the same time.
From the links you have provided it's evident that dashes are the way to go and I'll update the PR to reflect that.
This PR has received new commits.
CC: @brianteeman, @MATsxm
I will note that filenames based entirely on a datetime could be problematic, as one can upload multiple files at the same time.
I understand, but is there a multilple upload function on Joomla media component today ?...
And the same will apply if multiple uploads of files named for example joomla.jpg ;-)
Note: in this case, an auto-increment if file exists is not difficult to add for media manager too, as the same process as for alias.
In all cases, this is not to be included until feedback from translators.
I mention PR #7841, as i think "Why not to have norms that will suit all languages, in one shot!"...
Ok, so it seems that we have to process step by step...
About dash to replace space:
I understand, but is there a multilple upload function on Joomla media component today ?...
Indeed, the Media Manager already allow you to select multiple files to upload in the same request.
@infograf768 this is exactly the question i had just above... The fact is in 3.5, no more issue, but #7841 is still open... and i don't find where, and when this was fixed... :-|
@ralain oh! you're right, didn't notice it before
Will test this PR, as it is already an improvement for users, and a more advanced solution is not for tomorrow ;-)
I have tested this item successfully on 11c1b75
Milestone |
Added: |
Another question is whether spaces make sense at all in file names on the server? IMO it would be better to change the behavior globally in the makeSafe function but this could lead to a b/c break...
I agree with @JoomliC, thank you for the examples. In one of my plugins I also use underscores for the replacement of whitespaces but will change it to dashes in the next version. Additionally I also make the string lowercase.
We need another test, then it can go into the 3.5.2 version.
Milestone |
Added: |
Milestone |
Removed: |
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-29 11:07:42 |
Closed_By | ⇒ | Kubik-Rubik |
Labels |
Removed:
?
|
Another question is whether spaces make sense at all in file names on the server?
Of course, no space on server! ;-)
But users mainly have spaces in their images, pdf... on their computer. That's where this PR is really useful for a better UX.
IMO it would be better to change the behavior globally in the makeSafe function but this could lead to a b/c break...
I would better think of a new function (globalizeFilenameOnUpload...), for file renaming on upload only, which could be used for com_media, and taking care of all possible filename depending of culture. A globalized naming.
On Asian sites i've visited, i mainly found numerics as the file name, often something like this : 20160329-000010.jpg (where the last 6 digits seems to be an increment).
But i'm not Asian, so i wonder if there are norms or recommendations for such languages...
Thanks @ralain
@Kubik-Rubik No problem, will do.
infograf768 this is exactly the question i had just above... The fact is in 3.5, no more issue, but #7841 is still open... and i don't find where, and when this was fixed... :-|
@JoomliC I just tested it in J3.6.2 and it's not fixed at all. Accented characters still get stripped entirely and filenames only containing accented characters result in the "filetype not supported" error thrown.
@ZoFx Can you please try my plugin "EIR" (https://joomla-extensions.kubik-rubik.de/eir-easy-image-resizer) with the option "Safe Upload Names" enabled and check whether these images are uploaded with a readable image name?
@ZoFx when testing since 3.5 with åäö.jpg, this one works, and file is renamed aao.jpg (just tested again on Joomla 3.6.2, and still works for me).
That's why i supposed this fixed, but didn't see where the change was applied...
Tested with åäöéèû.jpg and it returns aaoeeu.jpg, and file successfully uploaded.
(note: tested in com_media by using the upload button)
What does not work yet is a filename like this : 代替品.jpg
This why i proposed this in comment : #9608 (comment) (i can do a PR for asian, arabic ... characters to generate a datetime-based filename if accepted as a possible workaround ? IMO to be solved for all people from those cultures and languages!)
A similar discussion is going on in #7841 ...
@JoomliC maybe I'm completely barking up the wrong tree, but when I'm uploading files through media manager, there's no transliteration at all and åäö.jpg is refused ("filetype not supported"). I have everything updated to the latest stable (J3.6.2, com_media 3.0.0). If it matters (it proably does not ...) I'm using FF48.0.2 on Win 10. Either you or I seem to have installed an extension which influences com_media upload. I might need to test it on a complete clean install ...
@Kubik-Rubik with EIR installed in version 3.4.2 it works as described by JoomliC. åäö.jpg gets uploaded and renamed to aaeoe.jpg.
@ZoFx commented here #7841 (comment) to keep messages on the right place ;-)
I have tested this item successfully on a7709ed
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9608.