http://api.drupal.org/api/function/theme_image/5

It might not be completely obvious to all that in order to specify a URL as the image path, the 5th argument needs to be set to TRUE. The documentation does not currently specify this.

Files: 
CommentFileSizeAuthor
#20 theme_image_getsize-new4-225950-D6.patch902 bytesdaniels220
#18 theme_image_getsize-new3-225950-D6.patch896 bytesdaniels220
#16 theme_image_getsize-new2-225950-D6.patch630 bytesdaniels220
#13 theme_image-getsize-new-225950-D6.patch795 bytesdaniels220
#11 theme_image-getsize-225950-D6.patch808 bytesdaniels220
#6 theme_image-getsize_doc-D7-225950-5.patch710 bytesmgriego
PASSED: [[SimpleTest]]: [MySQL] 19,591 pass(es).
[ View ]
#6 theme_image-getsize_doc-D6-225950-5.patch637 bytesmgriego
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch theme_image-getsize_doc-D6-225950-5.patch.
[ View ]
#6 theme_image-getsize_doc-D5-225950-5.patch633 bytesmgriego
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch theme_image-getsize_doc-D5-225950-5.patch.
[ View ]

Comments

Title:Need more clarity on how to specify image url to theme_image()Improve documentation for how to specify image url to theme_image()
Project:Drupal.org infrastructure» Drupal core
Version:» 7.x-dev
Component:Other» theme system
Category:feature» task

Can we get some love on this?

Version:7.x-dev» 6.x-dev
Component:theme system» documentation

Moving to documentation component, and moving down to D6. I think #238681: theme_image() not evaluating properly? is a better approach for D7.

Version:6.x-dev» 7.x-dev
Category:task» bug
Priority:Minor» Normal

Looking at the code, it looks like in Drupal 5/6, theme_image() only works if either getsize is FALSE or if the URL is relative to the base path and it can figure out the image dimensions. The original report is wrong, IMO -- it needs to be $getsize=FALSE to work with absolute URLs.

It is also still working the same in Drupal 7. The issue mentioned above has been closed as a "won't fix".

So this needs to be documented in Drupal 7, 6, and 5.

Issue tags:+Novice

Could be a good novice project.

Status:Active» Needs review
StatusFileSize
new633 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch theme_image-getsize_doc-D5-225950-5.patch.
[ View ]
new637 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch theme_image-getsize_doc-D6-225950-5.patch.
[ View ]
new710 bytes
PASSED: [[SimpleTest]]: [MySQL] 19,591 pass(es).
[ View ]

I assume this is what you're looking for... My reading of the code was the same that $getsize needs to be false for fully qualified URLs to work, so I added the appropriate parameter comments for D5/6/7.

Status:Needs review» Needs work

The last submitted patch, theme_image-getsize_doc-D5-225950-5.patch, failed testing.

Can you rework this a bit? (I"m talking about the D7 patch, didn't look at the others yet -- let's get that one right first):
- We don't usually use all-caps -- it's a bit shouty. Just say "This must be set..." rather than "This MUST be set".
- There is no $path in the D7 args - it's a component of the variables array
- We should probably mention on the 'path' component as well that you need to set the 'getsize' component to FALSE if you use a full URL

Also, please read the instructions below the file attach about file names for D6/D5, which will avoid having the test bot complain.

Version:7.x-dev» 6.x-dev

This doesn't make sense anymore for D7 because the $getsize component was removed in #908282: Remove unnecessary I/O from theme_image().

Thanks for checking into that -- you are correct. So we just need a Drupal 6 patch.

Assigned:Unassigned» daniels220
StatusFileSize
new808 bytes

Here's my stab at a D6 patch.

+ *   If this is a full URL, $getsize must be set to FALSE

Missing a . at the end of the sentence.

+ *   $getsize must be set to false if $path is a full URL.

false -> FALSE

Also, when you attach a patch, please set the status to "needs review", so that we know there's something to review, thanks!

Status:Needs work» Needs review
StatusFileSize
new795 bytes

Thanks for the feedback. New patch attached.

Status:Needs review» Needs work

patch header needs fixing - wrong file names, can't apply

Actually, the text here is not correct. You don't need to set $get_size to FALSE if a URL is used, you need to supply the sizes yourself because $get_size will be ignored... correct?

Status:Needs work» Needs review
StatusFileSize
new630 bytes

Not sure on the wording for $getsize, but I noted that it is ignored if $path is a full URL.

Ugh, my mistake.

I looked closer at the code and I think you do need to set $get_size to FALSE, because if it's set to TRUE and is_file() returns FALSE, nothing will get printed. Yuck! who wrote this code?!?

StatusFileSize
new896 bytes

You are indeed right. New patch attached with slightly more noticeable warnings about needing to set $getsize = false.

Status:Needs review» Needs work

Hmmm.

   *   Either the path of the image file (relative to base_path()) or a full URL.
+ *   If this is a full url, $getsize must be set to FALSE or an empty <img> tag
+ *   will be returned.

- In 2nd sentence, url -> URL.
- I don't think that 2nd sentence is accurate. It looks to me like if $getsize is TRUE, nothing at all is returned (not an empty IMG tag).

  * @param $getsize
  *   If set to TRUE, the image's dimension are fetched and added as width/height attributes.
+ *   Must be set to FALSE if $path is a full URL.
  * @return

Maybe this should mention that TRUE is the default? Also, there should be a blank line between the @param and @return.

Status:Needs work» Needs review
StatusFileSize
new902 bytes

Fixed all that.

Status:Needs review» Reviewed & tested by the community

Now we're talking. Looks good. Thanks for the repeat patching.

Status:Reviewed & tested by the community» Fixed

Great, committed, thanks.

Status:Fixed» Closed (fixed)
Issue tags:-Novice

Automatically closed -- issue fixed for 2 weeks with no activity.