Contrib.social

template_preprocess_image_formatter() doesn't respect signature of image theme function

Open on Drupal.org →
Created on 4 January 2021, over 3 years ago
Updated 8 August 2024, about 1 month ago

Problem/Motivation

The image theme function requires an array for the attributes variable:

'image' => [
      'variables' => ['uri' => NULL, 'width' => NULL, 'height' => NULL, 'alt' => '', 'title' => NULL, 'attributes' => [], 'sizes' => NULL, 'srcset' => [], 'style_name' => NULL],
    ],

But template_preprocess_image_formatter uses its item attributes for the image render array:

function template_preprocess_image_formatter(&$variables) {
  //...
  $variables['image']['#attributes'] = $variables['item_attributes'];
  //...
}

According to the theme definition these can be NULL, which is not valid:

function image_theme() {
  return [
    //...
    'image_formatter' => [
      'variables' => ['item' => NULL, 'item_attributes' => NULL, 'url' => NULL, 'image_style' => NULL],
      'file' => 'image.field.inc',
    ],
  ];
}

Proposed resolution

Cast the attributes to an array in template_preprocess_image_formatter.

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component
Image system  →

Last updated about 11 hours ago

Created by

🇧🇪Belgium Matthijs

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs reroll

    The patch will have to be re-rolled with new suggestions/changes described in the comments in the issue.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    This will need a test case to show the issue and that it's been resolved please

    Thanks!

  • Comment about 1 month ago →
    🇦🇺Australia acbramley

    This popped up as part of BSI.

    It needs a reroll on to an MR as well as the above mentioned tests.

    We could also just NULL coallesce instead of casting to an array.

  • Comment about 1 month ago →
    🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia

    From what I can see, casting to an array would produce an error trying to convert a string or NULL to an array?

    Perhaps we could do something like

    is_array($var) ? $var : [$var] ?

    This needs a test to verify we aren't breaking anything.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024