theme_image() doesn't actually output width and height of image [#30935]

Comment	File	Size	Author
#8	system.module_10.patch	1.53 KB	m3avrck

#3	drupal_13.patch	2.75 KB	m3avrck

#2	drupal_12.patch	2.75 KB	m3avrck

#1	drupal_11.patch	2.74 KB	m3avrck

Comment #1

m3avrck CreditAttribution: m3avrck commented 11 September 2005 at 19:30

Status:

Active

» Needs review

File	Size
drupal_11.patch	2.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!).

Log in or register to post comments

Comment #2

m3avrck CreditAttribution: m3avrck commented 11 September 2005 at 19:33

Status:

Needs review

» Reviewed & tested by the community

File	Size
drupal_12.patch	2.75 KB

Fixed a tab issue.

Log in or register to post comments

Comment #3

m3avrck CreditAttribution: m3avrck commented 11 September 2005 at 19:35

File	Size
drupal_13.patch	2.75 KB

Fixed a spacing issue.

Log in or register to post comments

Comment #4

Souvent22 CreditAttribution: Souvent22 commented 12 September 2005 at 14:18

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

Log in or register to post comments

Comment #5

Robrecht Jacques CreditAttribution: Robrecht Jacques commented 12 September 2005 at 16:21

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".

Log in or register to post comments

Comment #6

m3avrck CreditAttribution: m3avrck commented 12 September 2005 at 16:40

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.

Log in or register to post comments

Comment #7

Dries CreditAttribution: Dries commented 12 September 2005 at 18:41

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.

Log in or register to post comments

Comment #8

m3avrck CreditAttribution: m3avrck commented 12 September 2005 at 18:49

Status:

Fixed

» Reviewed & tested by the community

File	Size
system.module_10.patch	1.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.

Log in or register to post comments

Comment #9

Robrecht Jacques CreditAttribution: Robrecht Jacques commented 12 September 2005 at 21:51

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.