"Allow user to view original" setting broken/Incorrect link building

As it is now a permission and no longer a global flag, this is even better than before! Thanks

I have tested the patch it works.
It would be really good to NOT display the link when the original is displayed! Even better if it is not displayed also when the preview and the original are the same (See drupal.org/node/51600).

I looked at the code, but it seems to me that the parameters of function image_link($type, $node, $main = 0) do not allow to decide. But as I am new to PHP and Drupal, can somebody confirm this before it is committed?

It would be possible if $type in function image_link($type, $node, $main = 0) could be "image-thumbnail", "image-preview", ..., "image-_original" rather than simply "image". But I am certainly dreaming ,-).

I still have a lot to learn in Drupal! It seems so easy when one knows!

But I have tested your new patch and "view original" is still shown even when the original is dispalyed.
I did some debugging and apparently, at least for me, $request is always null!

If you find somme fix for making $request works, can I suggest to use rather something like:
if($node->images[$request] != $node->images['_original']) so that the link is not displayed also when derivative(s) points to the original file (for example when the original image is small).

I discovered also that a user NOT having the permission to view original, if he knows a bit Drupal could in fact see it by http://www.example.com/node/123?size=_original. I think there is a need to protect more the access to the original, for example when it is the site of a professional photographer. So the permission check should certainly be put at other places of the code.

I did a bit more debugging. After

foreach (_image_get_sizes() as $size) {
   if ($size['label'] && !in_array($size['label'], array('thumbnail', '_original'))) {
     $sizes[] = $size;
  }
}

count($sizes) is equal to one in my case.

This is because I am in the default case: only two (required) derivatives "thumbnail" and "preview". It is then normal that count($sizes)==1 always. So $request is never set.

I dont think $request gives the "size" of the image seen by the user; it is rather a temporay variable to control the loop writing links corresponding to other derivatives that the user may have defined.

But as my previous comments have shown, I am a newbie and I may be wrong.

I have now understood (or rather guessed) what you mean by $request. In fact what is important is:
($_GET['size']) ? $_GET['size'] : 'preview' which gives for example: _original if the user is seeing http://example.com/node/194?size=_original and >preview if the user is seeing http://example.com/node/194.

It is now for me too late to propose a patch. I will try to do it and test it tomorrow.

But there is still the last paragraph of my comment #5 to do and I may be long to find the right patch!

I have modified the code so that a link is not displayed if it will show exactly the same image size as the one currently shown. For example for a small image like an icon the preview and even the thumbnail points to the original file (at least with the fix I propose in drupal.org/node/51600 which must be applied to test the above patch).

For example on my site, preview is 800x600 and any big image which user upload is resized automatically to fit within 800x800 to limit the amount of storage used. In such a case for almost all the "landscape" images the original and the preview are the same. In preview, I think it is better to suppress the link to the original because it will be confusing for the user to get exactly the same page after clicking on it.

I have also replaced the two successives loops which were used to generate the derivative links by one loop. I have found also that _original can never be in _image_get_sizes() (except if the user gives than name to a derivative! which I think will cause other problems). Finally I decided to display the link 'thumbnail' (a user may like to see also what gives the image in that size). Of course it can be easily suppressed by a reviewer of my patch if he does not like that idea).

To be consistent with the link names "thumbnail", "preview", 'label' I have put "original" rather than "view original". One could also choose "view thumbnail", "view preview", ... on the other end. To be decided.

For the problem "user can see original even when not allowed" (mentionned in the last paragraph of comment #5) I have open a new issue: drupal.org/node/54241.

As already explained in my comment #10, I use
if ($node->images[$request] != $node->images[$size['label']]) {
because there are cases where the preview (and even thumbnail) is the original; there is no derivative file generated; the original file is directly referenced by the derivative. For example, there is only one file in files/images if the original is smaller than the thumbnail (at least with my patch in drupal.org/node/51600) not three.

In such a case, when the user is in preview, if we offer him a link "view the original" or "view the thumbnail" he may expect to see something different (bigger or smaller). If he sees exactly the same page he will be a bit frustrated or confused.

Try to upload an image smaller than your thumbnail size (an icon for example) and test.

You certainly will understand by reading the else part in this piece of code extacted from the function _image_build_derivatives :

  foreach ($sizes as $size) {
    if ($size['label'] && $size['width'] && $size['height']) {
      if ($info['width'] > $size['width'] || $info['height'] > $size['height']) {
        $source = file_create_path($node->images['_original']);
        $destination = _image_filename(basename($source), $size['label'], $temp);
        if (!image_scale($source, file_create_path($destination), $size['width'], $size['height'])) {
          drupal_set_message(t('Unable to create %label image', array('%label' => $size['label'])), 'error');
        }
        else {
          $node->images[$size['label']] = $destination;
          if (!$temp) {
            _image_insert($node->nid, $size['label'], file_create_path($destination));
          }
        }
      }
      else {
        $node->images[$size['label']] = $node->images['_original'];
      }
    }
  }

I you think that my patch can be submitted, don't forget to change the status of this issue accordingly.

The simple check checks if the 'size' labels are different.
"My" check checks if the 'size' file name containing the image (.jpeg) are different.
That makes a difference ;-)

Thanks for committing it. Could you, please, review also drupal.org/node/51600