ImageCropperTemplateExtensions - GetCropUrl with cropAlias and imageCropMode ignores cropAlias width and height.

[rndfm]

When using the GetCropUrl and specifying both cropAlias and imageCropMode the width and height from the cropAlias is not included in the generated url.
This is true for all ImageCropMode's. But ImageCropMode.Crop in combination with useCropDimensions works.
But I would like to use the ImageCropMode.Max with an cropAlias.

Bug also mentioned in this post from 2017: https://our.umbraco.com/forum/extending-umbraco-and-using-the-api/84823-getcropurl-with-imagecropmode

Umbraco version

I am seeing this issue on Umbraco version: 8.9.1 and in the latest source in /contrib branch.

Reproduction

Use the GetCropUrl function:

media.GetCropUrl(cropAlias: "AliasGoesHere", imageCropMode: ImageCropMode.Crop);
Observe that the width and height from the cropAlias is not included in the url.

media.GetCropUrl(cropAlias: "AliasGoesHere", imageCropMode: ImageCropMode.Crop, useCropDimensions: true);
Observe that the width and height is now present.

media.GetCropUrl(cropAlias: "AliasGoesHere", imageCropMode: ImageCropMode.Max, useCropDimensions: true);
Observe that width and height is missing again.

Expected result

Width, height should be included in the crop url.

Actual result

Width, height is missing in some configurations.

[emmaburstow]

Hey @rndfm

Thanks for reporting this issue - we'd like to see if the community have any insight into what's causing this it. Any workarounds that folk have come up with might help us pinpoint it.

So with that in mind I'm going to label up-for-grabs but what we're really after is some diagnosis and discussion before anyone rushes off to create a PR.

Emma

[umbrabot]

Hi @rndfm,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post

Thanks muchly, from your friendly Umbraco GitHub bot :-)

[patrickdemooij9]

I always thought that this was intentional. Could you try adding the 'useCropDimensions: true' parameter?

[rndfm]

Ok. I see why this is happening.
When using the "simple" function GetCropUrl parsing only the cropAlias the parameter useCropDimensions is defaulted to true. And therefore the width and height from the cropAlias is used.

When adding the second parameter imageCropMode another function(overload) is used and this time useCropDimensions is defaulted to false. And therefore the cropAlias width and height is not used.

This could make some kind of sense because the second function also have separate width and height parameters. Those are optional as all of the parameters in the second function is.

public static string GetCropUrl(
             this IPublishedContent mediaItem,
             int? width = null,
             int? height = null,
             string propertyAlias = Constants.Conventions.Media.File,
             string cropAlias = null,
             int? quality = null,
             ImageCropMode? imageCropMode = null,
             ImageCropAnchor? imageCropAnchor = null,
             bool preferFocalPoint = false,
             bool useCropDimensions = false,
             bool cacheBuster = true,
             string furtherOptions = null,
             ImageCropRatioMode? ratioMode = null,
             bool upScale = true)=> ImageCropperTemplateCoreExtensions.GetCropUrl(mediaItem, Current.ImageUrlGenerator, width, height, propertyAlias, cropAlias, quality, imageCropMode, imageCropAnchor, preferFocalPoint, useCropDimensions, cacheBuster, furtherOptions, ratioMode, upScale);

What would had made real sense was:
if a cropAlias is defined. Use it.
if width and or height is defined. They should override the width and height from the cropAlias.

[rndfm]

I always thought that this was intentional. Could you try adding the 'useCropDimensions: true' parameter?

Might be intentional but its not intuitive :)

[rndfm]

After some more digging i found that useCropDimensions does have an effect if used in combination with imageCropMode = ImageCropMode.Crop
If some other imageCropMode is used the useCropDimensions bool is ignored.

I my case i would like to use the "max" crop mode together with an crop alias.

I would really like to suggest some changes in the GetCropUrl function:

• It should be possible to use CropAlias height and width with all image crop modes.
• When cropAlias is defined use the width and height from the cropAlias.
• If width and height parameters are defined they should override width and height from cropAlias.
• Keep useCropDimensions but mark it as obsolete.
• Until useCropDimensions is removed: When useCropDimensions is set to true, width and height parameters should not override cropAlias height and width.

This would change the behavior to a more predictable behavior, enable us to use all crop modes with crop aliases and still be backward compatible.

Let me know what you think and I can put a pull request together.

[rndfm]

Should i proceed with the suggested changes?

[mistyn8]

According to this post on v8 #8116...
media.GetCropUrl() should be obsolete?
instead using @Url.GetCropUrl(media, ...)

But even with that I'm still having lots of issues to get the frontend to render a crop exactly as the backoffice shows it :-(

[mistyn8]

I'm sure I used to be able to just do
media.GetCropUrl("CROPALIAS", width:200) and that would crop the image first to the aspect ratio from the back office and then resize to 200 wide maintaining the aspect ratio...

[umbrabot]

Hiya @rndfm,

Just wanted to let you know that we noticed that this issue got a bit stale and might not be relevant any more.

We will close this issue for now but we're happy to open it up again if you think it's still relevant (for example: it's a feature request that's not yet implemented, or it's a bug that's not yet been fixed).

To open it this issue up again, you can write @umbrabot still relevant in a new comment as the first line. It would be super helpful for us if on the next line you could let us know why you think it's still relevant.

For example:

@umbrabot still relevant
This bug can still be reproduced in version x.y.z

This will reopen the issue in the next few hours.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