From 50d8e74b5bb3fb8af09a49729d855f2ea26ed6c1 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 9 Aug 2021 15:52:55 +0200 Subject: [PATCH] Obsolete unsupported image URL generation options and update ImageSharp implementation --- .../Models/ImageUrlGenerationOptions.cs | 45 +++++-- .../Media/ImageSharpImageUrlGenerator.cs | 87 +++++-------- .../Media/ImageSharpImageUrlGeneratorTests.cs | 77 ++--------- .../Umbraco.Web.Common/ImageCropperTest.cs | 120 ++++++++---------- .../ImageCropperTemplateCoreExtensions.cs | 45 ++++--- 5 files changed, 159 insertions(+), 215 deletions(-) diff --git a/src/Umbraco.Core/Models/ImageUrlGenerationOptions.cs b/src/Umbraco.Core/Models/ImageUrlGenerationOptions.cs index 99f42bbefa..3118162786 100644 --- a/src/Umbraco.Core/Models/ImageUrlGenerationOptions.cs +++ b/src/Umbraco.Core/Models/ImageUrlGenerationOptions.cs @@ -1,55 +1,71 @@ -namespace Umbraco.Cms.Core.Models +using System; + +namespace Umbraco.Cms.Core.Models { /// - /// These are options that are passed to the IImageUrlGenerator implementation to determine - /// the propery URL that is needed + /// These are options that are passed to the IImageUrlGenerator implementation to determine the URL that is generated. /// public class ImageUrlGenerationOptions { - public ImageUrlGenerationOptions (string imageUrl) - { - ImageUrl = imageUrl; - } + public ImageUrlGenerationOptions(string imageUrl) => ImageUrl = imageUrl; public string ImageUrl { get; } + public int? Width { get; set; } + public int? Height { get; set; } + + [Obsolete("This property is unsupported by the default implementation, manually calculate the width based on the height instead.")] public decimal? WidthRatio { get; set; } + + [Obsolete("This property is unsupported by the default implementation, manually calculate the height based on the width instead.")] public decimal? HeightRatio { get; set; } + public int? Quality { get; set; } + public ImageCropMode? ImageCropMode { get; set; } + public ImageCropAnchor? ImageCropAnchor { get; set; } + + [Obsolete("Images are already cropped from the center to the specified width/height by default.")] public bool DefaultCrop { get; set; } + public FocalPointPosition FocalPoint { get; set; } + public CropCoordinates Crop { get; set; } + public string CacheBusterValue { get; set; } + public string FurtherOptions { get; set; } + + [Obsolete("This property is unsupported by the default implementation, images should always be resized to the specified dimensions (within the configured maximums) to prevent different sizes depending on the source image.")] public bool UpScale { get; set; } = true; + + [Obsolete("This property is unsupported by the default implementation, all frames should be processed by default.")] public string AnimationProcessMode { get; set; } /// - /// The focal point position, in whatever units the registered IImageUrlGenerator uses, - /// typically a percentage of the total image from 0.0 to 1.0. + /// The focal point position, in whatever units the registered IImageUrlGenerator uses, typically a percentage of the total image from 0.0 to 1.0. /// public class FocalPointPosition { - public FocalPointPosition (decimal top, decimal left) + public FocalPointPosition(decimal top, decimal left) { Left = left; Top = top; } public decimal Left { get; } + public decimal Top { get; } } /// - /// The bounds of the crop within the original image, in whatever units the registered - /// IImageUrlGenerator uses, typically a percentage between 0 and 100. + /// The bounds of the crop within the original image, in whatever units the registered IImageUrlGenerator uses, typically a percentage between 0.0 and 1.0. /// public class CropCoordinates { - public CropCoordinates (decimal x1, decimal y1, decimal x2, decimal y2) + public CropCoordinates(decimal x1, decimal y1, decimal x2, decimal y2) { X1 = x1; Y1 = y1; @@ -58,8 +74,11 @@ } public decimal X1 { get; } + public decimal Y1 { get; } + public decimal X2 { get; } + public decimal Y2 { get; } } } diff --git a/src/Umbraco.Infrastructure/Media/ImageSharpImageUrlGenerator.cs b/src/Umbraco.Infrastructure/Media/ImageSharpImageUrlGenerator.cs index d72ee2b4eb..8e0a8eb350 100644 --- a/src/Umbraco.Infrastructure/Media/ImageSharpImageUrlGenerator.cs +++ b/src/Umbraco.Infrastructure/Media/ImageSharpImageUrlGenerator.cs @@ -1,22 +1,19 @@ +using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; using System.Text; +using SixLabors.ImageSharp; using Umbraco.Cms.Core.Media; using Umbraco.Cms.Core.Models; -using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Media { public class ImageSharpImageUrlGenerator : IImageUrlGenerator { - public IEnumerable SupportedImageFileTypes => new[] - { - "jpeg", - "jpg", - "gif", - "bmp", - "png" - }; + private static readonly string[] s_supportedImageFileTypes = Configuration.Default.ImageFormats.SelectMany(f => f.FileExtensions).ToArray(); + + public IEnumerable SupportedImageFileTypes { get; } = s_supportedImageFileTypes; public string GetImageUrl(ImageUrlGenerationOptions options) { @@ -27,84 +24,66 @@ namespace Umbraco.Cms.Infrastructure.Media var imageUrl = new StringBuilder(options.ImageUrl); + bool queryStringHasStarted = false; + void AppendQueryString(string value) + { + imageUrl.Append(queryStringHasStarted ? '&' : '?'); + queryStringHasStarted = true; + + imageUrl.Append(value); + } + void AddQueryString(string key, params IConvertible[] values) + => AppendQueryString(key + '=' + string.Join(",", values.Select(x => x.ToString(CultureInfo.InvariantCulture)))); + if (options.FocalPoint != null) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "?rxy={0},{1}", options.FocalPoint.Top, options.FocalPoint.Left); - } - else if (options.Crop != null) - { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "?crop={0},{1},{2},{3}&cropmode=percentage", options.Crop.X1, options.Crop.Y1, options.Crop.X2, options.Crop.Y2); - } - else if (options.DefaultCrop) - { - imageUrl.Append("?ranchor=center&rmode=crop"); - } - else - { - imageUrl.Append("?rmode=").Append((options.ImageCropMode ?? ImageCropMode.Crop).ToString().ToLowerInvariant()); - - if (options.ImageCropAnchor != null) - { - imageUrl.Append("&ranchor=").Append(options.ImageCropAnchor.ToString().ToLowerInvariant()); - } + AddQueryString("rxy", options.FocalPoint.Top, options.FocalPoint.Left); } - var hasFormat = options.FurtherOptions != null && options.FurtherOptions.InvariantContains("&format="); - - // Only put quality here, if we don't have a format specified. - // Otherwise we need to put quality at the end to avoid it being overridden by the format. - if (options.Quality.HasValue && hasFormat == false) + if (options.Crop != null) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&quality={0}", options.Quality.Value); + AddQueryString("crop", options.Crop.X1, options.Crop.Y1, options.Crop.X2, options.Crop.Y2); + AddQueryString("cropmode", "percentage"); } - if (options.HeightRatio.HasValue) + if (options.ImageCropMode.HasValue) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&heightratio={0}", options.HeightRatio.Value); + AddQueryString("rmode", options.ImageCropMode.Value.ToString().ToLowerInvariant()); } - if (options.WidthRatio.HasValue) + if (options.ImageCropAnchor.HasValue) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&widthratio={0}", options.WidthRatio.Value); + AddQueryString("ranchor", options.ImageCropAnchor.Value.ToString().ToLowerInvariant()); } if (options.Width.HasValue) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&width={0}", options.Width.Value); + AddQueryString("width", options.Width.Value); } if (options.Height.HasValue) { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&height={0}", options.Height.Value); + AddQueryString("height", options.Height.Value); } - if (options.UpScale == false) + if (options.Quality.HasValue) { - imageUrl.Append("&upscale=false"); - } - - if (!string.IsNullOrWhiteSpace(options.AnimationProcessMode)) - { - imageUrl.Append("&animationprocessmode=").Append(options.AnimationProcessMode); + AddQueryString("quality", options.Quality.Value); } if (!string.IsNullOrWhiteSpace(options.FurtherOptions)) { - imageUrl.Append(options.FurtherOptions); - } - - // If furtherOptions contains a format, we need to put the quality after the format. - if (options.Quality.HasValue && hasFormat) - { - imageUrl.AppendFormat(CultureInfo.InvariantCulture, "&quality={0}", options.Quality.Value); + AppendQueryString(options.FurtherOptions.TrimStart('?', '&')); } if (!string.IsNullOrWhiteSpace(options.CacheBusterValue)) { - imageUrl.Append("&rnd=").Append(options.CacheBusterValue); + AddQueryString("rnd", options.CacheBusterValue); } return imageUrl.ToString(); } + + } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Media/ImageSharpImageUrlGeneratorTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Media/ImageSharpImageUrlGeneratorTests.cs index c57c53b52e..f6d87ef5a1 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Media/ImageSharpImageUrlGeneratorTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Media/ImageSharpImageUrlGeneratorTests.cs @@ -45,7 +45,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media } /// - /// Test that if a crop alias has been specified that doesn't exist the method returns null + /// Test that if options is null, the generated image URL is also null. /// [Test] public void GetCropUrlNullTest() @@ -55,13 +55,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media } /// - /// Test that if a crop alias has been specified that doesn't exist the method returns null + /// Test that if the image URL is null, the generated image URL is empty. /// [Test] public void GetCropUrlEmptyTest() { var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(null)); - Assert.AreEqual("?rmode=crop", urlString); + Assert.AreEqual(string.Empty, urlString); } /// @@ -74,35 +74,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media Assert.AreEqual("?crop=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&cropmode=percentage&width=100&height=100", urlString); } - /// - /// Test the height ratio mode with predefined crop dimensions - /// - [Test] - public void GetCropUrl_CropAliasHeightRatioModeTest() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Crop = s_crop, Width = 100, HeightRatio = 1 }); - Assert.AreEqual(MediaPath + "?crop=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&cropmode=percentage&heightratio=1&width=100", urlString); - } - - /// - /// Test the height ratio mode with manual width/height dimensions - /// - [Test] - public void GetCropUrl_WidthHeightRatioModeTest() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { FocalPoint = s_focus1, Width = 300, HeightRatio = 0.5m }); - Assert.AreEqual(MediaPath + "?rxy=0.80827067669172936,0.96&heightratio=0.5&width=300", urlString); - } - - /// - /// Test the height ratio mode with width/height dimensions - /// - [Test] - public void GetCropUrl_HeightWidthRatioModeTest() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { FocalPoint = s_focus1, Height = 150, WidthRatio = 2 }); - Assert.AreEqual(MediaPath + "?rxy=0.80827067669172936,0.96&widthratio=2&height=150", urlString); - } /// /// Test that if Crop mode is specified as anything other than Crop the image doesn't use the crop @@ -139,28 +110,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media [Test] public void GetCropUrl_PreferFocalPointCenter() { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { DefaultCrop = true, Width = 300, Height = 150 }); - Assert.AreEqual(MediaPath + "?ranchor=center&rmode=crop&width=300&height=150", urlString); - } - - /// - /// Test to check if height ratio is returned for a predefined crop without coordinates and focal point in centre when a width parameter is passed - /// - [Test] - public void GetCropUrl_PreDefinedCropNoCoordinatesWithWidth() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { DefaultCrop = true, Width = 200, HeightRatio = 0.5962962962962962962962962963m }); - Assert.AreEqual(MediaPath + "?ranchor=center&rmode=crop&heightratio=0.5962962962962962962962962963&width=200", urlString); - } - - /// - /// Test to check if height ratio is returned for a predefined crop without coordinates and focal point is custom when a width parameter is passed - /// - [Test] - public void GetCropUrl_PreDefinedCropNoCoordinatesWithWidthAndFocalPoint() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { FocalPoint = s_focus2, Width = 200, HeightRatio = 0.5962962962962962962962962963m }); - Assert.AreEqual(MediaPath + "?rxy=0.41,0.4275&heightratio=0.5962962962962962962962962963&width=200", urlString); + var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Width = 300, Height = 150 }); + Assert.AreEqual(MediaPath + "?width=300&height=150", urlString); } /// @@ -173,24 +124,14 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media Assert.AreEqual(MediaPath + "?rxy=0.41,0.4275&width=270&height=161", urlString); } - /// - /// Test to check if width ratio is returned for a predefined crop without coordinates and focal point in centre when a height parameter is passed - /// - [Test] - public void GetCropUrl_PreDefinedCropNoCoordinatesWithHeight() - { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { DefaultCrop = true, Height = 200, WidthRatio = 1.6770186335403726708074534161m }); - Assert.AreEqual(MediaPath + "?ranchor=center&rmode=crop&widthratio=1.6770186335403726708074534161&height=200", urlString); - } - /// /// Test to check result when only a width parameter is passed, effectivly a resize only /// [Test] public void GetCropUrl_WidthOnlyParameter() { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { DefaultCrop = true, Width = 200 }); - Assert.AreEqual(MediaPath + "?ranchor=center&rmode=crop&width=200", urlString); + var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Width = 200 }); + Assert.AreEqual(MediaPath + "?width=200", urlString); } /// @@ -199,8 +140,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Media [Test] public void GetCropUrl_HeightOnlyParameter() { - var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { DefaultCrop = true, Height = 200 }); - Assert.AreEqual(MediaPath + "?ranchor=center&rmode=crop&height=200", urlString); + var urlString = s_generator.GetImageUrl(new ImageUrlGenerationOptions(MediaPath) { Height = 200 }); + Assert.AreEqual(MediaPath + "?height=200", urlString); } /// diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs index 1873b30c99..16b2268a47 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ImageCropperTest.cs @@ -1,8 +1,10 @@ // Copyright (c) Umbraco. // See LICENSE for more details. +using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; using System.Text; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -130,21 +132,21 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common public void GetCropUrl_WidthHeightTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, width: 200, height: 300); - Assert.AreEqual(MediaPath + "?f=0.80827067669172936x0.96&w=200&h=300", urlString); + Assert.AreEqual(MediaPath + "?f=0.80827067669172936,0.96&w=200&h=300", urlString); } [Test] public void GetCropUrl_FocalPointTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, cropAlias: "thumb", preferFocalPoint: true, useCropDimensions: true); - Assert.AreEqual(MediaPath + "?f=0.80827067669172936x0.96&w=100&h=100", urlString); + Assert.AreEqual(MediaPath + "?f=0.80827067669172936,0.96&w=100&h=100", urlString); } [Test] public void GetCropUrlFurtherOptionsTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, width: 200, height: 300, furtherOptions: "&filter=comic&roundedcorners=radius-26|bgcolor-fff"); - Assert.AreEqual(MediaPath + "?f=0.80827067669172936x0.96&w=200&h=300&filter=comic&roundedcorners=radius-26|bgcolor-fff", urlString); + Assert.AreEqual(MediaPath + "?f=0.80827067669172936,0.96&w=200&h=300&filter=comic&roundedcorners=radius-26|bgcolor-fff", urlString); } /// @@ -175,7 +177,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common public void GetCropUrl_CropAliasHeightRatioModeTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, cropAlias: "Thumb", useCropDimensions: true, ratioMode: ImageCropRatioMode.Height); - Assert.AreEqual(MediaPath + "?c=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&hr=1&w=100", urlString); + Assert.AreEqual(MediaPath + "?c=0.58729977382575338,0.055768992440203169,0,0.32457553600198386&w=100&h=100", urlString); } /// @@ -185,7 +187,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common public void GetCropUrl_WidthHeightRatioModeTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, width: 300, height: 150, ratioMode: ImageCropRatioMode.Height); - Assert.AreEqual(MediaPath + "?f=0.80827067669172936x0.96&hr=0.5&w=300", urlString); + Assert.AreEqual(MediaPath + "?f=0.80827067669172936,0.96&w=300&h=150", urlString); } /// @@ -195,7 +197,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common public void GetCropUrl_HeightWidthRatioModeTest() { var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: CropperJson1, width: 300, height: 150, ratioMode: ImageCropRatioMode.Width); - Assert.AreEqual(MediaPath + "?f=0.80827067669172936x0.96&wr=2&h=150", urlString); + Assert.AreEqual(MediaPath + "?f=0.80827067669172936,0.96&w=300&h=150", urlString); } /// @@ -236,7 +238,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\":\"thumb\",\"width\": 100,\"height\": 100,\"coordinates\": {\"x1\": 0.58729977382575338,\"y1\": 0.055768992440203169,\"x2\": 0,\"y2\": 0.32457553600198386}}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, width: 300, height: 150, preferFocalPoint: true); - Assert.AreEqual(MediaPath + "?m=defaultcrop&w=300&h=150", urlString); + Assert.AreEqual(MediaPath + "?w=300&h=150", urlString); } /// @@ -248,7 +250,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, cropAlias: "home", width: 200); - Assert.AreEqual(MediaPath + "?m=defaultcrop&hr=0.5962962962962962962962962963&w=200", urlString); + Assert.AreEqual(MediaPath + "?w=200&h=119", urlString); } /// @@ -260,7 +262,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.4275,\"top\": 0.41},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, cropAlias: "home", width: 200); - Assert.AreEqual(MediaPath + "?f=0.41x0.4275&hr=0.5962962962962962962962962963&w=200", urlString); + Assert.AreEqual(MediaPath + "?f=0.41,0.4275&w=200&h=119", urlString); } /// @@ -272,7 +274,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.4275,\"top\": 0.41},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, cropAlias: "home", width: 200, useCropDimensions: true); - Assert.AreEqual(MediaPath + "?f=0.41x0.4275&w=270&h=161", urlString); + Assert.AreEqual(MediaPath + "?f=0.41,0.4275&w=270&h=161", urlString); } /// @@ -284,7 +286,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, cropAlias: "home", height: 200); - Assert.AreEqual(MediaPath + "?m=defaultcrop&wr=1.6770186335403726708074534161&h=200", urlString); + Assert.AreEqual(MediaPath + "?w=335&h=200", urlString); } /// @@ -296,7 +298,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, width: 200); - Assert.AreEqual(MediaPath + "?m=defaultcrop&w=200", urlString); + Assert.AreEqual(MediaPath + "?w=200", urlString); } /// @@ -308,7 +310,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common const string cropperJson = "{\"focalPoint\": {\"left\": 0.5,\"top\": 0.5},\"src\": \"/media/1005/img_0671.jpg\",\"crops\": [{\"alias\": \"home\",\"width\": 270,\"height\": 161}]}"; var urlString = MediaPath.GetCropUrl(new TestImageUrlGenerator(), imageCropperValue: cropperJson, height: 200); - Assert.AreEqual(MediaPath + "?m=defaultcrop&h=200", urlString); + Assert.AreEqual(MediaPath + "?h=200", urlString); } /// @@ -325,92 +327,82 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.Common internal class TestImageUrlGenerator : IImageUrlGenerator { - public IEnumerable SupportedImageFileTypes => new[] { "jpeg", "jpg", "gif", "bmp", "png", "tiff", "tif" }; + public IEnumerable SupportedImageFileTypes => new[] + { + "jpeg", + "jpg", + "gif", + "bmp", + "png", + "tiff", + "tif" + }; public string GetImageUrl(ImageUrlGenerationOptions options) { - var imageProcessorUrl = new StringBuilder(options.ImageUrl ?? string.Empty); + if (options == null) + { + return null; + } + + var imageUrl = new StringBuilder(options.ImageUrl); + + bool queryStringHasStarted = false; + void AppendQueryString(string value) + { + imageUrl.Append(queryStringHasStarted ? '&' : '?'); + queryStringHasStarted = true; + + imageUrl.Append(value); + } + void AddQueryString(string key, params IConvertible[] values) + => AppendQueryString(key + '=' + string.Join(",", values.Select(x => x.ToString(CultureInfo.InvariantCulture)))); if (options.FocalPoint != null) { - imageProcessorUrl.Append("?f="); - imageProcessorUrl.Append(options.FocalPoint.Top.ToString(CultureInfo.InvariantCulture)); - imageProcessorUrl.Append("x"); - imageProcessorUrl.Append(options.FocalPoint.Left.ToString(CultureInfo.InvariantCulture)); + AddQueryString("f", options.FocalPoint.Top, options.FocalPoint.Left); } else if (options.Crop != null) { - imageProcessorUrl.Append("?c="); - imageProcessorUrl.Append(options.Crop.X1.ToString(CultureInfo.InvariantCulture)).Append(","); - imageProcessorUrl.Append(options.Crop.Y1.ToString(CultureInfo.InvariantCulture)).Append(","); - imageProcessorUrl.Append(options.Crop.X2.ToString(CultureInfo.InvariantCulture)).Append(","); - imageProcessorUrl.Append(options.Crop.Y2.ToString(CultureInfo.InvariantCulture)); - } - else if (options.DefaultCrop) - { - imageProcessorUrl.Append("?m=defaultcrop"); - } - else - { - imageProcessorUrl.Append("?m=" + options.ImageCropMode.ToString().ToLower()); - if (options.ImageCropAnchor != null) - { - imageProcessorUrl.Append("&a=" + options.ImageCropAnchor.ToString().ToLower()); - } + AddQueryString("c", options.Crop.X1, options.Crop.Y1, options.Crop.X2, options.Crop.Y2); } - var hasFormat = options.FurtherOptions != null && options.FurtherOptions.InvariantContains("&f="); - if (options.Quality != null && hasFormat == false) + if (options.ImageCropMode.HasValue) { - imageProcessorUrl.Append("&q=" + options.Quality); + AddQueryString("m", options.ImageCropMode.Value.ToString().ToLowerInvariant()); } - if (options.HeightRatio != null) + if (options.ImageCropAnchor.HasValue) { - imageProcessorUrl.Append("&hr=" + options.HeightRatio.Value.ToString(CultureInfo.InvariantCulture)); - } - - if (options.WidthRatio != null) - { - imageProcessorUrl.Append("&wr=" + options.WidthRatio.Value.ToString(CultureInfo.InvariantCulture)); + AddQueryString("a", options.ImageCropAnchor.Value.ToString().ToLowerInvariant()); } if (options.Width != null) { - imageProcessorUrl.Append("&w=" + options.Width); + AddQueryString("w", options.Width.Value); } if (options.Height != null) { - imageProcessorUrl.Append("&h=" + options.Height); + AddQueryString("h", options.Height.Value); } - - if (options.UpScale == false) + + if (options.Quality.HasValue) { - imageProcessorUrl.Append("&u=no"); - } - - if (options.AnimationProcessMode != null) - { - imageProcessorUrl.Append("&apm=" + options.AnimationProcessMode); + AddQueryString("q", options.Quality.Value); } if (options.FurtherOptions != null) { - imageProcessorUrl.Append(options.FurtherOptions); - } - - if (options.Quality != null && hasFormat) - { - imageProcessorUrl.Append("&q=" + options.Quality); + AppendQueryString(options.FurtherOptions.TrimStart('?', '&')); } if (options.CacheBusterValue != null) { - imageProcessorUrl.Append("&r=").Append(options.CacheBusterValue); + AddQueryString("r", options.CacheBusterValue); } - return imageProcessorUrl.ToString(); + return imageUrl.ToString(); } } } diff --git a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs index 879a70cdb5..27de0d22b2 100644 --- a/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/ImageCropperTemplateCoreExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Globalization; using Newtonsoft.Json.Linq; using Umbraco.Cms.Core; @@ -449,16 +449,17 @@ namespace Umbraco.Extensions height = crop.Height; } - // If a predefined crop has been specified & there are no coordinates & no ratio mode, but a width parameter has been passed we can get the crop ratio for the height - if (crop != null && string.IsNullOrEmpty(cropAlias) == false && crop.Coordinates == null && ratioMode == null && width != null && height == null) + // Calculate missing dimension if a predefined crop has been specified, there are no coordinates and no ratio mode + if (crop != null && string.IsNullOrEmpty(cropAlias) == false && crop.Coordinates == null && ratioMode == null) { - options.HeightRatio = (decimal)crop.Height / crop.Width; - } - - // If a predefined crop has been specified & there are no coordinates & no ratio mode, but a height parameter has been passed we can get the crop ratio for the width - if (crop != null && string.IsNullOrEmpty(cropAlias) == false && crop.Coordinates == null && ratioMode == null && width == null && height != null) - { - options.WidthRatio = (decimal)crop.Width / crop.Height; + if (width != null && height == null) + { + height = (int)MathF.Round(width.Value * ((float)crop.Height / crop.Width)); + } + else if (width == null && height != null) + { + width = (int)MathF.Round(height.Value * ((float)crop.Width / crop.Height)); + } } } else @@ -477,16 +478,28 @@ namespace Umbraco.Extensions if (ratioMode == ImageCropRatioMode.Width && height != null) { - // if only height specified then assume a square - if (width == null) width = height; - options.WidthRatio = (decimal)width / (decimal)height; + // If only height specified then assume a square + if (width == null) + { + options.Width = height; + } + else + { + options.Width = (int)MathF.Round(height.Value * ((float)width.Value / height.Value)); + } } if (ratioMode == ImageCropRatioMode.Height && width != null) { - // if only width specified then assume a square - if (height == null) height = width; - options.HeightRatio = (decimal)height / (decimal)width; + // If only width specified then assume a square + if (height == null) + { + options.Height = width; + } + else + { + options.Height = (int)MathF.Round(width.Value * ((float)height.Value / width.Value)); + } } options.UpScale = upScale;