Make image module compatible with thickbox [#179855]

The thickbox module works on the assumption that all image derivative files exist.

However, the image module refuses to create derivative files if the original image is already smaller than the derivative size. No doubt this was done for reasons of space efficiency.

The attached patch compromises by creating the derivatives as hardlinks if they don't need to be resized. Hardlinked files don't take up any extra file space. For Windows installs where the link() function is unavailable, it falls back to a simple copy.

As a bonus, the resultant code is slightly smaller and simpler, and the image_get_derivative_sizes() function is no longer needed.

Comment	File	Size	Author
#3	patch.txt	2.01 KB	jennycita
	image.module_23.patch	5.69 KB	pillarsdotnet

Comments

Comment #1

drewish commented 1 October 2007 at 15:00

Status:

Needs review

» Needs work

i guess you weren't following the queue when i tried to have it just copy files. people went nuts. and in the end i think they're right. you only create derivative images when you need them. if symlinks were cross platform i'd be into that approach but since it'd fallback to a copy it's a no go.

i'd like to get image.module with thickbox but this isn't the way to do it.

Comment #2

pillarsdotnet commented 1 October 2007 at 17:35

Status:

Needs work

» Needs review

Sorry; I was following http://drupal.org/node/125610 where you said that it had been fixed.

In that issue, you hadn't retracted your claim.

I'm going to try and patch thickbox so that if the image it's looking for doesn't exist, it falls back to the original on its own. I don't think it's possible or practical to have it do a database lookup from the javascript.

(looking...)

Thickbox sets the size of the preview image once at the top of the page.. Could override on a per-image basis, but how? If there was a clue in the link attributes, that would be sufficient. But that would mean modifying or overriding the theme_image_teaser() function.

How about setting a class on the link if and only if the preview is the same size as the original?

Here's the least-invasive patch I'd need:

--- orig/image/image.module     2007-09-26 15:40:42.000000000 -0400
+++ patched/image/image.module  2007-10-01 12:45:49.000000000 -0400
@@ -816,7 +816,11 @@
  * Theme a teaser
  */
 function theme_image_teaser($node) {
-  return l(image_display($node, IMAGE_THUMBNAIL), 'node/'. $node->nid, array(), NULL, NULL, TRUE, TRUE);
+ $attributes = array();
+ if ($node->images[IMAGE_PREVIEW] == $node->images[IMAGE_ORIGINAL]) {
+   $attributes['class'] = 'no-resize';
+ }
+ return l(image_display($node, IMAGE_THUMBNAIL), 'node/'. $node->nid, $attributes, NULL, NULL, TRUE, TRUE);
 }
 
 /**

Associated patch for thickbox:

--- orig/thickbox/thickbox_auto.js      2007-09-30 03:14:38.000000000 -0400
+++ patched/thickbox/thickbox_auto.js   2007-10-01 12:34:19.000000000 -0400
@@ -37,7 +37,7 @@
      * ATTENTION: Until the derivate Bug (http://drupal.org/node/125610) is fixed,
      * the script should allways use the original picture ("true || ").
      */
-    if (thickbox_derivative == "_original") {
+    if (thickbox_derivative == "_original" || $(this).is('.no-resize')) {
       var href = img.attr("src").replace(".thumbnail", "");
     }
     else {

And for acidfree:

--- orig/acidfree/class_image.inc       2007-05-10 01:07:29.000000000 -0400
+++ patched/acidfree/class_image.inc    2007-10-01 13:30:07.000000000 -0400
@@ -53,14 +53,18 @@
     }
     $info = image_get_info(file_create_path($node->images['thumbnail']));
 
-    $image = _acidfree_image_display($node, 'thumbnail', array('width' => $info['width'], 'height' => $info['height']));
+    $image = _acidfree_image_display($node, 'thumbnail', array('width' => $info['width'], 'height' => $info['height'], 'class' => 'image-thumbnail'));
     
     $h = $info['height'] + variable_get('acidfree_extra_length',12);
     $w = $info['width'] + variable_get('acidfree_extra_length',12);
     
     $path = "node/{$node->nid}";
     $overlay = l('', $path, array('title' => $node->title), $p, NULL, true, true);
-    $image = l($image, $path, array('title' => $node->title), $p, NULL, true, true);
+    $attributes = array('title' => $node->title);
+    if ($node->images[IMAGE_PREVIEW] == $node->images[IMAGE_ORIGINAL]) {
+      $attributes['class'] = 'no-resize';
+    }
+    $image = l($image, $path, $attributes, $p, NULL, true, true);
     $title = l($node->title, $path, array('title' => $node->title), $p, NULL, true, true);
 
     $imagediv = '<div class="acidfree-cell"><div class="acidfree-item acidfree-image">';

Tested and working here: http://www.kbsystem.com/albums/products/woods

Comment #3

jennycita commented 25 February 2008 at 21:54

Status	File	Size
new	patch.txt	2.01 KB

The thickbox module works on the assumption that all image derivative files exist.

I think this is the root of the problem. It should be using the excellent image_fetch function instead. It allows you to access any form of the image you want like so: image/view/$nid/$derivative -- and also handles substituting the original image for any derivatives that don't exist.

Here's a patch that's working for me. There are a few things that could probably be done better (it'd be nice if thickbox didn't require an extension to recognize an image, for instance), but I've spent enough time on it. In order to support pathauto, an image.module patch is also necessary.