[4.x]: Setting transformGifs to false means exception thrown instead of non-transformed url returned

[sunscreem]

What happened?

Description

This was introduced in v4.4.14.

cms/src/imagetransforms/ImageTransformer.php

Lines 88 to 90 in e386943

	if ($mimeType === 'image/gif' && !$generalConfig->transformGifs) {
	throw new NotSupportedException('GIF files shouldn’t be transformed.');
	}

I can't find the related PR.

Steps to reproduce

1. Install craft 4.4.14 or greater
2. Create an assets field
3. Upload a gif
4. Set ->transformGifs(false) in config.

Expected behavior

Non transformed url to asset is returned.

Actual behavior

Exception is thrown in transform request URL.

Craft CMS version

4.4.14 or higher

PHP version

n/a

Operating system and version

n/a

Database type and version

n/a

Image driver and version

n/a

Installed plugins and versions

-n/a

[i-just]

Hi, thanks for getting in touch!

The code you linked to was released in Craft 4.5 (related PR: #13321).

I can get to the exception you’ve mentioned if I use version 4.5+ and if, after the steps you mentioned, I e.g. access a template with the following code:

{% set transformTest = {
    width: 100,
    height: 'auto'
} %}

<img src="{{ craft.assets.id(7).one().getUrl(transformTest) }}" />

In that case, the src attribute is empty. I’ve raised a PR to adjust it so that the original asset URL is returned.

I also checked v4.4.14, and I can’t see any related issues there (apart from the one fixed by PR#13321). If you’re having issues on that version, please provide more info on what’s going on and how to replicate.

[sunscreem]

Thank you @i-just - I got my version numbers wrong here for sure. Thanks for looking at this. I've edited the issue to title to reduce confusion :)

[brandonkelly]

Craft 4.5.4 is out with a fix for this.

[carlcs]

Hi, and thanks for the fix!

For some reason I am still running into this when using the GraphQL transform directive directly on the url field. The fix is working when the directive is added to the asset query.

featuredImage {
  url @transform (width: 300)
}

[i-just]

@carlcs, I’m having some trouble getting it to throw an exception. Could you please share an example of what you’re doing that still causes this issue?

[carlcs]

@i-just sorry for giving you a different example to what I was actually doing. It seems to have more to do with using the directive when running asset element queries, and might just be a different issue altogether.

{
  asset(id: "1607") {
    url @transform(width: 300)
  }
}

[i-just]

@carlcs, thanks for the updated snippet. It still works as expected for me, and I can’t get it to throw an exception. At this point, it would be great if you could please open a new issue and confirm which Craft version you’re on, the config you’re using (I assume transformGifs is set to false), the exception you get and any other info that you think would help to replicate this issue.

[carlcs]

@i-just I just reproduced it on a fresh 4.5 install with default configs besides transformGifs disabled. I don‘t know what else I can say to help reproduce on your side. I initially forgot that I had to test with a GIF, so maybe this is something you overlooked as well?

[i-just]

Well, I was hung up on trying to get it to throw an exception and in the new issue you raised, you clearly said it’s returning null instead of the non-transformed URL. I think we’re on the same page now! It just took me a while to get there 🤦‍♀️

[carlcs]

I’m sorry @i-just, this one is clearly on me. While I did see that you all were talking about an exception, I thought that it must be caught and just be the cause for null being returned.