File extensions don't match the actual MIME type - Some browsers do not display converted images

Fair enough, that hack was improper, and bordering on illegal, I know :-)
It just seemed to work anyway in the browsers I tested at the time, but it made me feel dirty.

Hm.
This approach is certainly a move in the right direction but it doesn't feel holistic enough. There are any number of other things out there that may make assumptions about imagecache naming schemes that would have to be tweaked individually. Overriding formatter callback functions (though logical/inevitable) makes using this a lot tricker than before, IMO.

Thinking about it freshly (a year since I put the hack in) I'd possibly approach it with a subtle little .htaccess rewrite in the file dir itself. Basically content-negotiation at the file-level.
Leaving everything else as it is...
if file.jpg does not exist in imagecache/profilename/files/ folder,
see if file.jpg.png does, and sned a 301 to that real file.
This will (should) give browsers enough to go on, while being transparent to all callers.

So, three lines of .htaccess (although I can't invent that OTTOMH right now)
In leu of htaccess, we can get the same effect using hook_menu - that's how imagecache works now anyway. Less efficient though.

OR - a full though possibly chunkier solution - dig at imagecache_url() (or whatever that function is called) and insert a hook callback so I can change the imagecache pathnames on the fly. Most well-behaved other modules that leverage imagecache should be going through that. IIRC

it could be left to the user to modify theme_imagecache()

Yeah, but I'd rather see a configurable solution that works out-of-the-box.

Supposing file.jpg is actually a png file

? Then someone has been evil like have, and is introducing confusion like I have. :=}
Two requests to the browser are no problem because that's not two data transfers, just header negotiation. Two Apache requests are much more lightweight than a Drupal bootstrap.
I'm pretty sure imagecache just won't work on a system that doesn't already have rewrites in place. Not going to let IIS users bother me.

I'd have to try this out on a platfom or two to see if it really gels together. I'm uncomfortable about multiplying factors here - by introducing a whole new storage area and behaviour here. I'd hope for a more transparent fix - a fallback that just happens - but am not sure it exists.
I'll have to have a play and a think

egfrith, I found a small bug in your code. You are retrieving the file format two times, once in

function imagecache_rename_get_extension($preset) {
  $format = '';
  $extension = '';
  foreach ($preset['actions'] as $action) {
    if (($action['action'] == 'imagecache_convert') && isset($action['data']['format'])) {
      $format = $action['data']['format'];
    }
  }
  if (isset($format)) {
    $formats = imagecache_file_formats();
    $extension = $formats[$format];
  }
  return $extension;
}

and then in line 142, which I changed to the following:

  //$formats = imagecache_file_formats();
  //$filepath .= '.' . $formats[$format];
  $filepath .= '.' . $format;
  $path = _imagecache_strip_file_directory($filepath);

With the change your module works great! Thanks for the effort, the ability to have the correct extension is a must have for IE6 and the pngfix scripts!

--Alex Sartogo

Hot Sauce Studios

True, I hadn't considered the effect this thing has on pngfix.
Re the patch/fix/module.
I appreciate and understand the effort, and I'm sorry it takes so much work to work around.

My gut feeling is that something so small should not take so much code to change, although approaching it as a drop-in pluggable rewrite module was ... appropriate.
I'm thinking there is a simpler, tidier fix out there, but I'm not sure what it is.
If it was a 3-line option we could patch into the appropriate place in imagecache itself, then maybe these three pages could get trimmed.

The current behaviour (not updating the file extension) also causes issues with PDFs in the Print module. Subscribing.

subscribing

subscribing

renaming

Another potential approach that doesn't require creating a new module and could a file with the change extension such as:

Original file:
sites/default/files/foo.png
File with type changed:
sites/default/files/imagecache/[preset-name]/foo.jpg

1) Use hook_menu_alter() to alter the imagecache menu callback, pointing it to a new function such as imagecache_coloractions_cache() rather than imagecache_cache().
2) At this callback check if the requested path contains an ImageCache preset defined by imagecache_coloractions that changes a file extension
3) If the path does not refer to such a preset, return imagecache_cache() with the default parameters. If it does contain such a preset, rename the file extension requested in the path per the preset (eg, png.jpg)
4) return imagecache_cache() with the altered file path, so we know which source file we want to serve

This could take care of making the altered file available at that path, but not *rendering* images to use this path. There are lots of places that imagecache paths to images are generated. As egfrith points out, we'd have to chance that on the display side of things, such as in CCK formatters and in the Insert module. Luckily, Insert makes this possible for us with hook_alter_styles, which lets imagecache_coloractions() define its own insert style that could use the altered path with new file extension. However, we'd probably want to also alter imagecache_create_url, which doesn't seem to have a drupal_alter() implementation in it or around it in the codeflow within ImageCache, so this part would probably require a patch to imagecache.

Actually, you don't need to rename the files.

What you need is to serve the right mime type.

Do that by creating the appropriate .htaccess file in the preset folder when it's created.

Elegant enough? :-)

