Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#15 | imagecache_347566.D5.patch | 529 bytes | drewish |
#15 | imagecache_347566.D6.patch | 594 bytes | drewish |
#13 | imagecache_347566.patch | 1.28 KB | drewish |
#6 | 347566.patch | 800 bytes | Bevan |
transliteration.patch | 500 bytes | Bevan |
Comments
Comment #1
drewish CreditAttribution: drewish commentedI'm reluctant to force people to use transliterate... perhaps we should just add a link to it from the imagecache project page.
Comment #2
Bevan CreditAttribution: Bevan commented> 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.
Comment #3
drewish CreditAttribution: drewish commentedi'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.
Comment #4
Bevan CreditAttribution: Bevan commentedHmmm. 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?
Comment #5
drewish CreditAttribution: drewish commentedyeah, documenting it in that manner would be very helpful.
Comment #6
Bevan CreditAttribution: Bevan commentedPatch attached
Comment #7
Bevan CreditAttribution: Bevan commentedComment #8
drewish CreditAttribution: drewish commentedI 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.
Comment #9
Bevan CreditAttribution: Bevan commented> 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.
Comment #10
drewish CreditAttribution: drewish commentedyou'd be surprised. but it's fine with as is since it's on the freaking project page.
Comment #12
drewish CreditAttribution: drewish commentedBevan, 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.
Comment #13
drewish CreditAttribution: drewish commentedRolled a backport of #357569: Add support for transliteration in the module as well.
Comment #15
drewish CreditAttribution: drewish commentedbacking this out temporarily.
Comment #16
drewish CreditAttribution: drewish commentedrecommitted now that the security fixes have gone out.
Comment #18
joachim CreditAttribution: joachim commentedI'm not seeing the dependency in beta10.
Comment #19
earthday47Confirmed - Imagecache can be installed without the Transliteration module.
Thought this was a bug until I read this. And yes, even though it's written on the project page, I did not read it. There's a lot of links on that page!
Comment #20
yettyn CreditAttribution: yettyn commentedJust passing by as I need this, but...
It's obvious to anyone who actually read the available information that the Transliteration patch was applied to the dev version, and temporary roled back before the Beta (security) release, than put back in again.
So this bug should really have stayed closed as it has nothing to do with 6.x-2.0-beta10, but... if we now should be nitty gritty pedantic, it should say on the project page for dependencies:
Install and enable Transliteration module (dev version only)
possibly with a note it's recommended for others. I don't know what went round in the head of drewish but I can guess he didn't want to add it as it would have mock up things for those who have a beta installed but not Transliteration, taking into account the beta is the currently recommended version and as such sort of expected to be "stable", while using dev you sort of expect odd things as a sudden dependency happen. Now anyone using the beta can take action and install Translitteration and be prepared for the next step w/o sudden surprises.
So setting this back to dev and Documentation for the dev note above to be added on project page.
Comment #21
drewish CreditAttribution: drewish commentedI'm going to back this out after all. I changed my mind on making it required, I don't think it makes sense to have on every site and therefore should be optional.
Comment #22
nicholasThompsonIt said on the 6.x-2.0-beta12 page that this was committed...
http://drupal.org/node/1159260
I assume that's an error? beta12 doesn't seem to have it as a dependancy...
Comment #23
Bevan CreditAttribution: Bevan commentedI think it was added, removed, added again, then finally removed again. I don't know it's current status.
Comment #24
drewish CreditAttribution: drewish commentedYeah it was added, then removed twice. It will not be added again in the future.