New image styles are not brought into the feature nor are existing image styles updated. It seems all that works properly is exporting and not importing via Enable or Revert. This is a major flaw in regards to Image Styles features capabilities as it makes keeping image styles in code revision impossible.

To reproduce:
1. Create an Image Styles feature with some image styles in it.
2. Enable the feature so that it says "Default"
3. Add a new image style
4. Click Override
5. Select items to revert
6. Click revert
7. Observe that Overridden status still exists.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

crystaldawn’s picture

A known workaround for this issue can be found here:
https://drupal.org/comment/7503246#comment-7503246

hefox’s picture

Priority: Critical » Major

Please provide more detialed instructions - where are you clicking override? this is in image styles setting? is this a core bug?

crystaldawn’s picture

Adding a new image style will cause the Feature (In the features administration panel where you can see ALL feature modules installed) to display "Overriden" which is correct because something has changed. But if you click override to resolve the issue (remove the change or add the change in most cases involved with importing new changes), you will see that it does not add or remove anything at all and the overriden status does not change because nothing is changed and the features module and site state do not match causing the overriden status to be displayed.

nicobot’s picture

I can see some issues when reverting image styles that I would like to expose,

1- image_default_style_revert() deletes the image style. I'm not sure why this is the default operation when reverting an image style, and also when reverting some features components through hook_features_revert. My assumption is that it's a mistake to just delete the image style.

2- image_features_revert() is not doing anything when processing new image styles

3- when exporting an image style, the 'ieid' (image effect id) property it's being added as the key of the effects array, and while there is a sanitization process this is not correctly handled.
Example:

// Exported image style: arranger_team_full.
  $styles['arranger_team_full'] = array(
    'name' => 'arranger_team_full',
    'label' => 'Arranger team full',
    'effects' => array(
      43 => array(
        'label' => 'Escalar y recortar',
        'help' => 'Escalar y recortar mantendrán la relación de proporciones de la imagen original y se desechará lo que sobre por el lado mayor. Esto es especialmente útil para crear miniaturas perfectamente cuadradas sin deformar la imagen.',
        'effect callback' => 'image_scale_and_crop_effect',
        'dimensions callback' => 'image_resize_dimensions',
        'form callback' => 'image_resize_form',
        'summary theme' => 'image_resize_summary',
        'module' => 'image',
        'name' => 'image_scale_and_crop',
        'data' => array(
          'width' => 152,
          'height' => 152,
        ),
        'weight' => 1,
      ),
    ),
  );

In the example the key 43 corresponds to the IEID of the system where the image style was created.
This makes that reverting a feature with image styles remains on overriden status all the time.

I'm attaching a patch based on the assumptions made above. Hope it helps.

nicobot’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 4: new_image_styles_are-2148453-4.patch, failed testing.

Dane Powell’s picture

Title: New image styles are NOT brought in when using "Revert" » Image styles do not revert properly
Status: Needs work » Needs review
FileSize
1.34 KB

I think deleting the image style via image_default_style_revert() is the desired behavior- that way existing image derivatives will be deleted and regenerated to pick up the new image style settings. When I used the patch in #4 as-is, stale image derivatives always hung around after updating an image style, and I had to manually override/revert the image style to regenerate them.

This patch fixes that.

Status: Needs review » Needs work

The last submitted patch, 7: features-2148453-7.patch, failed testing.

Dane Powell’s picture

Version: 7.x-2.x-dev » 7.x-2.3

Not sure why tests are failing... possibly due to the changes in #2308801: Export less data for image effects (within image styles)

Dane Powell’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: features-2148453-7.patch, failed testing.

Dane Powell’s picture

Status: Needs work » Needs review
FileSize
1.18 KB

Yeah, looks like this conflicted with #2308801: Export less data for image effects (within image styles). This should fix it.

EDIT: this patch introduces another problem, where every image style is reported as overridden after every revert, due to image_default_style_save being called every time.

Dane Powell’s picture

I think we got off to a bad start here... here's a new patch written from the ground up.

The only real problem I see, which this patch addresses, is that Features assumes that if image style storage is not reported as being overridden, it doesn't need to be reverted. This is obviously a bad assumption, because the whole point of Features is that you can push changes to the image styles in code, and the core image module has no mechanism for detecting code changes and marking the image style as overridden. So it's up to Features to check if the file has changed at revert time.

slucero’s picture

Status: Needs review » Needs work

In the case where the image style already existed and just needed to be updated the patch from #14 worked great, but when the image style didn't exist yet and needed to be created the revert didn't create it and left the component status as Overridden.

RavindraSingh’s picture

Who all are still facing this issue, please update with latest release 7.x-2.5. I have tested as mentioned and it is not showing me override. attached screenshot is a reference.

