Require transliteration module
Bevan - December 16, 2008 - 04:59
| Project: | ImageCache |
| Version: | 6.x-2.0-beta10 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Description
Drupal 6's menu handler stops doesn't parse characters after an ampersand in arguments. (Nor should it)
Therefore imagecache is unable to find the targeted file to cache when an incomplete path is passed to it as an argument to the menu callback. (If the file has a an ampersand in it's name).
The tranliteration module resolves this issue handsomely by removing characters like ampersands from file names before writing them to disk. This patch introduces a dependency on transliteration module. This fixes the bug in an elegant way.
| Attachment | Size |
|---|---|
| transliteration.patch | 500 bytes |
#1
I'm reluctant to force people to use transliterate... perhaps we should just add a link to it from the imagecache project page.
#2
> I'm reluctant to force people to use transliterate.
How come? This is a major bug in imagecache. It's not reasonable to expect end users (let alone developers) to know that they need to remove ampersands and non-latin characters from filenames, and filenames with these characters are common enough in all languages that it's likely the bug will be encountered on most sites.
> perhaps we should just add a link to it from the imagecache project page.
IMHO this is not a good solution, because it admits that imagecache is buggy yet forces the IC user to search for the fix themselves (even though the fix may apparently be obvious), when the bug could easily be avoided in the first place.
#3
i'm not convinced because in many cases it's platform specific. the real bug is in core not imagecache so feel free to work on getting transliterate committed to core.
#4
Hmmm. I think we can agree to disagree. By adding transliteration as a dependency to IC, transliteration is going to get a lot more attention that will help it get into core someday.
Would you consider or accept a patch adding documentation to imagecache.info's description that transliteration module is highly recommended?
#5
yeah, documenting it in that manner would be very helpful.
#6
Patch attached
#7
#8
I added a note to the project page. I think I'd rather avoid adding it to the .info file. A note in the README.txt or INSTALL.txt would probably be more appropriate.
#9
> A note in the README.txt or INSTALL.txt would probably be more appropriate.
People generally don't read these files. I think it's a shame you aren't willing to give this issue the attention it deserves.
#10
you'd be surprised. but it's fine with as is since it's on the freaking project page.
#11
Automatically closed -- issue fixed for two weeks with no activity.
#12
Bevan, I've changed my mind. I'm going to require transliteration. Committed the original patch to HEAD, need to back port to DRUPAL-5--2.
#13
Rolled a backport of #357569: Add support for transliteration in the module as well.
#14
Automatically closed -- issue fixed for 2 weeks with no activity.
#15
backing this out temporarily.
#16
recommitted now that the security fixes have gone out.
#17
Automatically closed -- issue fixed for 2 weeks with no activity.
#18
I'm not seeing the dependency in beta10.