Image styles don't add width/height attributes [#1012416]

Image styles theme function theme_image_style() uses $variables['getsize'] = TRUE to determine width/height
But this feature was removed from theme_image() in favour of explicitly passing in $variables['width'] and $variables['height']

This causes problems such as #1012324: Images don't display fully when visiting the view's URL

The following adds this functionality back:

function theme_image_style($variables) {
  $style_name = $variables['style_name'];
  $path = $variables['path'];

  // theme_image() can only honor the $getsize parameter with local file paths.
  // The derivative image is not created until it has been requested so the file
  // may not yet exist, in this case we just fallback to the URL.
  $style_path = image_style_path($style_name, $path);
  if (!file_exists($style_path)) {
    $style_path = image_style_url($style_name, $path);
  }
  $variables['path'] = $style_path;

  if ($variables['getsize'] && (is_file($style_path) && (list($width, $height, $type, $attributes) = @getimagesize($style_path)))) {
    $variables['width'] = $width;
    $variables['height'] = $height;
  }

  return theme('image', $variables);
}

Comment	File	Size	Author
#6	1012416-getsize-4.patch	1.85 KB	redndahead
#5	1012416-getsize-3.patch	1.85 KB	redndahead
#3	1012416-getsize-2.patch	1.62 KB	redndahead
#1	1012416-getsize-1.patch	851 bytes	redndahead

Comments

Comment #1

redndahead commented 3 January 2011 at 00:54

Status:

Needs work

» Needs review

Status	File	Size
new	1012416-getsize-1.patch	851 bytes

Here is the patch

Comment #2

3 January 2011 at 01:31

Status:

Needs review

» Needs work

The last submitted patch, 1012416-getsize-1.patch, failed testing.

Comment #3

redndahead commented 3 January 2011 at 21:17

Status:

Needs work

» Needs review

Status	File	Size
new	1012416-getsize-2.patch	1.62 KB

Test weren't passing because during render getsize was set to false when the default value is true. So output wasn't matching. Fixed the test.

Comment #4

3 January 2011 at 22:10

Status:

Needs review

» Needs work

The last submitted patch, 1012416-getsize-2.patch, failed testing.

Comment #5

redndahead commented 4 January 2011 at 00:51

Status:

Needs work

» Needs review

Status	File	Size
new	1012416-getsize-3.patch	1.85 KB

Let's try this one. Need to use theme('image_style' since that is what is being used to display the image on the node. This also takes different variables.

Comment #6

redndahead commented 4 January 2011 at 01:38

Status	File	Size
new	1012416-getsize-4.patch	1.85 KB

This patch just removes extra white spacing. Will still pass tests.

Comment #7

redndahead commented 4 January 2011 at 02:10

If you want to see a place where the change is made you can add an image to a user profile. Before the patch the image will not have the width and height attributes on them. After the patch they will be there. This should also be the case with an image field.

You can also see where this existed in imagefield module before being incorporated into d7. On line 328:
http://drupalcode.org/viewvc/drupal/contributions/modules/imagefield/ima...

Comment #8

threewestwinds commented 4 January 2011 at 03:05

Status:

Needs review

» Reviewed & tested by the community

Works great for me, and code looks good. The change to the test has no functional change - it just uses the API properly instead of unnecessarily doing a manual step. Let's make stuff work like it's supposed to.

Comment #9

webchick

she/they

English

Vancouver 🇨🇦

commented 4 January 2011 at 03:35

Status:

Reviewed & tested by the community

» Closed (duplicate)

Ok, I knew I recognized this issue.

See #908282: Remove unnecessary I/O from theme_image() where this was removed, on purpose. :( :( We'll need to get some reviews from the folks over there in order to add this back, though I agree it's a major WTF right now that we have a function argument that completely lies.

Comment #10

sun

German

Karlsruhe

commented 4 January 2011 at 03:52

Status:

Closed (duplicate)

» Closed (works as designed)

Reasoning for why getsize doesn't work for image styles:
#908282-15: Remove unnecessary I/O from theme_image()

Comment #11

eric_a commented 5 January 2011 at 05:34

In the other issue I just suggested that we need a new render element type.
I feel that code like this really should be in a preprocessor or in a render element type. The point being that theme functions and templates are for markup and overriding markup, basically.