I read through the transliteration issue, mainly #357569: Add support for transliteration in the module quite a few times but now I experienced a problem with it and I kind of give it a second thought and I think the use of that code is mistaken, otherwise I am.

I am upgrading a drupal 5.x site to drupal 6.x, with use of imagecache. I enabled transliteration and the old untransliterated filenames, when used in imagecache presets stopped working. The problem basically is that although the filename gets transliterated for the preset, as the original filename is different imagecache fails to create the preset image. A read at #199660: Convert pre-existing filenames was good to know there's a define in the transliteration.install file so the transliteration is retroactive to all the files in the files table. That might be a solution for the imagecache files, but I also have files in the db that are hard coded in node's body, so I am not ready to do this retroactive thing. I think I should be able to enable transliteration module for new files and be sure that the old content (even that one generated from imagecache presets) is still properly rendered.

While looking at the code (both imagecache and transliteration) I wondered... why is the transliteration function needed at all?

imagecache_create_url() received a path that should have been transliterated in order imagecache preset can properly create the image (am I right here?).

transliteration plugs into the $_FILES variable so with that module enabled, files would be transliterated as soon as they are upload.

If so, there's no need to use the translitartion_get() function at all, and even more, if I am right, leads to trouble.

Thoughts?

Attached is the patch that removes the code, but the coding part is very clear if my thinking with this issue is right.

CommentFileSizeAuthor
imagecache.module_no_transliteration_code.patch631 byteshanoii

Comments

smk-ka’s picture

smk-ka commented
Assigned: Unassigned » smk-ka
Priority: Minor » Normal

This is actually true. ImageCache cannot simply change the path to the original image file in generated URLs, since after doing that it wouldn't be able to find the original image anymore when it needs to generate the derivate image. Instead (and unfortunately), the module that saves the original image on disc needs to transliterate the target path, if it allows dynamic replacements in the path (such as Node Gallery module; issue: #598804: Imagecache not work with Cyrillic; corresponding issue for transliteration: #599942: Support transliteration).

I have yet to understand what was the primary cause for adding transliteration, so not setting to RTBC. Could somebody point me to a clarifying issue comment or provide me a summary?

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented

As far as I can tell, but it's not more from what I said before on this issue, that bit of code was added in #357569: Add support for transliteration in the module. Now, as an outside observer, it seems to me that this was just a misconception back then on how transliteration module works and how it should be used with imagecache.

Just as a reminider for anybody (me included), this documentation page is probably worth editing it to update the transliteration part: Troubleshooting imagecache

naxoc’s picture

naxoc commented

I have some trouble with files because of the transliteration too. On a site older than Mon Apr 13 00:27:57 2009 http://drupalcode.org/viewvc/drupal/contributions/modules/filefield/file... file names were not transliterated, so I have a lot of filenames with funny danish letters. That worked fine. When filefield started to transliterate that worked fine too, but the retrival of the older files with funny letters does not work.

The patch in #1 fixes that.

Not sure if that answers the question of the primary cause for adding transliteration?

smk-ka’s picture

smk-ka commented
Status: Needs review » Reviewed & tested by the community

Yes, this definitely needs to be fixed.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: I think the use of transliteration as part of the code is not really needed and actually causes trouble » Wrong integration with Transliteration
smk-ka’s picture

smk-ka commented

Marked #703352: Don't transliterate path, or make it an option as a duplicate of this issue.

drewish’s picture

drewish commented

humm... what about requiring transliteration since it'll just fix new files? then backing out this patch?

drupalfan2’s picture

drupalfan2 commented
Priority: Normal » Critical
Status: Reviewed & tested by the community » Patch (to be ported)

When will this patch be applied?

I run into this problem on a live site after enabling the module
Mime Mail CSS Compressor

This module seems to activate "transliteration" in some way, but I did'nt use transliteration till now on a live site with thousand of pictures (attached to cck field).
So everyone who wants to use Simplemail runs into this problem and all images with special characters (Umlaute) in image filename will not be displayed anymore in imagecache.

This is critical error, it occurs on live sites after enabling other modules!

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented
Status: Patch (to be ported) » Reviewed & tested by the community

patch (to be ported) is used for patches that need porting to a different version, not sure if it's the best state for this issue.

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Needs work

still waiting for my question in #7 to be answered.

drupalfan2’s picture

drupalfan2 commented

When will this patch be applied?

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented
Status: Needs work » Needs review

@drewish: I don't see how requiring transliteration will fix this issue. For what I understood, and I think most is explained on my first report on the issue, the problem is not whether to require transliteration or not, but what to do with the already uploaded files. If you make transliteration required, we will face the same problem I am describing and the need to fix all hard-coded links to old files. On the other hand, as I explained, I think the use of transliteration as part of the module is unneeded, as the transliteration module will fix the filename before being stored on the db, so there's no need to transliterate in the imagecache module again, and while not using it, it won't cause any problem to the files uploaded before enabling the module.

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented

I actually saw that #347566: Require transliteration module when into the latest 5.x release. I think that transliteration must not be a requirement because of what explained here, unless I am mistaken.

szy’s picture

szy commented

Upgrading to 6.x-2.x-dev from 6.x-2.0-beta10 is a little disaster,
as it takes a while to find out that transliteration is the problem.

It is not fine, when one module turns on another without a word
to admin - 6.x-2.x-dev does it with Transliteration module.

Szy.

YK85’s picture

YK85 commented

subscribing

thommyboy’s picture

thommyboy commented

there is no need for imagecache to transliterate any filenames as they are changed by transliteration-module and stored correctly in the files-db.
so i think (in the newest .dev of imagecache) lines 316ff need to be removed:

if (module_exists('transliteration')) {
$path = transliteration_get($path);
}

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented

that's exactly what the patch in #0 does and I am still feel that's the proper thing.

thommyboy’s picture

thommyboy commented

absolutely ;) but i think it could/should be removed from the module completely so on updates you don't have to remember to apply a patch...

hanoii’s picture

hanoii
Primary language Spanish
Location 🇦🇷UTC-3
commented
Status: Needs review » Reviewed & tested by the community

the idea of the patch is to be applied and included in the module, I was just noting that what you suggested is the same as the patch, so it's basically a test of it, so maybe it's ready to be committed?

thommyboy’s picture

thommyboy commented

well- i removed these three lines and everything is fine...

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Fixed

Committed to 6.x-2.x

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.