Function theme_image() doesn't actually return a width and height for an image like it claims to do.

CommentFileSizeAuthor
#8 system.module_10.patch1.53 KBm3avrck
#3 drupal_13.patch2.75 KBm3avrck
#2 drupal_12.patch2.75 KBm3avrck
#1 drupal_11.patch2.74 KBm3avrck

Comments

m3avrck’s picture

m3avrck commented
Status: Active » Needs review
StatusFileSize
new drupal_11.patch2.74 KB

Ok patched attached, which fixes this issue. Also, included a patch for system.module which sets the screen shots to 'TRUE' so image dimensions will also be outputted there as well (which they should be!).

m3avrck’s picture

m3avrck commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new drupal_12.patch2.75 KB

Fixed a tab issue.

m3avrck’s picture

m3avrck commented
StatusFileSize
new drupal_13.patch2.75 KB

Fixed a spacing issue.

Souvent22’s picture

Souvent22 commented

Used the patch, and did a quick test. Worked well for me. +1.

Robrecht Jacques’s picture

Robrecht Jacques commented

I don't see why this patch is needed, "theme_image" returns a img tag with the width and height set if $getsize = TRUE.
Eg:

$node->body = theme('image', file_create_path('druplicon.png'), 'no alt', 'no title', array(), TRUE) .
              theme('image', file_create_path('druplicon.png'), 'no alt', 'no title', array(), FALSE)

will return:

<img src="files/druplicon.png" alt="no alt" title="no title" width="88" height="100" />
<img src="files/druplicon.png" alt="no alt" title="no title"  />

(if druplicon.png is copied to the files/ directory).

I don't see what you are fixing...

You are right about the use of theme('image') in system.module though. The "false" should be "true".

m3avrck’s picture

m3avrck commented

Well the actual code in theme_image() returned this:

return '<img src="'. check_url($path) .'" alt="'. check_plain($alt) .'" title="'. check_plain($title) .'" '. $image_attributes . $attributes .'/>';

There is *no* mention of the $width and $height variables that are assigned above, not used at all. I checked images in the themes directory and none had this information, unless I was missing something obvious, I see no way that is being generated... nothing in the above img src about it.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Taken from http://php.net/getimagesize: "Index 3 is a text string with the correct height="yyy" width="xxx" string that can be used directly in an IMG tag.". $image_attributes contains this information. Marking this fixed. Please reopen if not.

m3avrck’s picture

m3avrck commented
Status: Fixed » Reviewed & tested by the community
StatusFileSize
new system.module_10.patch1.53 KB

Dries, great catch! What prompted this originally was that the screenshots on the theme page didn't have dimensions, didn't realize that index [3] returned this. Anyways, this patch fixes the screen shots and adds widths/heights.

Robrecht Jacques’s picture

Robrecht Jacques commented

This is correct, the theme('image') for the screenshots need to have TRUE as last parameter, or omit the parameter (like the patch does). This last patch is ok. Didn't test it, but I'm sure it is correct. Without the width and height is will work too (it has before), but this is cleaner HTML.

Patch is ready to commit.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks.

Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)