Issue status update for http://drupal.org/node/21397 Project: Drupal Version: cvs Component: theme system Category: bug reports Priority: critical Assigned to: Anonymous Reported by: jjeff Updated by: Dries Status: patch AFAIK, we always use $attributes = NULL without 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. Dries Previous comments: ------------------------------------------------------------------------ April 27, 2005 - 04:45 : jjeff 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 ------------------------------------------------------------------------ May 2, 2005 - 17:44 : jjeff Attachment: http://drupal.org/files/issues/theme_image-attr_fix.patch (1.51 KB) 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 ------------------------------------------------------------------------ May 2, 2005 - 17:47 : jjeff Attachment: http://drupal.org/files/issues/theme_image-attr_fix_0.patch (1.51 KB) Whoops... Found a mistake in my code... Here's the correction. -Jeff ------------------------------------------------------------------------ May 2, 2005 - 18:04 : jjeff Attachment: http://drupal.org/files/issues/theme_image-attr_fix_1.patch (1.51 KB) It's amazing how many mistakes one person can make in two lines of code. New patch... this one works! -jeff ------------------------------------------------------------------------ May 2, 2005 - 18:16 : Steven Attributes should be consistently passed as an array, and rendered with drupal_attributes(). This is to ensure data is escaped correctly. ------------------------------------------------------------------------ May 5, 2005 - 17:09 : jjeff Attachment: http://drupal.org/files/issues/theme_image-attr_fix_2.patch (1.34 KB) Now the $attr argument requires an array. And the $attr argument gets sent through drupal_attributes(). Simple and easy, really. -Jeff ------------------------------------------------------------------------ May 6, 2005 - 11:02 : Dries Tidied up the code a little, and committed it to HEAD and DRUPAL-4-6. Thanks. ------------------------------------------------------------------------ May 8, 2005 - 18:26 : jjeff Attachment: http://drupal.org/files/issues/theme-img_attr_fix.patch (1 KB) 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