ImageStyle::createDerivative should return fail if one of the style's effects fails

Created on 7 December 2015, over 9 years ago
Updated 18 February 2025, about 2 months ago

Follow-up to #2359443: Allow creating image derivatives from an Image object

Problem/Motivation

From #2359443-17: Allow creating image derivatives from an Image object :

+++ b/core/modules/image/src/Entity/ImageStyle.php
@@ -309,6 +320,23 @@ public function createDerivative($original_uri, $derivative_uri) {
+    // Apply the effects to the image object.
+    foreach ($this->getEffects() as $effect) {
+      $effect->applyEffect($image);
+    }
...
+    return TRUE;

Only now I see this crap. So ImageEffectInterface::applyEffect() does return a success flag but we are not using it here. We just return TRUE regardless if the effects were applied correct. Is there a good reason for this? I don't remember. If there's a reason, we need to document it, otherwise we need something like:
if (!$effect->applyEffect($image)) {
  return FALSE;
}

Proposed resolution

Remaining tasks

  • discuss what to do
  • write a patch

User interface changes

API changes

🐛 Bug report
Status

Needs work

Version

11.0 🔥

Component

image.module

Created by

🇮🇹Italy mondrake 🇮🇹

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 issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

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.

  • 🇺🇸United States smustgrave

    This came up as a daily bugsmash target.

    From what I can tell the code is still the same. But I also wonder if this is a task or feature request vs a bug?

Production build 0.71.5 2024