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.

CommentFileSizeAuthor
#14 image.inc-gd-message.patch882 byteswalkah
#7 system_16.patch614 bytesDarren Oh
#4 system_module_2.patch458 byteschx
#2 system.module_43.patch594 bytesDarren Oh
system.module_40.patch418 bytesDarren Oh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

I'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.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
FileSize
system.module_43.patch594 bytes

The 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.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Just 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.

chx’s picture

chx CreditAttribution: chx commented
Version: 4.7.x-dev » 5.x-dev
Status: Needs review » Reviewed & tested by the community
FileSize
system_module_2.patch458 bytes

I needed to reroll but we are good to go otherwise.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Needs work

What is this doing on the admin modules page, only when throttle module is enabled?

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented

What are you seeing? This one-line patch should only affect the admin/settings page.

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented
Status: Needs work » Needs review
FileSize
system_16.patch614 bytes

Here's a corrected patch for Drupal 5. My original patch will fix Drupal 4.7.

ChrisKennedy’s picture

ChrisKennedy CreditAttribution: ChrisKennedy commented

Why is image.module's settings page in system.module instead of image.module?

Darren Oh’s picture

Darren Oh
Primary language English
Location Lakeland, Florida
CreditAttribution: Darren Oh commented

Because 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.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

It'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.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
Status: Fixed » Patch (to be ported)

Marking this to backport to 4.7 for killes.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented
Status: Patch (to be ported) » Fixed

applied

walkah’s picture

walkah CreditAttribution: walkah commented
Status: Fixed » Reviewed & tested by the community
FileSize
image.inc-gd-message.patch882 bytes

While 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.

Dries’s picture

Dries CreditAttribution: Dries commented

I 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.

walkah’s picture

walkah CreditAttribution: walkah commented

Dries: 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.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Reviewed & tested by the community » Needs work

That line of text needs a <p> tag.

drewish’s picture

drewish CreditAttribution: drewish commented

This 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...

drewish’s picture

drewish CreditAttribution: drewish commented
Status: Needs work » Fixed

scratch that, i just needed to submit the form with image magick selected.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)