This module needs to transliterate the actual file name (i.e., the URI to the downloaded file) in order to function correctly, but it shouldn't need to transliterate the human-readable name (i.e., what Drupal stores in the $file->filename property) too.

This can especially cause an issue with modules like File Entity and Media, where the human readable name is displayed in various places and users might have a good reason to want the original uploaded file name (with whatever special characters it has in it) to display in those locations.

CommentFileSizeAuthor
#5 transliteration-human-readable-filename-2034407-5.patch2.56 KBDavid_Rothstein
#1 transliteration-human-readable-filename-2034407-1.patch1017 bytesDavid_Rothstein

Comments

David_Rothstein’s picture

David_Rothstein commented
Status: Active » Needs review
StatusFileSize
new transliteration-human-readable-filename-2034407-1.patch1017 bytes

Here is a patch that fixes the issue.

I guess it could be argued this behavior should be controlled by an option in the admin UI, but I think not touching the human-readable name is what's expected so this patch just fixes it always.

amateescu’s picture

amateescu commented
Status: Needs review » Active

I guess we need to turn this into an optional behavior. Patches welcome :)

David_Rothstein’s picture

David_Rothstein commented
Status: Active » Needs review

Wow, that was a quick crosspost! (And yes, looks like we had the same idea about turning it into an optional behavior.)

amateescu’s picture

amateescu commented
Status: Needs review » Needs work

Heh, that was funny :) So, while this may be the expected behavior, the current status quo is that it is happening, and existing sites might rely on it. So I think we need the checkbox in this patch, defaulting to the value of 'transliteration_file_uploads'.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs work » Needs review
StatusFileSize
new transliteration-human-readable-filename-2034407-5.patch2.56 KB

OK, here is a reroll that adds a checkbox.

I thought it would make sense to add #states to hide this checkbox when it's not relevant, but afterwards realized that the checkbox underneath it is in the same situation. So rather than undo what I did, I just added #states in both places.

amateescu’s picture

amateescu commented
Version: 7.x-3.x-dev » 6.x-3.x-dev
Status: Needs review » Patch (to be ported) 
+++ b/transliteration.module
@@ -171,6 +187,23 @@ function transliteration_init() {
+  if (!empty($_FILES['files']['name']) && variable_get('transliteration_file_uploads', TRUE) && !variable_get('transliteration_file_uploads_display_name', TRUE)) {

Hm.. so you bypassed defaulting to the value of 'transliteration_file_uploads' by checking it too. I guess that's ok :)

Committed to 7.x-3.x and moving to 6.x-3.x for backport.

http://drupalcode.org/project/transliteration.git/commit/a4aba37

David_Rothstein’s picture

David_Rothstein commented

Thanks for the incredibly fast review and commit! Yeah, it seemed simpler to me to just keep the two variables totally separate - forgot to mention it when I posted the patch though.

amateescu’s picture

amateescu commented

Taking a small break from core does wonders for contrib :P Thank you as well for the patch.

mtoscano’s picture

mtoscano commented
Issue summary: View changes

HI,
I don't see any difference when I uncheck this option.
I am using File Entity and Media with this patch and with both options (checked/unchecked) the file name is transliterated during upload. By contrast, it is not transliterated with both options if the name is edited after upload.
If I remove the patch the file name cannot be set during upload and user need to re-edit it to set a proper name instead of the file name (transliterated or not).
Am I missing something?

mtoscano’s picture

mtoscano commented

Hi,
I find out something interesting. If files are uploaded to the Media root folder their file names are not transliterated with this option unchecked, so it works as expected.
Instead, if files are uploaded to a different nested folder the file name it is always transliterated.
Folders (taxonomy terms that can create also physical folders on the server) are added to the Media module by the Media Browser Plus module.
Is it possible to adjust this patch in order to works with files upload also in nested folders?
Thanks

nixar’s picture

nixar commented

I think there is a problem with the patch provided in #5 because the code does the opposite of what the new variable is expected to do.

If you check the box, you decide to transliterate the filename, if you don't check it, you leave it as is but in the code, the checking is done against the opposite value:

  if (!empty($_FILES['files']['name']) && variable_get('transliteration_file_uploads', TRUE) && !variable_get('transliteration_file_uploads_display_name', TRUE)) {

so instead shouldn't it be

  if (!empty($_FILES['files']['name']) && variable_get('transliteration_file_uploads', TRUE) && variable_get('transliteration_file_uploads_display_name', TRUE)) {

?

David_Rothstein’s picture

David_Rothstein commented

@nixar, I wrote this years ago so I don't really remember, but from reading the code comment it looks correct as is. I think it checks the opposite condition since the code is undoing the transliteration, not transliterating it.

Is the functionality not actually working for you when you try it? If so, check #9 and #10 above (those sound like they could be actual bugs), and if so, a followup issue to fix that would be a good idea.