Closing this issue, please reopen if you face this issue in different scenario.

Thanks
Ravindra

mpotter’s picture

Priority: Major » Normal
Status: Needs work » Closed (works as designed)

As mentioned in #16, please reopen this if this patch is still needed in the latest version.

marcelovani’s picture

I know this issue is closed and will not re-open it.
Just wanted to mention what I found.

The features overrides for Image try to fix the problem with the ieid in image_features_override_component_overrides_alter()
When you try to add an override via UI you will see the key for the effect as [0] but in the database it could be any other value.

I managed to make this work by resetting the keys of the effects, starting the count from 0 on every image exported, i.e.

$styles['thumbnail_small'] = array(
  'name' => 'thumbnail_small',
  'label' => 'thumbnail_small',
  'effects' => array(
    14 => array(
      'label' => 'Javascript crop',
...      
),
   15 => array(
  'label' => 'Rotate',
...      
),
   ),
   16 => array(
  'label' => 'Scale and crop',
...      
),
   ),
 ),
);
$styles['wide'] = array(
  'name' => 'wide',
  'label' => 'wide',
  'effects' => array(
    17 => array(
      'label' => 'Javascript crop',
...      
),
   18 => array(
  'label' => 'Rotate',
...      
),
   ),
   19 => array(
  'label' => 'Scale and crop',
...      
),
   ),
 ),
);

Will have the keys changed to

$styles['thumbnail_small'] = array(
  'name' => 'thumbnail_small',
  'label' => 'thumbnail_small',
  'effects' => array(
    0 => array(
      'label' => 'Javascript crop',
...      
),
   1 => array(
  'label' => 'Rotate',
...      
),
   ),
   2 => array(
  'label' => 'Scale and crop',
...      
),
   ),
 ),
);
$styles['wide'] = array(
  'name' => 'wide',
  'label' => 'wide',
  'effects' => array(
    0 => array(
      'label' => 'Javascript crop',
...      
),
   1 => array(
  'label' => 'Rotate',
...      
),
   ),
   2 => array(
  'label' => 'Scale and crop',
...      
),
   ),
 ),
);

This fixes the problem with not being able to override the images, but there is still a problem where if the original image style is changed and a new effect is added, the Features overrides will not detect that change.
The only way to fix that would be to have either a machine_name to be used as keys for each effect or md5 the array of effect and use that as key.
I don't know if that would be the end of the problems, it's just an idea, I haven't spent time to proof it.

2pha’s picture

I am facing this issue today.
I have a feature which includes a couple of image styles.
The "image styles" display as "overridden" on the feature admin page.
reverting the image styles component on this feature does not add the image styles and they remain as "overridden".

ComboPrime’s picture

I can confirm this issue is still present in 7.x-2.7, although oddly enough only on one of 3 identical servers (works correctly on local and dev, broken on demo).

Note in screenshot that the Image Styles have supposedly been reverted, but still display "Overridden". Checking the actual style confirms that the Feature module update is not being used:

Screenshot of Image Styles feature showing "reverted" message but still Overridden.

ComboPrime’s picture

UPDATE: Since the Feature wasn't taking, I updated my image style manually. Then on a whim I tried to Revert again--and this time it worked!

No idea what made the difference, or whether this will happen again next time. I'll try to remember to post here again when I have another Image Style Feature update.

ShaneOnABike’s picture

Same for me I had this issue. Essentially, the only way to make it work was to create the style and then revert the feature. Can't hte feature just determine if the style doesn't exist and create it?

ashooner’s picture

I was experiencing this issue, but simply using calling drush fr --force with the force flag seemed to work to get the new image styles created, and the feature really reverted.

joseph.olstad’s picture

Status: Closed (works as designed) » Reviewed & tested by the community

using 7.x-2.10+3-dev I came accross this issue. drush fr image_styles --force -y did not work either.

However, the patch does work!

Patching with patch #14 resolves the issue, after patching the problem goes away.

This applies to the latest dev of features 7.x-2.x

joseph.olstad’s picture

Thanks to @Dane Powel for the patch.

joseph.olstad’s picture

Version: 7.x-2.3 » 7.x-2.10

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: features-2148453-18.patch, failed testing.

joseph.olstad’s picture

Patch #14 is my preferred solution.

joseph.olstad’s picture

Status: Needs work » Reviewed & tested by the community
lamp5’s picture

#14 works great! Thanks!

joseph.olstad’s picture

Priority: Normal » Major

Re-triggered tests on patch #14, it's still good.

joseph.olstad’s picture

Version: 7.x-2.10 » 7.x-2.x-dev

joseph.olstad’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.