This is popping up for people using the file replacement functionality and their browsers caching the image. I'm thinking we should add cache-busting query strings based on {file}.filemtime which would only be updated when file_save() is run.

Comments

raytiley’s picture

I looked through the code to try and figure out where to do this as i thought it would be pretty simple, but need to get more familiar with it...

Etherway I found this info pretty interesting: https://github.com/h5bp/html5-boilerplate/wiki/cachebusting

Looks like just adding a simple ?v= will probably work for a majority of users, but isn't guaranteed to work for users behind a proxy server. I doubt we could get better though since I guess we can't change the actual files name or assume any server config.

-ray

Dave Reid’s picture

Status: Active » Postponed

I'm going to postpone this for now. Looking at theme_image and theme_image_style it looks like this might be impossible to do.

micbar’s picture

I created a sandbox for adding cache-busting query strings to images.
http://drupal.org/sandbox/micbar/1867948

I rewrote the theme_image function and added a cache busting query string based on filemtime().
I created an altered theme_image function and registered it in the theme registry via theme_registry_alter.

This is my altered theme_image() function:

function theme_cachebustimage($variables) {
  $attributes = $variables['attributes'];
  $internal_path = str_replace($GLOBALS['base_url'] . '/', '', $variables['path']);
  if (is_file($internal_path)) {
    $cachebust = filemtime($internal_path);
  }
  else {
    $cachebust = time();
  }
  $attributes['src'] = file_create_url($variables['path']) . '?' .$cachebust;

  foreach (array('width', 'height', 'alt', 'title') as $key) {

    if (isset($variables[$key])) {
      $attributes[$key] = $variables[$key];
    }
  }

  return '<img' . drupal_attributes($attributes) . ' />';
}
micbar’s picture

Status: Postponed » Active

change status

bleen’s picture

micbar ... your solution would put a cachebusting string on EVERY image all the time... That is not good for performance

Ignore that comment ... I just loked closer and realized that you are writing filetime as your cachbuster so that might be a feasible solution

micbar’s picture

As you can see, i modified the theme_cachebustimage() to improve compatibility with drupal 7.20+

function theme_cachebustimage($variables) {
  $attributes = $variables['attributes'];
  $path = drupal_parse_url($variables['path']);
  $token_query = $path['query'];
  $internal_path = str_replace($GLOBALS['base_url'] . '/', '', $path['path']);

  if (is_file($internal_path)) {
    $cachebust = filemtime($internal_path);
  }
  else {
    $cachebust = time();
  }
  $token_query += array('time' => $cachebust);
  $file_url = file_create_url($path['path']);

  $attributes['src'] =  $file_url . (strpos($file_url, '?') !== FALSE ? '&' : '?') . drupal_http_build_query($token_query) ;

  foreach (array('width', 'height', 'alt', 'title') as $key) {

    if (isset($variables[$key])) {
      $attributes[$key] = $variables[$key];
    }
  }

  return '<img' . drupal_attributes($attributes) . ' />';
}
asgorobets’s picture

I guess we need to patch core in order to achieve this. Replacing core theme_image with our own function will solve the problem only locally, but other modules might need the same fix. We're facing the same issue with files being cached using File Entity and an alternative solution to Media. Here is an issue created in D7 core issue queue, I hope to collect some suggestions there.
#2313539: Allow cache busting string in theme_image

micbar’s picture

In the related issue #2313539: Allow cache busting string in theme_image I uploaded a patch for D8.