theme_image_formatter should pass along attributes

Would be nice if we just used straght up $variables there instead of having to switch over to $image for Drupal 8. Save us a couple lines of code. For Drupal 7 though, it looks fine.

#1025796: Rename path to uri in image functions was committed, so this might not be needed for Drupal 8. For Drupal 7 though? Bot?

I'm thinking we should be checking $item['attributes'] as well as $variable['attributes'] ... if I'm working with upstream code that sets a 'title', 'alt', and 'fid' attribute on an image and then returns that image, these all get tacked on to $item.

If I want it to appear in $variables['#attributes'] for proper rendering with this patch, I would then have to pull the 'fid' out of $item and plug it into $variables['#attributes'] later in the pre-process function. This feels hackish to me.

That said, I can also see the benefit of being able to tack on '#attributes' when defining the render array.

How about something like this?

$image['attributes'] = array_merge(
  (isset($item['attributes']) ? $item['attributes'] : array(),
  (isset($variables['attributes']) ? $variables['attributes'] : array()
);

What if it were to JUST check $item['attributes']? Looking at theme_image_formatter(), we see "item: An array of image data."..... Then looking at theme_image(), we see that the image data is along side the "attributes".....

Actually, I agree that we probably don't want to override it if it already exists... hmm.

#10: drupal-1329586-10.patch queued for re-testing.

#10 works great. I think it's time to RTBC.

1. We need some documentation of that newly accepted parameter.
2. Let's also add a test for this.

1. We need some documentation of that newly accepted parameter.

It's technically not a new parameter, as it was expected in theme_image(), but absolutely ignored in theme_image_formatter(). I'm with you that more documentation never hurts though!

2. Let's also add a test for this.

Explicit test added!

And just for the sake of completeness ...

#17: image-attributes-testonly.patch queued for re-testing.

The patch is missing the $item['attributes'] updates from #14.

re-upped.

Alright.. this is definitely ready to go.. good documentation, a test which has been shown to fail in #17 and a fully functional test-passing patch in #22.

RobLoach: That was intentional. (ie. test-only). ;)

Hmm, can someone confirm that it's not needed in D8 (as per #5)?

Hmm, can someone confirm that it's not needed in D8 (as per #5)?

Correct, Drupal 8 got the more elegant solution over at #1025796: Rename path to uri in image functions, and has been in since November. Drupal 7 just gets this simple $item['attributes'] check. I posted the difference between the branched patches right here.

RobLoach: That was intentional. (ie. test-only). ;)

Ah, proof that this patch is needed :-) .

Cool, that looks great!

Committed and pushed to 7.x. Yay!

Does this patch also include the title attribute? I have the title attribute set but I don't see it in the output.

False alarm. It was a JS issue.