The point in #13 is correct -- the best solution is to serve the right MIME type. The following code will create a .htaccess within each pre-set directory that forces the MIME type iff the imagecache_convert action is used.

   // Build the .htaccess folder
   if (!file_exists($dir.'/.htaccess')) {
     if (!touch($dir.'/.htaccess')) {
       watchdog('imagecache', 'Failed to create imagecache settings file: %dir/.htaccess', array('%dir' => $dir), WATCHDOG_ERROR);
       return FALSE;
     } else {
       // Find the format
       $forcetype = "None";
       foreach ($actions as $action) {
         if ($action['action'] == 'imagecache_convert') {
           $forcetype = $action['data']['format'];
         }
       }
       file_save_data("ForceType $forcetype\n", $dir.'/.htaccess', FILE_EXISTS_REPLACE);
     }
   }

I've put it right after the block that uses mkdir in imagecache.module.

Is this code/solution usable for drupal 7.x?

AFAIK, the image system does serve the right mime type. It certainly does so in D7 and I thought it currently does as well in D6. But I would have to sort that out to be sure.

Are there still (relatively modern versions of) browsers around that do have problems with this?

This is an active problem in the current version of IE.

There is a problem with this from multiple browsers. (IE on windows 7 fro example)

This is a major problem which breaks the display of images JPG converted to PNG on IE up to version 11 (Edge does respect the mime type).

IMO, this fact cannot be ignored by saying "MIME type should respected by the browser" because of the very large market share of these versions of IE.

It's not something we can easily change, nor do we have to, IE11 works fine and as far as I remember it has done so as of IE7. ..

Unless someone comes with proof that it does not work with IE7+ AND with a patch or at least a rather complete outline of how to change this, We are not going to change this. BTW: D8 has solved this in core.

Nice to hear that (after these number of years) the D8 image processing has a solution for this! Bravo!

To make this happen, changes had to be made to the image style processing and theming, something that for us, effect suppliers, is nearly impossible in D7 without overtaking the whole image style processing (dumping core, not extending it).

But because the responsive image module (in core in D8) needed this, it was pushed through, see #2330899: Allow image effects to change the MIME type + extension, add a "convert" image effect.

If anyone has an issue with broken images in IE9,10,11, please check if you added the nosniff header (part of drupal 7.40 release) in .htaccess. This is the reason why my images as described above in #19 were broken.

details: http://blog.merge.nl/drupal-broken-images-convert-png-extension-nosniff-...

Good catch and explaining why this issue popped up again.

Oops, now I got myself caught by this IE problem. Instead of removing the nosniff header I added this to the .htaccess:

<IfModule mod_rewrite.c>
    RewriteRule ^{path/to/public-file-system}/styles/{style-with-change-format}/.*/.*\.*(\?itok=.*)?$ - [NC,T=image/{new-format}]
    # resulting in something like
    RewriteRule ^sites/default/files/styles/rotated-polaroid/.*/.*\.*(\?itok=.*)?$ - [NC,T=image/png]
</IfModule>

The rule can be generated but I am not sure if we can and want to change the .htaccess. So should we do something and what?

generate:
- generate and add these lines to .htaccess on adding a convert format effect to a style
- update function (+ releasing new version) to add this for existing styles with a convert format effect.
- if we generate: .htaccess in root, public or even create our own in public/styles?

or just warn:
- status report: warn to add these lines for all format changing styles.
- warn on adding a change format effect to a style.
- update function to warn for existing styles.

Note: we don't have to act on private files, they are always served by Drupal and already get the correct Content-Type header (based on content not extension).

Drastic but clever.

The most self-contained and least weird answer seems to be to put this sort of approach into public://styles/rotated-polaroid/.htaccess itself.
That could (I guess) be something done by the image preset 'save' action every time the preset is edited. It *is* legal and clean enough for a preset to write to the filesystem inside that directory, but it would not have permissions to do that at a higher level.

I'd hope there would be an Apache way to just set-header for the contents of a directory without a rewrite?

Unfortunately, within an .htaccess the directives Directory, DirectoryMatch, Location and LocationMatch are not allowed, that would certainly have been much easier to read and understand.

I also thought about adding it to that level, but then it would be deleted on a flush. But, if we can hook into an AFTER-flush event that would be OK. I will have a look into that. It would make the .htaccess even simpler, just:

ForceType image/png

hook_image_style_flush is called AFTER deleting the directory, so we can use that to recreate the directory and place an .htaccess in it. I am going to write code that will do so, but will do so in a follow-up issue.

For anyone who's looking for the follow-up, you can find it here: #2664020: IE fails to display images whose extension and Content-Type header do not match the actual image type

Also being a child issue of this one ... :)

Good that this is solved in D8 now. For D7, I found just another workaround:

You can use module filefield_paths to rename files using your target file extension on upload. When only using image styles (with the file type convert action) for actually serving the image, browsers will always get images where file extension and file type match. The downsides are that the original image stored on the server can have an image type differing from the file extension, since conversion happens when applying image styles.

That said, the clean solution for D7 seems to me an option that would convert the file type and change the file extension on upload. Since the image is not in use by then, the missing mechanism to inform other parts of Drupal about a filename change is not an issue. It will however store the image original in the converted format, so won't fit use cases that want to also preserve the uploaded version. But it would be technically clean … . Nothing for the scope of imagecache_actions though.

That wouldn't work if you have multiple image styles converting to different image formats or leaving the format unchanged. Changing all images to png because you have a rotated thumbnail and want the corners to be transparent won't make your website especially fast ...

The solution as made for IE will do for D7.

Thanks for the update on D8 progress.
(sorry, yeah it's old now, but just wanted to tag the issue relationship)