image_toolkit_invoke('settings') was removed in CVS commit 26667 to fix issue 50078. It generated errors at the time because image.imagemagick.inc had not been updated to work with Drupal 4.7. I have submitted a patch to the Image project (issue 51467) to fix this and would like this setting to be restored.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | image.inc-gd-message.patch | 882 bytes | walkah |
| #7 | system_16.patch | 614 bytes | darren oh |
| #4 | system_module_2.patch | 458 bytes | chx |
| #2 | system.module_43.patch | 594 bytes | darren oh |
| system.module_40.patch | 418 bytes | darren oh |
Comments
Comment #1
merlinofchaos commentedI'm guessing I didn't know what that setting did when I removed that function, and it appeared to be causing problems of some sort. I have nothing against restoring it so long as it works.
Comment #2
darren ohThe reason this patch is necessary is that image toolkits are .inc files, not modules. Image_toolkit_invoke is the only way they can alter the settings form. Re-rolled a patch for HEAD.
Comment #3
merlinofchaos commentedJust to add some explanation:
Image toolkits are simple .inc files. image.inc searches for them and makes them available on the settings page.
Since they are just .inc files and .modules, they cannot implement hooks at all. They implement only a series of functions defined by image.inc
If they need settings, a mechanism like this is the only option they have for getting them to the user. I did not understand how that worked when I made the original patch.
Comment #4
chx commentedI needed to reroll but we are good to go otherwise.
Comment #5
drummWhat is this doing on the admin modules page, only when throttle module is enabled?
Comment #6
darren ohWhat are you seeing? This one-line patch should only affect the admin/settings page.
Comment #7
darren ohHere's a corrected patch for Drupal 5. My original patch will fix Drupal 4.7.
Comment #8
ChrisKennedy commentedWhy is image.module's settings page in system.module instead of image.module?
Comment #9
darren ohBecause these are not image module settings. If you think the way Drupal handles image toolkits should be changed, please share your ideas in issue 99171.
Comment #10
merlinofchaos commentedIt's not image.module's settings, it's image.inc's settings, for the pluggable toolkits.
I think at this point the pluggable toolkits should probably be modules and not .incs because modules are a lot easier to track, but it's too late to change this in Drupal 5.
Comment #11
dries commentedCommitted to CVS HEAD. Thanks.
Comment #12
merlinofchaos commentedMarking this to backport to 4.7 for killes.
Comment #13
killes@www.drop.org commentedapplied
Comment #14
walkah commentedWhile this is a great (long long long overdue) fix, there is a slight problem. the GD (default) toolkit returns a string for the settings if it installed and running properly - which now (if 2+ toolkits are available) gets added into the form. the attached patch changes the image_gd_settings function to return form markup rather than the plain text.
Comment #15
dries commentedI don't have a good test environment handy for this. What does the patch accomplish? Maybe it could be documented in the code? I think I know what is going on, but I'd like to be sure. Thanks.
Comment #16
walkah commentedDries: all you need to test is to copy image.imagemagick.inc to includes (from image module CVS / tarball). This issue describes what happens: http://drupal.org/node/105075 . i.e. WSOD on the image toolkit settings page.
Comment #17
drummThat line of text needs a
<p>tag.Comment #18
drewish commentedThis patch has been broken by #105164, which seems to do mostly the same thing but I can't seem to figure out where the image magick settings are supposed to be...
Comment #19
drewish commentedscratch that, i just needed to submit the form with image magick selected.
Comment #20
(not verified) commented