The documentation of hook_image_toolkits() is inaccurate.
http://cgit.drupalcode.org/drupal/tree/modules/system/system.api.php?h=7...
The documentation says:
* The file which includes each toolkit's functions must be declared as part of
* the files array in the module .info file so that the registry will find and
* parse it.
Since the #497118: Remove the registry (for functions) is fixed the functions are not part of the registry. Instead this should say The file which includes each toolkit's functions must be included in this hook. See https://api.drupal.org/api/drupal/modules%21system%21system.module/funct... for an example.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | drupal7-Inaccurate-documentation-hook_image_toolkits-2383491-2.patch | 654 bytes | pushpinderchauhan |
Comments
Comment #1
chx commentedThere's no patch so the issue status is "active" also the tag used for these is "novice".
Comment #2
pushpinderchauhan commentedPlease review patch.
Comment #3
sweetchuckThere are some issue in the core's image toolkit system.
The problem is that when you need to do something with the image tookit you have to know which one is the current.
Call the
image_get_toolkit()which calls theimage_get_available_toolkits()which collects the available toolkits by thehook_image_toolkits()at this point all image toolkit file/function will be included even if it is not active.The problem is that all image toolkit file/function have to be included because there is a very silly fallback system in the
image_get_toolkit().If the configured image toolkit is not available then it uses the first available toolkit.
This fallback system is nowhere promoted on the UI. I think nobody knows it is even exists.
The image tookits are ORDER BY
-
hook_module_implements_alter()-
{system}.weight-
{system}.nameThe site administrator has no control over the fallback system.
It would have better to add
weight,pathandfileproperties tohook_image_toolkits()and then the core would have been able to include only the necessary files.Yes, you can include the necessary files in the
hook_image_toolkits(), just because the result of themodule_invoke_all('image_toolkits');is not cached withcache_set().But very ugly. And maybe there is no better solution.
Comment #4
jhodgdonRE #3, are you saying that the patch is not good? Is all of this commentary a separate issue?
Because I think the patch looks fine for the issue originally reported... and I'm not following all of #3 but I think it's a separate issue (and maybe not just documentation)?
Comment #5
sweetchuckThe last paragraph of comment #3 is my relevant opinion about patch #2
I just tried to explain what is the problem with the toolkit system.
patch #2 RTBC++
Comment #6
jhodgdonThanks! Committed to 7.x.