theme_image doesn't accept html attributes
jjeff - April 27, 2005 - 02:45
| Project: | Drupal |
| Component: | theme system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Description
Here's the theme_image function from theme.inc
<?php
function theme_image($path, $alt = '', $title = '', $attr = '', $getsize = true) {
if (!$getsize || (file_exists($path) && (list($width, $height, $type, $attr) = @getimagesize($path)))) {
return "<img src=\"$path\" $attr alt=\"$alt\" title=\"$title\" />";
}
}
?>Any arguments sent to the function for $attr get clobbered by the getimagesize() call. And $getsize defaults to true, so in a call to:
<?php
theme_image('files/images/foo', 'alt_text', 'title_text', 'align="left"')
?>the 'align="left"' never makes it to the page. The two attribute values really should be able to coexist, allowing for both getsize attributes and additional 'argumented' attributes as well.
I recommend changing the $attr in the second line to $imgattr, then adding $imgattr to the tag returned in the third.
-Jeff
#1
Okay, here's a patch to fix the theme_image attribute problem.
I've also added a handler so that attributes can either be a string (as they were in the past - although it wasn't working) or an associative array.
I've marked this issue as critical because the current version of theme_image() does not accept attributes at all.
-Jeff
#2
Whoops... Found a mistake in my code...
Here's the correction.
-Jeff
#3
It's amazing how many mistakes one person can make in two lines of code.
New patch... this one works!
-jeff
#4
Attributes should be consistently passed as an array, and rendered with drupal_attributes(). This is to ensure data is escaped correctly.
#5
Now the $attr argument requires an array.
And the $attr argument gets sent through drupal_attributes().
Simple and easy, really.
-Jeff
#6
Tidied up the code a little, and committed it to HEAD and DRUPAL-4-6. Thanks.
#7
Oops... the new fix sets the default value for $attributes to NULL. It should at least be array(), now that $attributes is an array.
This patch fixes the default value and adds some other error checking that should help this error that people may be seeing when they load a page with images:
Invalid argument supplied for foreach() in /Users/jeff/Sites/drupal/includes/common.inc on line 1536
-Jeff
#8
AFAIK, we always use
$attributes = NULLwithout special error-checking (see form_ functions in common.inc, for example). Does this fix a bug? If so, how can we reproduce it? I'd love to test it.#9
I also get this error:
Invalid argument supplied for foreach() in /includes/common.inc on line 1536
When using image.module CVS and drupal CVS. When viewing an image no tag is outputted.
#10
Do you still get that error? If it is specific to the image.module, we might have to move this bug report.
#11
#12
#13
Why is the if statement still present in version
$Id: theme.inc,v 1.228.2.8 2005-08-17 20:03:28 dries Exp
when nothing is done with the results of the call to getimagesize?