When an image style is deleted you're given the opportunity to choose a new style to replace the one that is being deleted, modules are then given the opportunity to react to this change. Image module should be responding on behalf of image fields and updating each field instances display settings as needed.
Steps to repeat the problem.
1. Add a custom image style
2. Use the newly created image style as the formatter for an image field. For example the field_image field on the article node type.
3. Delete the newly created image style, when presented with the option to choose a replacement style choose 'thumbnail' from the list.
Desired effect: Custom image style is deleted and the field_image formatter that was using it is now updated to use the 'thumbnail' formatter.
Actual effect: Custom image style is deleted, field_image formatter is reset to default format.
The attached patch attempts to fix this problem however I'm running into a couple of issues.
First. image_style_delete() invokes hook_image_style_delete after the custom style has already been deleted so the new image_image_style_delete() function has no way to know which fields were using the old style as they are already reset to default. So, I've moved the hook invocation earlier in the function. Which is working.
Second. While the attached implementation of image_image_style_delete() does the trick, for some reason the custom style isn't deleted the first time you click delete (formatters are updated as expected). You have to click it a second time and go through the process again before it is deleted. This seems to be caused by the call to field_update_instance() -> _field_write_instance(), however I can't figure out why. Any help would be appreciated.
Finally, this patch depends on #601966: Tests for image field as it extends the tests in that patch.
Comments
Comment #1
naxoc CreditAttribution: naxoc commentedI can't see a patch - did you forget to upload it? ;)
Comment #2
eojthebraveApparently I did. Here it is.
Comment #3
eojthebraveTests are passing locally now, lets see what the bot has to say about it.
Comment #4
cosmicdreams CreditAttribution: cosmicdreams commentedI'm subscribing so I can test this patch tomorrow.
Comment #5
cosmicdreams CreditAttribution: cosmicdreams commentedAfter I deleted the custom image style in my first test, my content type (Article) displayed a image style set at
Comment #6
cosmicdreams CreditAttribution: cosmicdreams commentedflushed cache, applied database update, ran cron, tried again.
By following the testing procedure described by the OP, I was able to still see this bug happen. I'll switch this back to needs works as it doesn't seem to be fixed yet.
Comment #7
andypostsubscribe
Comment #8
andypostPatch does not work or me!
Delete happens only if I visit delete twice, after first attempt only changed display style, after second delete happens and as #5
Also when preset is renamed all settings are reverts to "Image" and preview set to ""
Comment #9
eojthebraveHere's another go at this, found another issues while trying to figure this out the solution for which is wrapped in to this patch since it is necessary in order to make it work.
image_style_load()
should allow you to load an image style by it's unique numeric ID but doesn't. The function locates but never returns the appropriate $style array.This version of the patch also adds support for renaming a style via hook_image_style_save().
chx expressed concern in IRC about this line:
if (preg_match('/__([a-z0-9_]+)/', $display['type'], $matches)) {
Which is used to determine which style an image field is using. I'm not entirely sure what he didn't like about it. My argument is that it is the same way we are detecting which style an image field uses in
image_field_formatter_view()
and for consistency it should be done the same way.Comment #10
yched CreditAttribution: yched commentedMinor : please name the var $view_mode instead of $display_name.
About the regexp : true, that's the way to do it currently. The style name is buried in the string name of the formatter. In an ideal world, there would just be a single 'use_image_style' formatter with the style name as a formatter setting, but we could not come up with a UI for formatter settings, so they are as good as dead in current D7...
Powered by Dreditor.
Comment #11
eojthebraveChanges $display_name to $view_mode as per #10.
Comment #12
andypostGreat now it works!
I just added a change of 'preview_image_style' and a bit optimizations - $instance should not be saved in loop!
Comment #13
cosmicdreams CreditAttribution: cosmicdreams commentedworked for me!!!! so happy this worked.
Comment #14
eojthebraveThe fix to image_style_load() in the patch above also fixes an issue where overriding an image style defined in code that has more than one image effect causes a WSOD. The problem is that image_effect_save() is called twice and as a result image_style_flush() is also called twice. The call to image_style_save() inside of image_effect_save() tries to load the style by ID and fails so image_style_flush just recursively deletes the entire files/style/* directory in the first call, and then tries to delete it again on the second call. Can't find it, and blam!
You can test this by creating a module which implements hook_image_styles_alter() and adds a second effect to one of the existing thumbnails, or by implementing hook_image_default_styles() and adding a new style with two or more effects. See: #750238: image_example.module demonstrates use of Drupal 7 Image styles and effects. And then clicking the "Override" option for a style with two or more effects.
Comment #15
sunCritical bump.
Comment #16
aspilicious CreditAttribution: aspilicious commentedcan't apply image.test part, needs a reroll :)
Comment #17
andypostRe-roll with changes from HEAD
Comment #18
aspilicious CreditAttribution: aspilicious commentedComment #19
webchickCommitted to HEAD. Thanks!