From 2a8f19c7bea8e644f7cecf4e26e0d4e8a94a1681 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 9 Jul 2025 11:43:33 +0530 Subject: [PATCH 01/60] Enable SrcSet support with three different input formats and test cases in ImageTagHelperFixture --- .../TagHelpers/Fields/ImageTagHelper.cs | 164 ++++++++++++++++++ .../Fields/ImageTagHelperFixture.cs | 148 ++++++++++++++++ 2 files changed, 312 insertions(+) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 84adca1..038b570 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -29,8 +29,87 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper private const string VSpaceAttribute = "vspace"; private const string TitleAttribute = "title"; private const string BorderAttribute = "border"; + private const string SrcSetAttribute = "srcset"; + private const string SizesAttribute = "sizes"; private readonly IEditableChromeRenderer _chromeRenderer = chromeRenderer ?? throw new ArgumentNullException(nameof(chromeRenderer)); + private static string GetWidthDescriptor(object parameters) + { + // Handle Dictionary + if (parameters is Dictionary dictionary) + { + foreach (var kvp in dictionary) + { + if (kvp.Key.Equals("mw", StringComparison.OrdinalIgnoreCase) || + kvp.Key.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)) + { + return $"{kvp.Value}w"; + } + + if (kvp.Key.Equals("w", StringComparison.OrdinalIgnoreCase) || + kvp.Key.Equals("width", StringComparison.OrdinalIgnoreCase)) + { + return $"{kvp.Value}w"; + } + } + } + else + { + // Handle anonymous objects via reflection + var properties = parameters.GetType().GetProperties(); + + foreach (var prop in properties) + { + if (prop.Name.Equals("mw", StringComparison.OrdinalIgnoreCase) || + prop.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)) + { + return $"{prop.GetValue(parameters)}w"; + } + + if (prop.Name.Equals("w", StringComparison.OrdinalIgnoreCase) || + prop.Name.Equals("width", StringComparison.OrdinalIgnoreCase)) + { + return $"{prop.GetValue(parameters)}w"; + } + } + } + + // If no width found, use 1x as fallback + return "1x"; + } + + private static object[]? ParseSrcSet(object? srcSetValue) + { + if (srcSetValue == null) + { + return null; + } + + // If already an object array, use as-is + if (srcSetValue is object[] objectArray) + { + return objectArray; + } + + // If it's a JSON string, parse it + if (srcSetValue is string jsonString) + { + try + { + var parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); + return parsed?.Cast().ToArray(); + } + catch + { + // If JSON parsing fails, return null to skip srcset generation + return null; + } + } + + // Single object - wrap in array + return new[] { srcSetValue }; + } + /// /// Gets or sets the model value. /// @@ -53,6 +132,19 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper /// public object? ImageParams { get; set; } + /// + /// Gets or sets the srcset configurations for responsive images. + /// Supports: object[] (anonymous objects), Dictionary arrays, or JSON string. + /// Each item should contain width parameters like { mw = 300 }, { w = 100 }. + /// + public object? SrcSet { get; set; } + + /// + /// Gets or sets the sizes attribute for responsive images. + /// Example: "(min-width: 960px) 300px, 100px". + /// + public string? Sizes { get; set; } + /// public override void Process(TagHelperContext context, TagHelperOutput output) { @@ -98,6 +190,22 @@ public override void Process(TagHelperContext context, TagHelperOutput output) { output.Attributes.Add(ScrAttribute, field.GetMediaLink(ImageParams)); + // Add srcset if configured + if (SrcSet != null) + { + string srcSetValue = GenerateSrcSetAttribute(field); + if (!string.IsNullOrEmpty(srcSetValue)) + { + output.Attributes.Add(SrcSetAttribute, srcSetValue); + } + } + + // Add sizes if provided + if (!string.IsNullOrEmpty(Sizes)) + { + output.Attributes.Add(SizesAttribute, Sizes); + } + if (!string.IsNullOrWhiteSpace(field.Value.Alt)) { output.Attributes.Add(AltAttribute, field.Value.Alt); @@ -152,6 +260,22 @@ private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) if (!string.IsNullOrWhiteSpace(image.Src)) { tagBuilder.Attributes.Add(ScrAttribute, imageField.GetMediaLink(ImageParams)); + + // Add srcset if configured + if (SrcSet != null) + { + string srcSetValue = GenerateSrcSetAttribute(imageField); + if (!string.IsNullOrEmpty(srcSetValue)) + { + tagBuilder.Attributes.Add(SrcSetAttribute, srcSetValue); + } + } + + // Add sizes if provided + if (!string.IsNullOrEmpty(Sizes)) + { + tagBuilder.Attributes.Add(SizesAttribute, Sizes); + } } if (!string.IsNullOrWhiteSpace(image.Alt)) @@ -230,8 +354,48 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp } imageNode.SetAttributeValue(ScrAttribute, imageField.GetMediaLink(ImageParams)); + + // Add srcset for editable content + if (SrcSet != null) + { + string srcSetValue = GenerateSrcSetAttribute(imageField); + if (!string.IsNullOrEmpty(srcSetValue)) + { + imageNode.SetAttributeValue(SrcSetAttribute, srcSetValue); + } + } + + // Add sizes for editable content + if (!string.IsNullOrEmpty(Sizes)) + { + imageNode.SetAttributeValue(SizesAttribute, Sizes); + } } return new HtmlString(doc.DocumentNode.OuterHtml); } + + private string GenerateSrcSetAttribute(ImageField imageField) + { + object[]? parsedSrcSet = ParseSrcSet(SrcSet); + if (parsedSrcSet == null || parsedSrcSet.Length == 0) + { + return string.Empty; + } + + List srcSetEntries = new(); + + foreach (object srcSetItem in parsedSrcSet) + { + string? mediaUrl = imageField.GetMediaLink(srcSetItem); + if (!string.IsNullOrEmpty(mediaUrl)) + { + // Extract width from parameters to create the descriptor + string descriptor = GetWidthDescriptor(srcSetItem); + srcSetEntries.Add($"{mediaUrl} {descriptor}"); + } + } + + return string.Join(", ", srcSetEntries); + } } \ No newline at end of file diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index e9d9baf..ed1eddd 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -739,6 +739,154 @@ public void Process_RenderingChromesAreNotNull_ChromesAreOutput( chromeRenderer.Received().Render(closingChrome); } + [Theory] + [AutoNSubstituteData] + public void Process_ScImgTagWithSrcSet_GeneratesSrcSetAttribute( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] { new { mw = 300 }, new { mw = 100 } }; + sut.Sizes = "(min-width: 960px) 300px, 100px"; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + string content = tagHelperOutput.Content.GetContent(); + content.Should().Contain("srcset="); + content.Should().Contain("300w"); + content.Should().Contain("100w"); + content.Should().Contain("sizes=\"(min-width: 960px) 300px, 100px\""); + } + + [Theory] + [AutoNSubstituteData] + public void Process_ImgTagWithSrcSet_GeneratesSrcSetAttribute( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] { new { w = 400 }, new { w = 200 } }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("400w"); + srcSetValue.Should().Contain("200w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + Image image = new() { Src = "http://styleguide/-/media/img/sc_logo.png", Alt = "Sitecore Logo" }; + ImageField imageField = new(image) + { + EditableMarkup = "\"Sitecore" + }; + + tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; + sut.For = GetModelExpression(imageField); + sut.SrcSet = new object[] { new { mw = 600 }, new { mw = 300 } }; + sut.Sizes = "(min-width: 768px) 600px, 300px"; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + string content = tagHelperOutput.Content.GetContent(); + content.Should().Contain("srcset="); + content.Should().Contain("600w"); + content.Should().Contain("300w"); + content.Should().Contain("sizes=\"(min-width: 768px) 600px, 300px\""); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithJsonString_GeneratesSrcSetAttribute( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = "[{\"mw\": 500}, {\"mw\": 250}]"; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("500w"); + srcSetValue.Should().Contain("250w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] + { + new { w = 800 }, // Anonymous object with 'w' + new { mw = 400 }, // Anonymous object with 'mw' + new Dictionary { { "width", 200 } }, // Dictionary with 'width' + new { maxWidth = 100 } // Anonymous object with 'maxWidth' + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + string content = tagHelperOutput.Content.GetContent(); + content.Should().Contain("srcset="); + content.Should().Contain("800w"); + content.Should().Contain("400w"); + content.Should().Contain("200w"); + content.Should().Contain("100w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithInvalidJsonString_FallsBackGracefully( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = "invalid json string"; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + // Should not throw and should not add srcset attribute + tagHelperOutput.Attributes.Should().NotContain(a => a.Name == "srcset"); + } + private static ModelExpression GetModelExpression(Field model) { DefaultModelMetadata? modelMetadata = Substitute.For( From 2d719cdc0672ea9a747e956eb8e2d6cce38dff72 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 9 Jul 2025 17:12:51 +0530 Subject: [PATCH 02/60] Remove unnecessary white spaces remained in changes --- .../TagHelpers/Fields/ImageTagHelper.cs | 16 ++++++++-------- .../TagHelpers/Fields/ImageTagHelperFixture.cs | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 038b570..9a5e6d8 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -45,7 +45,7 @@ private static string GetWidthDescriptor(object parameters) { return $"{kvp.Value}w"; } - + if (kvp.Key.Equals("w", StringComparison.OrdinalIgnoreCase) || kvp.Key.Equals("width", StringComparison.OrdinalIgnoreCase)) { @@ -57,7 +57,7 @@ private static string GetWidthDescriptor(object parameters) { // Handle anonymous objects via reflection var properties = parameters.GetType().GetProperties(); - + foreach (var prop in properties) { if (prop.Name.Equals("mw", StringComparison.OrdinalIgnoreCase) || @@ -65,7 +65,7 @@ private static string GetWidthDescriptor(object parameters) { return $"{prop.GetValue(parameters)}w"; } - + if (prop.Name.Equals("w", StringComparison.OrdinalIgnoreCase) || prop.Name.Equals("width", StringComparison.OrdinalIgnoreCase)) { @@ -84,13 +84,13 @@ private static string GetWidthDescriptor(object parameters) { return null; } - + // If already an object array, use as-is if (srcSetValue is object[] objectArray) { return objectArray; } - + // If it's a JSON string, parse it if (srcSetValue is string jsonString) { @@ -105,7 +105,7 @@ private static string GetWidthDescriptor(object parameters) return null; } } - + // Single object - wrap in array return new[] { srcSetValue }; } @@ -260,7 +260,7 @@ private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) if (!string.IsNullOrWhiteSpace(image.Src)) { tagBuilder.Attributes.Add(ScrAttribute, imageField.GetMediaLink(ImageParams)); - + // Add srcset if configured if (SrcSet != null) { @@ -354,7 +354,7 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp } imageNode.SetAttributeValue(ScrAttribute, imageField.GetMediaLink(ImageParams)); - + // Add srcset for editable content if (SrcSet != null) { diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index ed1eddd..56e4065 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -798,7 +798,7 @@ public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( { EditableMarkup = "\"Sitecore" }; - + tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; sut.For = GetModelExpression(imageField); sut.SrcSet = new object[] { new { mw = 600 }, new { mw = 300 } }; From 0937e2fba68c0b33ace3e93c05ddf5fa83ef5bf8 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 14 Jul 2025 14:09:28 +0530 Subject: [PATCH 03/60] Add changes to match with content SDK --- .../Extensions/SitecoreFieldExtensions.cs | 74 +++++++++++++++++++ .../TagHelpers/Fields/ImageTagHelper.cs | 61 +++++++-------- .../Fields/ImageTagHelperFixture.cs | 59 +++++++++++---- 3 files changed, 151 insertions(+), 43 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 710e258..a85e0a4 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -29,6 +29,80 @@ public static partial class SitecoreFieldExtensions return GetSitecoreMediaUri(urlStr, imageParams); } + /// + /// Gets modified URL string to Sitecore media item with merged parameters. + /// + /// The image field. + /// Base image parameters. + /// SrcSet specific parameters that override imageParams. + /// Media item URL. + public static string? GetMediaLink(this ImageField imageField, object? imageParams, object? srcSetParams) + { + ArgumentNullException.ThrowIfNull(imageField); + string? urlStr = imageField.Value.Src; + + if (urlStr == null) + { + return null; + } + + var mergedParams = MergeParameters(imageParams, srcSetParams); + return GetSitecoreMediaUri(urlStr, mergedParams); + } + + /// + /// Merges base parameters with override parameters. + /// + /// Base parameters. + /// Override parameters that take precedence. + /// Merged parameters as dictionary. + private static Dictionary MergeParameters(object? baseParams, object? overrideParams) + { + var result = new Dictionary(); + + // Add base parameters first + if (baseParams != null) + { + if (baseParams is Dictionary baseDict) + { + foreach (var kvp in baseDict) + { + result[kvp.Key] = kvp.Value; + } + } + else + { + var baseProps = baseParams.GetType().GetProperties(); + foreach (var prop in baseProps) + { + result[prop.Name] = prop.GetValue(baseParams); + } + } + } + + // Override with srcSet parameters + if (overrideParams != null) + { + if (overrideParams is Dictionary overrideDict) + { + foreach (var kvp in overrideDict) + { + result[kvp.Key] = kvp.Value; + } + } + else + { + var overrideProps = overrideParams.GetType().GetProperties(); + foreach (var prop in overrideProps) + { + result[prop.Name] = prop.GetValue(overrideParams); + } + } + } + + return result; + } + /// /// Gets URL to Sitecore media item. /// diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 9a5e6d8..0668167 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -33,49 +33,45 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper private const string SizesAttribute = "sizes"; private readonly IEditableChromeRenderer _chromeRenderer = chromeRenderer ?? throw new ArgumentNullException(nameof(chromeRenderer)); - private static string GetWidthDescriptor(object parameters) + private static string? GetWidthDescriptor(object parameters) { + string? width = null; + // Handle Dictionary if (parameters is Dictionary dictionary) { - foreach (var kvp in dictionary) + // Priority: w > mw (matching Content SDK behavior) + if (dictionary.TryGetValue("w", out object? wValue)) { - if (kvp.Key.Equals("mw", StringComparison.OrdinalIgnoreCase) || - kvp.Key.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)) - { - return $"{kvp.Value}w"; - } - - if (kvp.Key.Equals("w", StringComparison.OrdinalIgnoreCase) || - kvp.Key.Equals("width", StringComparison.OrdinalIgnoreCase)) - { - return $"{kvp.Value}w"; - } + width = wValue?.ToString(); + } + else if (dictionary.TryGetValue("mw", out object? mwValue)) + { + width = mwValue?.ToString(); } } else { // Handle anonymous objects via reflection var properties = parameters.GetType().GetProperties(); - - foreach (var prop in properties) + + // Priority: w > mw (matching Content SDK behavior) + var wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); + if (wProp != null) { - if (prop.Name.Equals("mw", StringComparison.OrdinalIgnoreCase) || - prop.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)) - { - return $"{prop.GetValue(parameters)}w"; - } - - if (prop.Name.Equals("w", StringComparison.OrdinalIgnoreCase) || - prop.Name.Equals("width", StringComparison.OrdinalIgnoreCase)) + width = wProp.GetValue(parameters)?.ToString(); + } + else + { + var mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); + if (mwProp != null) { - return $"{prop.GetValue(parameters)}w"; + width = mwProp.GetValue(parameters)?.ToString(); } } } - // If no width found, use 1x as fallback - return "1x"; + return width != null ? $"{width}w" : null; } private static object[]? ParseSrcSet(object? srcSetValue) @@ -387,11 +383,18 @@ private string GenerateSrcSetAttribute(ImageField imageField) foreach (object srcSetItem in parsedSrcSet) { - string? mediaUrl = imageField.GetMediaLink(srcSetItem); + // Get width descriptor first to check if this entry should be included + string? descriptor = GetWidthDescriptor(srcSetItem); + if (descriptor == null) + { + // Skip entries without valid width parameters (matching Content SDK behavior) + continue; + } + + // Use the new overload that merges ImageParams with srcSetItem params + string? mediaUrl = imageField.GetMediaLink(ImageParams, srcSetItem); if (!string.IsNullOrEmpty(mediaUrl)) { - // Extract width from parameters to create the descriptor - string descriptor = GetWidthDescriptor(srcSetItem); srcSetEntries.Add($"{mediaUrl} {descriptor}"); } } diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index 56e4065..e21cf9f 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -749,23 +749,21 @@ public void Process_ScImgTagWithSrcSet_GeneratesSrcSetAttribute( // Arrange tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; sut.For = GetModelExpression(new ImageField(_image)); - sut.SrcSet = new object[] { new { mw = 300 }, new { mw = 100 } }; - sut.Sizes = "(min-width: 960px) 300px, 100px"; + sut.SrcSet = new object[] { new { w = 400 }, new { w = 200 } }; // Act sut.Process(tagHelperContext, tagHelperOutput); - // Assert + // Assert - generates HTML content, not attributes string content = tagHelperOutput.Content.GetContent(); content.Should().Contain("srcset="); - content.Should().Contain("300w"); - content.Should().Contain("100w"); - content.Should().Contain("sizes=\"(min-width: 960px) 300px, 100px\""); + content.Should().Contain("400w"); + content.Should().Contain("200w"); } [Theory] [AutoNSubstituteData] - public void Process_ImgTagWithSrcSet_GeneratesSrcSetAttribute( + public void Process_SrcSetWithContentSDKBehavior_MatchesExpectedOutput( ImageTagHelper sut, TagHelperContext tagHelperContext, TagHelperOutput tagHelperOutput) @@ -773,7 +771,8 @@ public void Process_ImgTagWithSrcSet_GeneratesSrcSetAttribute( // Arrange tagHelperOutput.TagName = "img"; sut.For = GetModelExpression(new ImageField(_image)); - sut.SrcSet = new object[] { new { w = 400 }, new { w = 200 } }; + sut.ImageParams = new { h = 1000 }; // Base parameters + sut.SrcSet = new object[] { new { h = 1000, w = 1000 }, new { mh = 250, mw = 250 } }; // Act sut.Process(tagHelperContext, tagHelperOutput); @@ -781,8 +780,43 @@ public void Process_ImgTagWithSrcSet_GeneratesSrcSetAttribute( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("400w"); - srcSetValue.Should().Contain("200w"); + + // Should contain "1000w" (from w parameter taking priority over h) + srcSetValue.Should().Contain("1000w"); + // Should contain "250w" (from mw parameter) + srcSetValue.Should().Contain("250w"); + + // Verify it contains the expected format: "url 1000w, url 250w" + var entries = srcSetValue.Split(", "); + entries.Should().HaveCount(2); + entries[0].Should().EndWith("1000w"); + entries[1].Should().EndWith("250w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithoutValidWidth_FiltersEntries( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] { new { h = 1000 }, new { mw = 250 }, new { quality = 80 } }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + + // Should only contain the entry with mw parameter + srcSetValue.Should().Contain("250w"); + // Should not contain entries without width parameters + var entries = srcSetValue.Split(", "); + entries.Should().HaveCount(1); } [Theory] @@ -851,8 +885,7 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( { new { w = 800 }, // Anonymous object with 'w' new { mw = 400 }, // Anonymous object with 'mw' - new Dictionary { { "width", 200 } }, // Dictionary with 'width' - new { maxWidth = 100 } // Anonymous object with 'maxWidth' + // Only w and mw are supported by Content SDK for srcSet }; // Act @@ -863,8 +896,6 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( content.Should().Contain("srcset="); content.Should().Contain("800w"); content.Should().Contain("400w"); - content.Should().Contain("200w"); - content.Should().Contain("100w"); } [Theory] From defd382e6c951c34b4315bf15de3f9b0afe59206 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 14 Jul 2025 16:02:12 +0530 Subject: [PATCH 04/60] Alternative approach for parameter preservation in srcSet functionality --- .../Extensions/SitecoreFieldExtensions.cs | 101 +++++++++++++++++- .../TagHelpers/Fields/ImageTagHelper.cs | 8 +- 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a85e0a4..a7cc156 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -1,4 +1,5 @@ using System.Text.RegularExpressions; +using System.Web; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.WebUtilities; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; @@ -47,7 +48,29 @@ public static partial class SitecoreFieldExtensions } var mergedParams = MergeParameters(imageParams, srcSetParams); - return GetSitecoreMediaUri(urlStr, mergedParams); + return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); + } + + /// + /// Gets modified URL string to Sitecore media item for srcSet with parameter preservation. + /// This method preserves critical Sitecore parameters needed for proper image processing. + /// + /// The image field. + /// Base image parameters. + /// SrcSet specific parameters that override imageParams. + /// Media item URL. + public static string? GetMediaLinkForSrcSet(this ImageField imageField, object? imageParams, object? srcSetParams) + { + ArgumentNullException.ThrowIfNull(imageField); + string? urlStr = imageField.Value.Src; + + if (urlStr == null) + { + return null; + } + + var mergedParams = MergeParameters(imageParams, srcSetParams); + return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); } /// @@ -103,6 +126,82 @@ public static partial class SitecoreFieldExtensions return result; } + /// + /// Gets modified URL string to Sitecore media item with parameter preservation. + /// This preserves existing query parameters while adding new ones. + /// + /// Media item source URL. + /// Additional parameters to merge with existing ones. + /// Media item URL with preserved and merged parameters. + private static string GetSitecoreMediaUriWithPreservation(string url, object? parameters) + { + if (string.IsNullOrWhiteSpace(url)) + { + return url; + } + + // Parse the existing URL to separate base URL and query string + var uri = new Uri(url); + var baseUrl = uri.GetLeftPart(UriPartial.Path); + var existingQuery = HttpUtility.ParseQueryString(uri.Query); + + // Convert parameters to dictionary and merge with existing query parameters + var paramDict = ConvertToStringDictionary(parameters); + if (paramDict != null) + { + foreach (var param in paramDict) + { + existingQuery[param.Key] = param.Value; + } + } + + // Apply the media URL prefix transformation (jssmedia replacement) + var finalBaseUrl = baseUrl; + Match match = MediaUrlPrefixRegex().Match(finalBaseUrl); + if (match.Success) + { + finalBaseUrl = finalBaseUrl.Replace(match.Value, $"/{match.Groups[1]}/jssmedia/", StringComparison.InvariantCulture); + } + + // Build the final URL with merged parameters + var queryString = existingQuery.ToString(); + return string.IsNullOrEmpty(queryString) ? finalBaseUrl : $"{finalBaseUrl}?{queryString}"; + } + + /// + /// Converts an object to a string dictionary. + /// + /// The object to convert. + /// Dictionary representation of the object. + private static Dictionary? ConvertToStringDictionary(object? obj) + { + if (obj == null) + { + return null; + } + + var result = new Dictionary(); + + if (obj is Dictionary dict) + { + foreach (var kvp in dict) + { + result[kvp.Key] = kvp.Value?.ToString() ?? string.Empty; + } + } + else + { + var props = obj.GetType().GetProperties(); + foreach (var prop in props) + { + var value = prop.GetValue(obj); + result[prop.Name] = value?.ToString() ?? string.Empty; + } + } + + return result; + } + /// /// Gets URL to Sitecore media item. /// diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 0668167..5868b2a 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -36,7 +36,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper private static string? GetWidthDescriptor(object parameters) { string? width = null; - + // Handle Dictionary if (parameters is Dictionary dictionary) { @@ -54,7 +54,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper { // Handle anonymous objects via reflection var properties = parameters.GetType().GetProperties(); - + // Priority: w > mw (matching Content SDK behavior) var wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); if (wProp != null) @@ -391,8 +391,8 @@ private string GenerateSrcSetAttribute(ImageField imageField) continue; } - // Use the new overload that merges ImageParams with srcSetItem params - string? mediaUrl = imageField.GetMediaLink(ImageParams, srcSetItem); + // Use GetMediaLinkForSrcSet to preserve existing URL parameters (like ttc, tt, hash, quality, format) + string? mediaUrl = imageField.GetMediaLinkForSrcSet(ImageParams, srcSetItem); if (!string.IsNullOrEmpty(mediaUrl)) { srcSetEntries.Add($"{mediaUrl} {descriptor}"); From 6fc8c4f2b5880764952a909cbc6d02e698bf79a1 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 14 Jul 2025 17:24:04 +0530 Subject: [PATCH 05/60] Enhanced parameter support - Added legacy width and maxWidth parameter support, Specific exception handling - Only catch JsonException instead of all exceptions, Better ASP.NET Core compatibility - Replaced HttpUtility with QueryHelpers --- .../Extensions/SitecoreFieldExtensions.cs | 9 +++--- .../TagHelpers/Fields/ImageTagHelper.cs | 30 +++++++++++++++++-- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a7cc156..5233242 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -141,9 +141,9 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa } // Parse the existing URL to separate base URL and query string - var uri = new Uri(url); + var uri = new Uri(url, UriKind.RelativeOrAbsolute); var baseUrl = uri.GetLeftPart(UriPartial.Path); - var existingQuery = HttpUtility.ParseQueryString(uri.Query); + var existingQuery = QueryHelpers.ParseQuery(uri.Query); // Convert parameters to dictionary and merge with existing query parameters var paramDict = ConvertToStringDictionary(parameters); @@ -151,6 +151,7 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa { foreach (var param in paramDict) { + // QueryHelpers.ParseQuery returns StringValues, so we need to handle this properly existingQuery[param.Key] = param.Value; } } @@ -164,8 +165,8 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa } // Build the final URL with merged parameters - var queryString = existingQuery.ToString(); - return string.IsNullOrEmpty(queryString) ? finalBaseUrl : $"{finalBaseUrl}?{queryString}"; + var queryString = QueryHelpers.AddQueryString(string.Empty, existingQuery.ToDictionary(kvp => kvp.Key, kvp => kvp.Value.ToString())); + return queryString.StartsWith("?") ? $"{finalBaseUrl}{queryString}" : finalBaseUrl; } /// diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 5868b2a..162695b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -40,7 +40,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper // Handle Dictionary if (parameters is Dictionary dictionary) { - // Priority: w > mw (matching Content SDK behavior) + // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) if (dictionary.TryGetValue("w", out object? wValue)) { width = wValue?.ToString(); @@ -49,13 +49,21 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper { width = mwValue?.ToString(); } + else if (dictionary.TryGetValue("width", out object? widthValue)) + { + width = widthValue?.ToString(); + } + else if (dictionary.TryGetValue("maxWidth", out object? maxWidthValue)) + { + width = maxWidthValue?.ToString(); + } } else { // Handle anonymous objects via reflection var properties = parameters.GetType().GetProperties(); - // Priority: w > mw (matching Content SDK behavior) + // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) var wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); if (wProp != null) { @@ -68,6 +76,22 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper { width = mwProp.GetValue(parameters)?.ToString(); } + else + { + var widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); + if (widthProp != null) + { + width = widthProp.GetValue(parameters)?.ToString(); + } + else + { + var maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); + if (maxWidthProp != null) + { + width = maxWidthProp.GetValue(parameters)?.ToString(); + } + } + } } } @@ -95,7 +119,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper var parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); return parsed?.Cast().ToArray(); } - catch + catch (System.Text.Json.JsonException) { // If JSON parsing fails, return null to skip srcset generation return null; From eaa8bc5b4711af8499d6361dd5a4521ce402651e Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 14 Jul 2025 17:37:20 +0530 Subject: [PATCH 06/60] Replace var keywords with explicit type declarations --- .../Extensions/SitecoreFieldExtensions.cs | 45 ++++++++++--------- .../TagHelpers/Fields/ImageTagHelper.cs | 15 ++++--- .../Fields/ImageTagHelperFixture.cs | 14 +++--- 3 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 5233242..a365a0f 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -1,4 +1,5 @@ -using System.Text.RegularExpressions; +using System.Reflection; +using System.Text.RegularExpressions; using System.Web; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.WebUtilities; @@ -47,7 +48,7 @@ public static partial class SitecoreFieldExtensions return null; } - var mergedParams = MergeParameters(imageParams, srcSetParams); + Dictionary mergedParams = MergeParameters(imageParams, srcSetParams); return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); } @@ -69,7 +70,7 @@ public static partial class SitecoreFieldExtensions return null; } - var mergedParams = MergeParameters(imageParams, srcSetParams); + Dictionary mergedParams = MergeParameters(imageParams, srcSetParams); return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); } @@ -81,22 +82,22 @@ public static partial class SitecoreFieldExtensions /// Merged parameters as dictionary. private static Dictionary MergeParameters(object? baseParams, object? overrideParams) { - var result = new Dictionary(); + Dictionary result = new Dictionary(); // Add base parameters first if (baseParams != null) { if (baseParams is Dictionary baseDict) { - foreach (var kvp in baseDict) + foreach (KeyValuePair kvp in baseDict) { result[kvp.Key] = kvp.Value; } } else { - var baseProps = baseParams.GetType().GetProperties(); - foreach (var prop in baseProps) + PropertyInfo[] baseProps = baseParams.GetType().GetProperties(); + foreach (PropertyInfo prop in baseProps) { result[prop.Name] = prop.GetValue(baseParams); } @@ -108,15 +109,15 @@ public static partial class SitecoreFieldExtensions { if (overrideParams is Dictionary overrideDict) { - foreach (var kvp in overrideDict) + foreach (KeyValuePair kvp in overrideDict) { result[kvp.Key] = kvp.Value; } } else { - var overrideProps = overrideParams.GetType().GetProperties(); - foreach (var prop in overrideProps) + PropertyInfo[] overrideProps = overrideParams.GetType().GetProperties(); + foreach (PropertyInfo prop in overrideProps) { result[prop.Name] = prop.GetValue(overrideParams); } @@ -141,15 +142,15 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa } // Parse the existing URL to separate base URL and query string - var uri = new Uri(url, UriKind.RelativeOrAbsolute); - var baseUrl = uri.GetLeftPart(UriPartial.Path); - var existingQuery = QueryHelpers.ParseQuery(uri.Query); + Uri uri = new Uri(url, UriKind.RelativeOrAbsolute); + string baseUrl = uri.GetLeftPart(UriPartial.Path); + Dictionary existingQuery = QueryHelpers.ParseQuery(uri.Query); // Convert parameters to dictionary and merge with existing query parameters - var paramDict = ConvertToStringDictionary(parameters); + Dictionary? paramDict = ConvertToStringDictionary(parameters); if (paramDict != null) { - foreach (var param in paramDict) + foreach (KeyValuePair param in paramDict) { // QueryHelpers.ParseQuery returns StringValues, so we need to handle this properly existingQuery[param.Key] = param.Value; @@ -157,7 +158,7 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa } // Apply the media URL prefix transformation (jssmedia replacement) - var finalBaseUrl = baseUrl; + string finalBaseUrl = baseUrl; Match match = MediaUrlPrefixRegex().Match(finalBaseUrl); if (match.Success) { @@ -165,7 +166,7 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa } // Build the final URL with merged parameters - var queryString = QueryHelpers.AddQueryString(string.Empty, existingQuery.ToDictionary(kvp => kvp.Key, kvp => kvp.Value.ToString())); + string queryString = QueryHelpers.AddQueryString(string.Empty, existingQuery.ToDictionary(kvp => kvp.Key, kvp => (string?)kvp.Value.ToString())); return queryString.StartsWith("?") ? $"{finalBaseUrl}{queryString}" : finalBaseUrl; } @@ -181,21 +182,21 @@ private static string GetSitecoreMediaUriWithPreservation(string url, object? pa return null; } - var result = new Dictionary(); + Dictionary result = new Dictionary(); if (obj is Dictionary dict) { - foreach (var kvp in dict) + foreach (KeyValuePair kvp in dict) { result[kvp.Key] = kvp.Value?.ToString() ?? string.Empty; } } else { - var props = obj.GetType().GetProperties(); - foreach (var prop in props) + PropertyInfo[] props = obj.GetType().GetProperties(); + foreach (PropertyInfo prop in props) { - var value = prop.GetValue(obj); + object? value = prop.GetValue(obj); result[prop.Name] = value?.ToString() ?? string.Empty; } } diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 162695b..ae236b3 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -1,4 +1,5 @@ -using HtmlAgilityPack; +using System.Reflection; +using HtmlAgilityPack; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; @@ -61,31 +62,31 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper else { // Handle anonymous objects via reflection - var properties = parameters.GetType().GetProperties(); + PropertyInfo[] properties = parameters.GetType().GetProperties(); // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - var wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); + PropertyInfo? wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); if (wProp != null) { width = wProp.GetValue(parameters)?.ToString(); } else { - var mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); + PropertyInfo? mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); if (mwProp != null) { width = mwProp.GetValue(parameters)?.ToString(); } else { - var widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); + PropertyInfo? widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); if (widthProp != null) { width = widthProp.GetValue(parameters)?.ToString(); } else { - var maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); + PropertyInfo? maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); if (maxWidthProp != null) { width = maxWidthProp.GetValue(parameters)?.ToString(); @@ -116,7 +117,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper { try { - var parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); + Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); return parsed?.Cast().ToArray(); } catch (System.Text.Json.JsonException) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index e21cf9f..51b1d41 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -780,14 +780,15 @@ public void Process_SrcSetWithContentSDKBehavior_MatchesExpectedOutput( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - + // Should contain "1000w" (from w parameter taking priority over h) srcSetValue.Should().Contain("1000w"); + // Should contain "250w" (from mw parameter) srcSetValue.Should().Contain("250w"); - + // Verify it contains the expected format: "url 1000w, url 250w" - var entries = srcSetValue.Split(", "); + string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(2); entries[0].Should().EndWith("1000w"); entries[1].Should().EndWith("250w"); @@ -811,11 +812,12 @@ public void Process_SrcSetWithoutValidWidth_FiltersEntries( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - + // Should only contain the entry with mw parameter srcSetValue.Should().Contain("250w"); + // Should not contain entries without width parameters - var entries = srcSetValue.Split(", "); + string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(1); } @@ -883,9 +885,9 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( sut.For = GetModelExpression(new ImageField(_image)); sut.SrcSet = new object[] { + // Only w and mw are supported by Content SDK for srcSet new { w = 800 }, // Anonymous object with 'w' new { mw = 400 }, // Anonymous object with 'mw' - // Only w and mw are supported by Content SDK for srcSet }; // Act From e58c52c8bf1750134e18ab5eab1af5f82df9c71b Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 16 Jul 2025 14:59:05 +0530 Subject: [PATCH 07/60] feat: Keep URL parameter preservation for srcset and add comprehensive tests - Keep GetSitecoreMediaUriWithPreservation - Add 14 new test cases covering srcset parameter merging, validation, and edge cases - Fix null handling and zero/negative width filtering in ImageTagHelper - Ensure development environment compatibility with CDN/edge URLs --- .../Extensions/SitecoreFieldExtensions.cs | 144 +++++----- .../TagHelpers/Fields/ImageTagHelper.cs | 27 +- .../Fields/ImageTagHelperFixture.cs | 269 ++++++++++++++++++ 3 files changed, 369 insertions(+), 71 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a365a0f..4534c72 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -53,8 +53,8 @@ public static partial class SitecoreFieldExtensions } /// - /// Gets modified URL string to Sitecore media item for srcSet with parameter preservation. - /// This method preserves critical Sitecore parameters needed for proper image processing. + /// Gets modified URL string to Sitecore media item for srcSet. + /// This method preserves existing URL parameters and merges them with new ones. /// /// The image field. /// Base image parameters. @@ -128,103 +128,115 @@ public static partial class SitecoreFieldExtensions } /// - /// Gets modified URL string to Sitecore media item with parameter preservation. - /// This preserves existing query parameters while adding new ones. + /// Gets URL to Sitecore media item. /// - /// Media item source URL. - /// Additional parameters to merge with existing ones. - /// Media item URL with preserved and merged parameters. - private static string GetSitecoreMediaUriWithPreservation(string url, object? parameters) + /// The image URL. + /// Image parameters. + /// Media item URL. + private static string GetSitecoreMediaUri(string url, object? imageParams) { - if (string.IsNullOrWhiteSpace(url)) + // TODO What's the reason we strip away existing querystring? + if (imageParams != null) { - return url; - } - - // Parse the existing URL to separate base URL and query string - Uri uri = new Uri(url, UriKind.RelativeOrAbsolute); - string baseUrl = uri.GetLeftPart(UriPartial.Path); - Dictionary existingQuery = QueryHelpers.ParseQuery(uri.Query); + string[] urlParts = url.Split('?'); + if (urlParts.Length > 1) + { + url = urlParts[0]; + } - // Convert parameters to dictionary and merge with existing query parameters - Dictionary? paramDict = ConvertToStringDictionary(parameters); - if (paramDict != null) - { - foreach (KeyValuePair param in paramDict) + RouteValueDictionary parameters = new(imageParams); + foreach (string key in parameters.Keys) { - // QueryHelpers.ParseQuery returns StringValues, so we need to handle this properly - existingQuery[param.Key] = param.Value; + url = QueryHelpers.AddQueryString(url, key, parameters[key]?.ToString() ?? string.Empty); } } - // Apply the media URL prefix transformation (jssmedia replacement) - string finalBaseUrl = baseUrl; - Match match = MediaUrlPrefixRegex().Match(finalBaseUrl); + // TODO Review hardcoded matching and replacement + Match match = MediaUrlPrefixRegex().Match(url); if (match.Success) { - finalBaseUrl = finalBaseUrl.Replace(match.Value, $"/{match.Groups[1]}/jssmedia/", StringComparison.InvariantCulture); + url = url.Replace(match.Value, $"/{match.Groups[1]}/jssmedia/", StringComparison.InvariantCulture); } - // Build the final URL with merged parameters - string queryString = QueryHelpers.AddQueryString(string.Empty, existingQuery.ToDictionary(kvp => kvp.Key, kvp => (string?)kvp.Value.ToString())); - return queryString.StartsWith("?") ? $"{finalBaseUrl}{queryString}" : finalBaseUrl; + return url; } /// - /// Converts an object to a string dictionary. + /// Gets modified URL string to Sitecore media item with parameter preservation. + /// This method preserves existing URL parameters and merges them with new ones. /// - /// The object to convert. - /// Dictionary representation of the object. - private static Dictionary? ConvertToStringDictionary(object? obj) + /// The URL string. + /// Parameters to merge. + /// Modified URL string. + private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? parameters) { - if (obj == null) + if (string.IsNullOrEmpty(urlStr)) { - return null; + return urlStr; } - Dictionary result = new Dictionary(); + string url = urlStr; - if (obj is Dictionary dict) + // Parse existing query parameters + Dictionary existingParams = new Dictionary(); + if (url.Contains('?')) { - foreach (KeyValuePair kvp in dict) + string[] parts = url.Split('?', 2); + url = parts[0]; + string queryString = parts[1]; + + string[] paramPairs = queryString.Split('&'); + foreach (string paramPair in paramPairs) { - result[kvp.Key] = kvp.Value?.ToString() ?? string.Empty; + string[] keyValue = paramPair.Split('=', 2); + if (keyValue.Length == 2) + { + string key = HttpUtility.UrlDecode(keyValue[0]); + string value = HttpUtility.UrlDecode(keyValue[1]); + existingParams[key] = value; + } } } - else + + // Merge with new parameters (new parameters override existing ones) + Dictionary mergedParams = new Dictionary(); + + // Add existing parameters first + foreach (KeyValuePair kvp in existingParams) { - PropertyInfo[] props = obj.GetType().GetProperties(); - foreach (PropertyInfo prop in props) - { - object? value = prop.GetValue(obj); - result[prop.Name] = value?.ToString() ?? string.Empty; - } + mergedParams[kvp.Key] = kvp.Value; } - return result; - } - - /// - /// Gets URL to Sitecore media item. - /// - /// The image URL. - /// Image parameters. - /// Media item URL. - private static string GetSitecoreMediaUri(string url, object? imageParams) - { - // TODO What's the reason we strip away existing querystring? - if (imageParams != null) + // Add new parameters (these will override existing ones) + if (parameters != null) { - string[] urlParts = url.Split('?'); - if (urlParts.Length > 1) + if (parameters is Dictionary paramDict) { - url = urlParts[0]; + foreach (KeyValuePair kvp in paramDict) + { + mergedParams[kvp.Key] = kvp.Value; + } } + else + { + PropertyInfo[] properties = parameters.GetType().GetProperties(); + foreach (PropertyInfo prop in properties) + { + object? value = prop.GetValue(parameters); + if (value != null) + { + mergedParams[prop.Name] = value; + } + } + } + } - RouteValueDictionary parameters = new(imageParams); - foreach (string key in parameters.Keys) + // Add query parameters + foreach (KeyValuePair kvp in mergedParams) + { + if (kvp.Value != null) { - url = QueryHelpers.AddQueryString(url, key, parameters[key]?.ToString() ?? string.Empty); + url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty); } } diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index ae236b3..9b996e8 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -34,8 +34,13 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper private const string SizesAttribute = "sizes"; private readonly IEditableChromeRenderer _chromeRenderer = chromeRenderer ?? throw new ArgumentNullException(nameof(chromeRenderer)); - private static string? GetWidthDescriptor(object parameters) + private static string? GetWidthDescriptor(object? parameters) { + if (parameters == null) + { + return null; + } + string? width = null; // Handle Dictionary @@ -50,13 +55,13 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper { width = mwValue?.ToString(); } - else if (dictionary.TryGetValue("width", out object? widthValue)) + else if (dictionary.TryGetValue("width", out object? widthObj)) { - width = widthValue?.ToString(); + width = widthObj?.ToString(); } - else if (dictionary.TryGetValue("maxWidth", out object? maxWidthValue)) + else if (dictionary.TryGetValue("maxWidth", out object? maxWidthObj)) { - width = maxWidthValue?.ToString(); + width = maxWidthObj?.ToString(); } } else @@ -96,6 +101,12 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper } } + // Validate width is positive + if (width != null && int.TryParse(width, out int widthValue) && widthValue <= 0) + { + return null; + } + return width != null ? $"{width}w" : null; } @@ -408,6 +419,12 @@ private string GenerateSrcSetAttribute(ImageField imageField) foreach (object srcSetItem in parsedSrcSet) { + // Skip null items + if (srcSetItem == null) + { + continue; + } + // Get width descriptor first to check if this entry should be included string? descriptor = GetWidthDescriptor(srcSetItem); if (descriptor == null) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index 51b1d41..ab92c4f 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -920,6 +920,275 @@ public void Process_SrcSetWithInvalidJsonString_FallsBackGracefully( tagHelperOutput.Attributes.Should().NotContain(a => a.Name == "srcset"); } + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithImageParamsConflict_SrcSetParametersWin( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.ImageParams = new { w = 1000, quality = 50, format = "jpg" }; + sut.SrcSet = new object[] { new { w = 320, quality = 75 } }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("w=320"); + srcSetValue.Should().Contain("quality=75"); + srcSetValue.Should().Contain("format=jpg"); // Inherited from ImageParams + srcSetValue.Should().NotContain("w=1000"); + srcSetValue.Should().NotContain("quality=50"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithWidthParameterPriority_UsesCorrectPrecedence( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange - Test priority: w > mw > width > maxWidth + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] + { + new { w = 100, mw = 200, width = 300, maxWidth = 400 }, // Should use w=100 + new { mw = 200, width = 300, maxWidth = 400 }, // Should use mw=200 + new { width = 300, maxWidth = 400 }, // Should use width=300 + new { maxWidth = 400 } // Should use maxWidth=400 + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("100w"); + srcSetValue.Should().Contain("200w"); + srcSetValue.Should().Contain("300w"); + srcSetValue.Should().Contain("400w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithZeroOrNegativeWidths_SkipsInvalidEntries( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] + { + new { w = 0, quality = 75 }, // Should skip + new { w = -100, quality = 80 }, // Should skip + new { w = 320, quality = 75 }, // Should include + new { quality = 90 } // Should skip - no width + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("320w"); + srcSetValue.Should().NotContain(" 0w"); + srcSetValue.Should().NotContain(" -100w"); + + // Should only have one entry + string[] entries = srcSetValue.Split(", "); + entries.Should().HaveCount(1); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithExistingUrlParameters_PreservesAndMergesParameters( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + Image imageWithParams = new Image + { + Src = "https://edge.sitecorecloud.io/media/image.jpg?h=2001&iar=0&hash=abc123&w=3000", + Alt = "Test Image" + }; + + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(imageWithParams)); + sut.SrcSet = new object[] { new { w = 320, quality = 75 } }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + + // Should preserve existing parameters and merge with new ones + srcSetValue.Should().Contain("h=2001"); + srcSetValue.Should().Contain("iar=0"); + srcSetValue.Should().Contain("hash=abc123"); + srcSetValue.Should().Contain("quality=75"); + + // New w parameter should override existing w=3000 + srcSetValue.Should().Contain("w=320"); + srcSetValue.Should().NotContain("w=3000"); + srcSetValue.Should().Contain("320w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithMediaUrlTransformation_TransformsCorrectly( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + Image imageWithMediaUrl = new Image + { + Src = "/~/media/images/test.jpg", + Alt = "Test Image" + }; + + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(imageWithMediaUrl)); + sut.SrcSet = new object[] { new { w = 320, quality = 75 } }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + + // Should transform /~/media/ to /~/jssmedia/ + srcSetValue.Should().Contain("/~/jssmedia/images/test.jpg"); + srcSetValue.Should().NotContain("/~/media/images/test.jpg"); + srcSetValue.Should().Contain("320w"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithNullAndEmptyEntries_HandlesGracefully( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object?[] + { + new { w = 320, quality = 75 }, + null, // Should skip + new { w = 480, quality = 80 }, + new { } // Should skip - no width parameter + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("320w"); + srcSetValue.Should().Contain("480w"); + + // Should only have two entries + string[] entries = srcSetValue.Split(", "); + entries.Should().HaveCount(2); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithComplexParameters_HandlesAllParameters( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] + { + new { w = 320, quality = 75, format = "webp", dpr = 2, fit = "crop" } + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("320w"); + srcSetValue.Should().Contain("quality=75"); + srcSetValue.Should().Contain("format=webp"); + srcSetValue.Should().Contain("dpr=2"); + srcSetValue.Should().Contain("fit=crop"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithDictionaryParameters_GeneratesSrcSetAttribute( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] + { + new Dictionary { { "w", 320 }, { "quality", 75 } }, + new Dictionary { { "mw", 480 }, { "quality", 80 } } + }; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; + srcSetValue.Should().Contain("320w"); + srcSetValue.Should().Contain("480w"); + srcSetValue.Should().Contain("quality=75"); + srcSetValue.Should().Contain("quality=80"); + } + + [Theory] + [AutoNSubstituteData] + public void Process_SrcSetWithSizesAttribute_AddsBothAttributes( + ImageTagHelper sut, + TagHelperContext tagHelperContext, + TagHelperOutput tagHelperOutput) + { + // Arrange + tagHelperOutput.TagName = "img"; + sut.For = GetModelExpression(new ImageField(_image)); + sut.SrcSet = new object[] { new { w = 320 }, new { w = 480 } }; + sut.Sizes = "(min-width: 768px) 480px, 320px"; + + // Act + sut.Process(tagHelperContext, tagHelperOutput); + + // Assert + tagHelperOutput.Attributes.Should().Contain(a => a.Name == "srcset"); + tagHelperOutput.Attributes.Should().Contain(a => a.Name == "sizes"); + + string sizesValue = tagHelperOutput.Attributes["sizes"].Value.ToString()!; + sizesValue.Should().Be("(min-width: 768px) 480px, 320px"); + } + private static ModelExpression GetModelExpression(Field model) { DefaultModelMetadata? modelMetadata = Substitute.For( From 454e0492cc7b4cb36fdead398bd1f12f71e43492 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 16 Jul 2025 15:33:45 +0530 Subject: [PATCH 08/60] Remove unnessasary GetMediaLink method overloading --- .../Extensions/SitecoreFieldExtensions.cs | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 4534c72..02968f2 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -31,27 +31,6 @@ public static partial class SitecoreFieldExtensions return GetSitecoreMediaUri(urlStr, imageParams); } - /// - /// Gets modified URL string to Sitecore media item with merged parameters. - /// - /// The image field. - /// Base image parameters. - /// SrcSet specific parameters that override imageParams. - /// Media item URL. - public static string? GetMediaLink(this ImageField imageField, object? imageParams, object? srcSetParams) - { - ArgumentNullException.ThrowIfNull(imageField); - string? urlStr = imageField.Value.Src; - - if (urlStr == null) - { - return null; - } - - Dictionary mergedParams = MergeParameters(imageParams, srcSetParams); - return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); - } - /// /// Gets modified URL string to Sitecore media item for srcSet. /// This method preserves existing URL parameters and merges them with new ones. From 43a09f88ca0c60ff62f53868a4e0daaf70890f69 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 16 Jul 2025 16:09:33 +0530 Subject: [PATCH 09/60] Add comment explanation of GetMediaLinkForSrcSet preservation and using StringComparer.OrdinalIgnoreCase for the dictionary to ensure consistent parameter key handling --- .../Extensions/SitecoreFieldExtensions.cs | 6 +++--- .../TagHelpers/Fields/ImageTagHelper.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 02968f2..10e9e1c 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -61,7 +61,7 @@ public static partial class SitecoreFieldExtensions /// Merged parameters as dictionary. private static Dictionary MergeParameters(object? baseParams, object? overrideParams) { - Dictionary result = new Dictionary(); + Dictionary result = new Dictionary(StringComparer.OrdinalIgnoreCase); // Add base parameters first if (baseParams != null) @@ -157,7 +157,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? string url = urlStr; // Parse existing query parameters - Dictionary existingParams = new Dictionary(); + Dictionary existingParams = new Dictionary(StringComparer.OrdinalIgnoreCase); if (url.Contains('?')) { string[] parts = url.Split('?', 2); @@ -178,7 +178,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? } // Merge with new parameters (new parameters override existing ones) - Dictionary mergedParams = new Dictionary(); + Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); // Add existing parameters first foreach (KeyValuePair kvp in existingParams) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 9b996e8..7277347 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -433,7 +433,7 @@ private string GenerateSrcSetAttribute(ImageField imageField) continue; } - // Use GetMediaLinkForSrcSet to preserve existing URL parameters (like ttc, tt, hash, quality, format) + // Use GetMediaLinkForSrcSet to preserve existing URL parameters (like ttc, tt, hash, quality, format) because in preview context id the images doesn't get loaded with src-set implementation string? mediaUrl = imageField.GetMediaLinkForSrcSet(ImageParams, srcSetItem); if (!string.IsNullOrEmpty(mediaUrl)) { From 10c272af8a7455cb0e52c99258bf628a317ad9a6 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:04:28 +0530 Subject: [PATCH 10/60] refactor the code to eliminate the unnecessary dictionary copying by directly creating the final mergedParams dictionary --- .../Extensions/SitecoreFieldExtensions.cs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 10e9e1c..75e0f5b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -156,8 +156,8 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? string url = urlStr; - // Parse existing query parameters - Dictionary existingParams = new Dictionary(StringComparer.OrdinalIgnoreCase); + // Parse existing query parameters and build merged parameters dictionary + Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); if (url.Contains('?')) { string[] parts = url.Split('?', 2); @@ -172,20 +172,11 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? { string key = HttpUtility.UrlDecode(keyValue[0]); string value = HttpUtility.UrlDecode(keyValue[1]); - existingParams[key] = value; + mergedParams[key] = value; } } } - // Merge with new parameters (new parameters override existing ones) - Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); - - // Add existing parameters first - foreach (KeyValuePair kvp in existingParams) - { - mergedParams[kvp.Key] = kvp.Value; - } - // Add new parameters (these will override existing ones) if (parameters != null) { From 21d8af392cadbb10f12a33e3fa86014e01bec330 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:12:07 +0530 Subject: [PATCH 11/60] extract the duplicated URL prefix replacement logic into a separate method - ApplyJssMediaUrlPrefix --- .../Extensions/SitecoreFieldExtensions.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 75e0f5b..a74bae4 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -130,14 +130,7 @@ private static string GetSitecoreMediaUri(string url, object? imageParams) } } - // TODO Review hardcoded matching and replacement - Match match = MediaUrlPrefixRegex().Match(url); - if (match.Success) - { - url = url.Replace(match.Value, $"/{match.Groups[1]}/jssmedia/", StringComparison.InvariantCulture); - } - - return url; + return ApplyJssMediaUrlPrefix(url); } /// @@ -210,6 +203,16 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? } } + return ApplyJssMediaUrlPrefix(url); + } + + /// + /// Applies JSS media URL prefix replacement to the given URL. + /// + /// The URL to transform. + /// The URL with JSS media prefix applied if applicable. + private static string ApplyJssMediaUrlPrefix(string url) + { // TODO Review hardcoded matching and replacement Match match = MediaUrlPrefixRegex().Match(url); if (match.Success) From 87fabe50f373be59c21a5e22024340ebb218f3c7 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:26:49 +0530 Subject: [PATCH 12/60] reorder the methods below the properties in ImageTagHelper.cs - SDK.RenderingEngine --- .../TagHelpers/Fields/ImageTagHelper.cs | 216 +++++++++--------- 1 file changed, 108 insertions(+), 108 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 7277347..d4895d3 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -34,114 +34,6 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper private const string SizesAttribute = "sizes"; private readonly IEditableChromeRenderer _chromeRenderer = chromeRenderer ?? throw new ArgumentNullException(nameof(chromeRenderer)); - private static string? GetWidthDescriptor(object? parameters) - { - if (parameters == null) - { - return null; - } - - string? width = null; - - // Handle Dictionary - if (parameters is Dictionary dictionary) - { - // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - if (dictionary.TryGetValue("w", out object? wValue)) - { - width = wValue?.ToString(); - } - else if (dictionary.TryGetValue("mw", out object? mwValue)) - { - width = mwValue?.ToString(); - } - else if (dictionary.TryGetValue("width", out object? widthObj)) - { - width = widthObj?.ToString(); - } - else if (dictionary.TryGetValue("maxWidth", out object? maxWidthObj)) - { - width = maxWidthObj?.ToString(); - } - } - else - { - // Handle anonymous objects via reflection - PropertyInfo[] properties = parameters.GetType().GetProperties(); - - // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - PropertyInfo? wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); - if (wProp != null) - { - width = wProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); - if (mwProp != null) - { - width = mwProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); - if (widthProp != null) - { - width = widthProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); - if (maxWidthProp != null) - { - width = maxWidthProp.GetValue(parameters)?.ToString(); - } - } - } - } - } - - // Validate width is positive - if (width != null && int.TryParse(width, out int widthValue) && widthValue <= 0) - { - return null; - } - - return width != null ? $"{width}w" : null; - } - - private static object[]? ParseSrcSet(object? srcSetValue) - { - if (srcSetValue == null) - { - return null; - } - - // If already an object array, use as-is - if (srcSetValue is object[] objectArray) - { - return objectArray; - } - - // If it's a JSON string, parse it - if (srcSetValue is string jsonString) - { - try - { - Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); - return parsed?.Cast().ToArray(); - } - catch (System.Text.Json.JsonException) - { - // If JSON parsing fails, return null to skip srcset generation - return null; - } - } - - // Single object - wrap in array - return new[] { srcSetValue }; - } - /// /// Gets or sets the model value. /// @@ -281,6 +173,114 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } } + private static string? GetWidthDescriptor(object? parameters) + { + if (parameters == null) + { + return null; + } + + string? width = null; + + // Handle Dictionary + if (parameters is Dictionary dictionary) + { + // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) + if (dictionary.TryGetValue("w", out object? wValue)) + { + width = wValue?.ToString(); + } + else if (dictionary.TryGetValue("mw", out object? mwValue)) + { + width = mwValue?.ToString(); + } + else if (dictionary.TryGetValue("width", out object? widthObj)) + { + width = widthObj?.ToString(); + } + else if (dictionary.TryGetValue("maxWidth", out object? maxWidthObj)) + { + width = maxWidthObj?.ToString(); + } + } + else + { + // Handle anonymous objects via reflection + PropertyInfo[] properties = parameters.GetType().GetProperties(); + + // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) + PropertyInfo? wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); + if (wProp != null) + { + width = wProp.GetValue(parameters)?.ToString(); + } + else + { + PropertyInfo? mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); + if (mwProp != null) + { + width = mwProp.GetValue(parameters)?.ToString(); + } + else + { + PropertyInfo? widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); + if (widthProp != null) + { + width = widthProp.GetValue(parameters)?.ToString(); + } + else + { + PropertyInfo? maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); + if (maxWidthProp != null) + { + width = maxWidthProp.GetValue(parameters)?.ToString(); + } + } + } + } + } + + // Validate width is positive + if (width != null && int.TryParse(width, out int widthValue) && widthValue <= 0) + { + return null; + } + + return width != null ? $"{width}w" : null; + } + + private static object[]? ParseSrcSet(object? srcSetValue) + { + if (srcSetValue == null) + { + return null; + } + + // If already an object array, use as-is + if (srcSetValue is object[] objectArray) + { + return objectArray; + } + + // If it's a JSON string, parse it + if (srcSetValue is string jsonString) + { + try + { + Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); + return parsed?.Cast().ToArray(); + } + catch (System.Text.Json.JsonException) + { + // If JSON parsing fails, return null to skip srcset generation + return null; + } + } + + // Single object - wrap in array + return new[] { srcSetValue }; + } + private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) { Image image = imageField.Value; From 8b1bff55a73a87abf71c8d4619a7b4f57ec6f664 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:31:24 +0530 Subject: [PATCH 13/60] updated ParseSrcSet in - ImageTagHelper.cs code to use the [srcSetValue] return --- .../TagHelpers/Fields/ImageTagHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index d4895d3..7a048a6 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -278,7 +278,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } // Single object - wrap in array - return new[] { srcSetValue }; + return [srcSetValue]; } private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) From 3e0cb929a45c7624f25a0a70f888cce0873e65b0 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:43:42 +0530 Subject: [PATCH 14/60] Remove unnessasary comments in ImageTagHelper and refactor ParseSrcSet to ParseJsonSrcSet, move the type checks to the calling method and simplifying ParseJsonSrcSet --- .../TagHelpers/Fields/ImageTagHelper.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 7a048a6..9aa9afb 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -114,7 +114,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) { output.Attributes.Add(ScrAttribute, field.GetMediaLink(ImageParams)); - // Add srcset if configured if (SrcSet != null) { string srcSetValue = GenerateSrcSetAttribute(field); @@ -124,7 +123,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } } - // Add sizes if provided if (!string.IsNullOrEmpty(Sizes)) { output.Attributes.Add(SizesAttribute, Sizes); @@ -249,7 +247,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return width != null ? $"{width}w" : null; } - private static object[]? ParseSrcSet(object? srcSetValue) + private static object[]? ParseJsonSrcSet(object? srcSetValue) { if (srcSetValue == null) { @@ -293,7 +291,6 @@ private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) { tagBuilder.Attributes.Add(ScrAttribute, imageField.GetMediaLink(ImageParams)); - // Add srcset if configured if (SrcSet != null) { string srcSetValue = GenerateSrcSetAttribute(imageField); @@ -303,7 +300,6 @@ private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) } } - // Add sizes if provided if (!string.IsNullOrEmpty(Sizes)) { tagBuilder.Attributes.Add(SizesAttribute, Sizes); @@ -387,7 +383,6 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp imageNode.SetAttributeValue(ScrAttribute, imageField.GetMediaLink(ImageParams)); - // Add srcset for editable content if (SrcSet != null) { string srcSetValue = GenerateSrcSetAttribute(imageField); @@ -397,7 +392,6 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp } } - // Add sizes for editable content if (!string.IsNullOrEmpty(Sizes)) { imageNode.SetAttributeValue(SizesAttribute, Sizes); @@ -409,7 +403,7 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp private string GenerateSrcSetAttribute(ImageField imageField) { - object[]? parsedSrcSet = ParseSrcSet(SrcSet); + object[]? parsedSrcSet = ParseJsonSrcSet(SrcSet); if (parsedSrcSet == null || parsedSrcSet.Length == 0) { return string.Empty; From 99aa164f4c68e168601d3f4c4c21a61d8688cc3d Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 09:44:04 +0530 Subject: [PATCH 15/60] Remove unnessasary comments in ImageTagHelper and refactor ParseSrcSet to ParseJsonSrcSet, move the type checks to the calling method and simplifying ParseJsonSrcSet --- .../TagHelpers/Fields/ImageTagHelper.cs | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 9aa9afb..e1f8368 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -247,19 +247,8 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return width != null ? $"{width}w" : null; } - private static object[]? ParseJsonSrcSet(object? srcSetValue) + private static object[]? ParseJsonSrcSet(object srcSetValue) { - if (srcSetValue == null) - { - return null; - } - - // If already an object array, use as-is - if (srcSetValue is object[] objectArray) - { - return objectArray; - } - // If it's a JSON string, parse it if (srcSetValue is string jsonString) { @@ -403,7 +392,24 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp private string GenerateSrcSetAttribute(ImageField imageField) { - object[]? parsedSrcSet = ParseJsonSrcSet(SrcSet); + if (SrcSet == null) + { + return string.Empty; + } + + object[]? parsedSrcSet; + + // If already an object array, use as-is + if (SrcSet is object[] objectArray) + { + parsedSrcSet = objectArray; + } + else + { + // Parse JSON string or wrap single object + parsedSrcSet = ParseJsonSrcSet(SrcSet); + } + if (parsedSrcSet == null || parsedSrcSet.Length == 0) { return string.Empty; From ac23fcb2c986339ecd28fbbd991cc8aa87a48666 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:13:16 +0530 Subject: [PATCH 16/60] Throw exception in invalid JSON passing in ParseJsonSrcSet, updated the test cases to validate the fix --- .../TagHelpers/Fields/ImageTagHelper.cs | 6 +++--- .../TagHelpers/Fields/ImageTagHelperFixture.cs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index e1f8368..a917fd0 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -257,10 +257,10 @@ public override void Process(TagHelperContext context, TagHelperOutput output) Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); return parsed?.Cast().ToArray(); } - catch (System.Text.Json.JsonException) + catch (Exception ex) { - // If JSON parsing fails, return null to skip srcset generation - return null; + // JSON parsing failed - this is a programming error, invalid JSON was passed + throw new InvalidOperationException($"Failed to parse srcset JSON: {jsonString}", ex); } } diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index ab92c4f..a9e4c91 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -902,7 +902,7 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( [Theory] [AutoNSubstituteData] - public void Process_SrcSetWithInvalidJsonString_FallsBackGracefully( + public void Process_SrcSetWithInvalidJsonString_ThrowsInvalidOperationException( ImageTagHelper sut, TagHelperContext tagHelperContext, TagHelperOutput tagHelperOutput) @@ -912,12 +912,12 @@ public void Process_SrcSetWithInvalidJsonString_FallsBackGracefully( sut.For = GetModelExpression(new ImageField(_image)); sut.SrcSet = "invalid json string"; - // Act - sut.Process(tagHelperContext, tagHelperOutput); - - // Assert - // Should not throw and should not add srcset attribute - tagHelperOutput.Attributes.Should().NotContain(a => a.Name == "srcset"); + // Act & Assert + // Should throw InvalidOperationException due to invalid JSON + Action act = () => sut.Process(tagHelperContext, tagHelperOutput); + act.Should().Throw() + .WithMessage("Failed to parse srcset JSON: invalid json string*") + .WithInnerException(); } [Theory] From f87a037b2b0553121e1b68b7b396c523cc68eee1 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:29:37 +0530 Subject: [PATCH 17/60] Add clarification comment for the use of Dictionary[] --- .../TagHelpers/Fields/ImageTagHelper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index a917fd0..396d0e9 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -254,6 +254,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) { try { + // We need to use Dictionary[] to ensure proper deserialization of JSON objects into dictionaries that our GetWidthDescriptor method can handle Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); return parsed?.Cast().ToArray(); } From edd2bfd50477f1816950878aa2216d0678d81201 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:39:45 +0530 Subject: [PATCH 18/60] optimize this by using LINQ's Where() - in GenerateSrcSetAttribute --- .../TagHelpers/Fields/ImageTagHelper.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 396d0e9..c4afa5a 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -418,14 +418,9 @@ private string GenerateSrcSetAttribute(ImageField imageField) List srcSetEntries = new(); - foreach (object srcSetItem in parsedSrcSet) + // filter out null items directly + foreach (object srcSetItem in parsedSrcSet.Where(item => item != null)) { - // Skip null items - if (srcSetItem == null) - { - continue; - } - // Get width descriptor first to check if this entry should be included string? descriptor = GetWidthDescriptor(srcSetItem); if (descriptor == null) From efe5b45013d7cf0786f586de8d96e9ba430e3b8a Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:42:25 +0530 Subject: [PATCH 19/60] Remove Unnecessary comment --- .../TagHelpers/Fields/ImageTagHelper.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index c4afa5a..ec2a8b6 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -180,7 +180,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) string? width = null; - // Handle Dictionary if (parameters is Dictionary dictionary) { // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) From ad66f66a1934ce5a3d6b181c61709bbd8679a7ef Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:46:14 +0530 Subject: [PATCH 20/60] Add readable approach in priority checking in GetWidthDescriptor method --- .../TagHelpers/Fields/ImageTagHelper.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index ec2a8b6..b2911de 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -183,21 +183,21 @@ public override void Process(TagHelperContext context, TagHelperOutput output) if (parameters is Dictionary dictionary) { // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - if (dictionary.TryGetValue("w", out object? wValue)) + if (dictionary.ContainsKey("w")) { - width = wValue?.ToString(); + width = dictionary["w"]?.ToString(); } - else if (dictionary.TryGetValue("mw", out object? mwValue)) + else if (dictionary.ContainsKey("mw")) { - width = mwValue?.ToString(); + width = dictionary["mw"]?.ToString(); } - else if (dictionary.TryGetValue("width", out object? widthObj)) + else if (dictionary.ContainsKey("width")) { - width = widthObj?.ToString(); + width = dictionary["width"]?.ToString(); } - else if (dictionary.TryGetValue("maxWidth", out object? maxWidthObj)) + else if (dictionary.ContainsKey("maxWidth")) { - width = maxWidthObj?.ToString(); + width = dictionary["maxWidth"]?.ToString(); } } else From 6e62708b4c6667208154f3b8a1139d2fc0f0776e Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:48:32 +0530 Subject: [PATCH 21/60] Remove unnecessary comment --- .../TagHelpers/Fields/ImageTagHelper.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index b2911de..e0b4654 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -202,7 +202,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } else { - // Handle anonymous objects via reflection PropertyInfo[] properties = parameters.GetType().GetProperties(); // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) From 6ccd0bd5812cb60b479e048c4abc8f09ccc3ea97 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 10:52:19 +0530 Subject: [PATCH 22/60] refactor the GetWidthDescriptor method to use a helper method and the null-coalescing pattern for better readability --- .../TagHelpers/Fields/ImageTagHelper.cs | 39 +++++-------------- 1 file changed, 10 insertions(+), 29 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index e0b4654..4d9ed9b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -171,6 +171,12 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } } + private static string? TryParseParameter(object parameters, string paramName, PropertyInfo[] properties) + { + PropertyInfo? prop = properties.FirstOrDefault(p => p.Name.Equals(paramName, StringComparison.OrdinalIgnoreCase)); + return prop?.GetValue(parameters)?.ToString(); + } + private static string? GetWidthDescriptor(object? parameters) { if (parameters == null) @@ -205,35 +211,10 @@ public override void Process(TagHelperContext context, TagHelperOutput output) PropertyInfo[] properties = parameters.GetType().GetProperties(); // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - PropertyInfo? wProp = properties.FirstOrDefault(p => p.Name.Equals("w", StringComparison.OrdinalIgnoreCase)); - if (wProp != null) - { - width = wProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? mwProp = properties.FirstOrDefault(p => p.Name.Equals("mw", StringComparison.OrdinalIgnoreCase)); - if (mwProp != null) - { - width = mwProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? widthProp = properties.FirstOrDefault(p => p.Name.Equals("width", StringComparison.OrdinalIgnoreCase)); - if (widthProp != null) - { - width = widthProp.GetValue(parameters)?.ToString(); - } - else - { - PropertyInfo? maxWidthProp = properties.FirstOrDefault(p => p.Name.Equals("maxWidth", StringComparison.OrdinalIgnoreCase)); - if (maxWidthProp != null) - { - width = maxWidthProp.GetValue(parameters)?.ToString(); - } - } - } - } + width = TryParseParameter(parameters, "w", properties) + ?? TryParseParameter(parameters, "mw", properties) + ?? TryParseParameter(parameters, "width", properties) + ?? TryParseParameter(parameters, "maxWidth", properties); } // Validate width is positive From 2d3978f50aad5b4e92eebf778732b51db8ac16cf Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 17 Jul 2025 12:51:01 +0530 Subject: [PATCH 23/60] refactor: improve ImageTagHelper test assertions with precise string comparisons - Replace Contains() assertions with exact string matching for srcset validation - Use full URL verification including preserved query parameters and HTML encoding - Rename Process_SrcSetWithImageParamsConflict_SrcSetParametersWin to Process_SrcSetWithImageParamsConflict_SrcSetParametersArePreferred - Consolidate image object usage by replacing custom Image instances with existing _image field --- .../Fields/ImageTagHelperFixture.cs | 150 +++++++++--------- 1 file changed, 78 insertions(+), 72 deletions(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index a9e4c91..edba006 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -754,11 +754,20 @@ public void Process_ScImgTagWithSrcSet_GeneratesSrcSetAttribute( // Act sut.Process(tagHelperContext, tagHelperOutput); - // Assert - generates HTML content, not attributes + // Assert string content = tagHelperOutput.Content.GetContent(); - content.Should().Contain("srcset="); - content.Should().Contain("400w"); - content.Should().Contain("200w"); + + var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + srcsetMatch.Success.Should().BeTrue("srcset attribute should be present in the HTML"); + + string srcsetValue = srcsetMatch.Groups[1].Value; + + // Verify each entry precisely with preserved query parameters and new width parameters + srcsetValue.Should().Contain("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=400 400w"); + srcsetValue.Should().Contain("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=200 200w"); + + var entries = srcsetValue.Split(", "); + entries.Should().HaveCount(2); } [Theory] @@ -781,17 +790,11 @@ public void Process_SrcSetWithContentSDKBehavior_MatchesExpectedOutput( tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - // Should contain "1000w" (from w parameter taking priority over h) - srcSetValue.Should().Contain("1000w"); - - // Should contain "250w" (from mw parameter) - srcSetValue.Should().Contain("250w"); - - // Verify it contains the expected format: "url 1000w, url 250w" + // Full string comparison - verify exact srcset format string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(2); - entries[0].Should().EndWith("1000w"); - entries[1].Should().EndWith("250w"); + entries[0].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&h=1000&w=1000 1000w"); + entries[1].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&h=1000&mh=250&mw=250 250w"); } [Theory] @@ -813,10 +816,9 @@ public void Process_SrcSetWithoutValidWidth_FiltersEntries( tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - // Should only contain the entry with mw parameter - srcSetValue.Should().Contain("250w"); + // Full string comparison - verify exact entry + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=250 250w"); - // Should not contain entries without width parameters string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(1); } @@ -829,10 +831,9 @@ public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( TagHelperOutput tagHelperOutput) { // Arrange - Image image = new() { Src = "http://styleguide/-/media/img/sc_logo.png", Alt = "Sitecore Logo" }; - ImageField imageField = new(image) + ImageField imageField = new(_image) { - EditableMarkup = "\"Sitecore" + EditableMarkup = "\"Sitecore" }; tagHelperOutput.TagName = RenderingEngineConstants.SitecoreTagHelpers.ImageHtmlTag; @@ -845,10 +846,19 @@ public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( // Assert string content = tagHelperOutput.Content.GetContent(); - content.Should().Contain("srcset="); - content.Should().Contain("600w"); - content.Should().Contain("300w"); - content.Should().Contain("sizes=\"(min-width: 768px) 600px, 300px\""); + + // Extract srcset and sizes using regex for full string comparison + var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + srcsetMatch.Success.Should().BeTrue(); + string srcsetValue = srcsetMatch.Groups[1].Value; + + var sizesMatch = System.Text.RegularExpressions.Regex.Match(content, "sizes=\"([^\"]*)\""); + sizesMatch.Success.Should().BeTrue(); + string sizesValue = sizesMatch.Groups[1].Value; + + // Full string comparison + srcsetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=600 600w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=300 300w"); + sizesValue.Should().Be("(min-width: 768px) 600px, 300px"); } [Theory] @@ -869,8 +879,9 @@ public void Process_SrcSetWithJsonString_GeneratesSrcSetAttribute( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("500w"); - srcSetValue.Should().Contain("250w"); + + // Full string comparison + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=500 500w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=250 250w"); } [Theory] @@ -885,7 +896,6 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( sut.For = GetModelExpression(new ImageField(_image)); sut.SrcSet = new object[] { - // Only w and mw are supported by Content SDK for srcSet new { w = 800 }, // Anonymous object with 'w' new { mw = 400 }, // Anonymous object with 'mw' }; @@ -895,9 +905,14 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( // Assert string content = tagHelperOutput.Content.GetContent(); - content.Should().Contain("srcset="); - content.Should().Contain("800w"); - content.Should().Contain("400w"); + + // Extract srcset using regex for full string comparison + var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + srcsetMatch.Success.Should().BeTrue(); + string srcsetValue = srcsetMatch.Groups[1].Value; + + // Full string comparison + srcsetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=800 800w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=400 400w"); } [Theory] @@ -913,7 +928,6 @@ public void Process_SrcSetWithInvalidJsonString_ThrowsInvalidOperationException( sut.SrcSet = "invalid json string"; // Act & Assert - // Should throw InvalidOperationException due to invalid JSON Action act = () => sut.Process(tagHelperContext, tagHelperOutput); act.Should().Throw() .WithMessage("Failed to parse srcset JSON: invalid json string*") @@ -922,7 +936,7 @@ public void Process_SrcSetWithInvalidJsonString_ThrowsInvalidOperationException( [Theory] [AutoNSubstituteData] - public void Process_SrcSetWithImageParamsConflict_SrcSetParametersWin( + public void Process_SrcSetWithImageParamsConflict_SrcSetParametersArePreferred( ImageTagHelper sut, TagHelperContext tagHelperContext, TagHelperOutput tagHelperOutput) @@ -939,11 +953,9 @@ public void Process_SrcSetWithImageParamsConflict_SrcSetParametersWin( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("w=320"); - srcSetValue.Should().Contain("quality=75"); - srcSetValue.Should().Contain("format=jpg"); // Inherited from ImageParams - srcSetValue.Should().NotContain("w=1000"); - srcSetValue.Should().NotContain("quality=50"); + + // Full string comparison - verify SrcSet parameters are preferred over ImageParams + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320&quality=75&format=jpg 320w"); } [Theory] @@ -970,10 +982,14 @@ public void Process_SrcSetWithWidthParameterPriority_UsesCorrectPrecedence( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("100w"); - srcSetValue.Should().Contain("200w"); - srcSetValue.Should().Contain("300w"); - srcSetValue.Should().Contain("400w"); + + // Full string comparison - verify parameter priority + string[] entries = srcSetValue.Split(", "); + entries.Should().HaveCount(4); + entries[0].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=100&mw=200&width=300&maxWidth=400 100w"); + entries[1].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=200&width=300&maxWidth=400 200w"); + entries[2].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&width=300&maxWidth=400 300w"); + entries[3].Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&maxWidth=400 400w"); } [Theory] @@ -1000,11 +1016,10 @@ public void Process_SrcSetWithZeroOrNegativeWidths_SkipsInvalidEntries( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("320w"); - srcSetValue.Should().NotContain(" 0w"); - srcSetValue.Should().NotContain(" -100w"); - // Should only have one entry + // Full string comparison - verify only valid entry remains + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320&quality=75 320w"); + string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(1); } @@ -1034,16 +1049,8 @@ public void Process_SrcSetWithExistingUrlParameters_PreservesAndMergesParameters tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - // Should preserve existing parameters and merge with new ones - srcSetValue.Should().Contain("h=2001"); - srcSetValue.Should().Contain("iar=0"); - srcSetValue.Should().Contain("hash=abc123"); - srcSetValue.Should().Contain("quality=75"); - - // New w parameter should override existing w=3000 - srcSetValue.Should().Contain("w=320"); - srcSetValue.Should().NotContain("w=3000"); - srcSetValue.Should().Contain("320w"); + // Full string comparison - verify parameters are preserved and merged + srcSetValue.Should().Be("https://edge.sitecorecloud.io/media/image.jpg?h=2001&iar=0&hash=abc123&w=320&quality=75 320w"); } [Theory] @@ -1071,10 +1078,8 @@ public void Process_SrcSetWithMediaUrlTransformation_TransformsCorrectly( tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - // Should transform /~/media/ to /~/jssmedia/ - srcSetValue.Should().Contain("/~/jssmedia/images/test.jpg"); - srcSetValue.Should().NotContain("/~/media/images/test.jpg"); - srcSetValue.Should().Contain("320w"); + // Full string comparison - verify URL transformation + srcSetValue.Should().Be("/~/jssmedia/images/test.jpg?w=320&quality=75 320w"); } [Theory] @@ -1101,10 +1106,10 @@ public void Process_SrcSetWithNullAndEmptyEntries_HandlesGracefully( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("320w"); - srcSetValue.Should().Contain("480w"); - // Should only have two entries + // Full string comparison - verify only valid entries remain + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320&quality=75 320w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=480&quality=80 480w"); + string[] entries = srcSetValue.Split(", "); entries.Should().HaveCount(2); } @@ -1130,11 +1135,9 @@ public void Process_SrcSetWithComplexParameters_HandlesAllParameters( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("320w"); - srcSetValue.Should().Contain("quality=75"); - srcSetValue.Should().Contain("format=webp"); - srcSetValue.Should().Contain("dpr=2"); - srcSetValue.Should().Contain("fit=crop"); + + // Full string comparison - verify all parameters are included + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320&quality=75&format=webp&dpr=2&fit=crop 320w"); } [Theory] @@ -1159,10 +1162,9 @@ public void Process_SrcSetWithDictionaryParameters_GeneratesSrcSetAttribute( // Assert tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - srcSetValue.Should().Contain("320w"); - srcSetValue.Should().Contain("480w"); - srcSetValue.Should().Contain("quality=75"); - srcSetValue.Should().Contain("quality=80"); + + // Full string comparison - verify dictionary parameters + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320&quality=75 320w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=480&quality=80 480w"); } [Theory] @@ -1182,10 +1184,14 @@ public void Process_SrcSetWithSizesAttribute_AddsBothAttributes( sut.Process(tagHelperContext, tagHelperOutput); // Assert - tagHelperOutput.Attributes.Should().Contain(a => a.Name == "srcset"); - tagHelperOutput.Attributes.Should().Contain(a => a.Name == "sizes"); + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); + tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "sizes"); + string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; string sizesValue = tagHelperOutput.Attributes["sizes"].Value.ToString()!; + + // Full string comparison + srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=320 320w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=480 480w"); sizesValue.Should().Be("(min-width: 768px) 480px, 320px"); } From fec93513ac290f4648dfb5770b8de56d311ed570 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 18 Jul 2025 11:04:48 +0530 Subject: [PATCH 24/60] remove the intermediate variable and casting in ParseJsonSrcSet --- .../TagHelpers/Fields/ImageTagHelper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 4d9ed9b..242b064 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -6,6 +6,7 @@ using Microsoft.AspNetCore.Razor.TagHelpers; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Properties; +using Sitecore.AspNetCore.SDK.LayoutService.Client.Serialization; using Sitecore.AspNetCore.SDK.RenderingEngine.Extensions; using Sitecore.AspNetCore.SDK.RenderingEngine.Rendering; @@ -234,8 +235,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) try { // We need to use Dictionary[] to ensure proper deserialization of JSON objects into dictionaries that our GetWidthDescriptor method can handle - Dictionary[]? parsed = System.Text.Json.JsonSerializer.Deserialize[]>(jsonString); - return parsed?.Cast().ToArray(); + return System.Text.Json.JsonSerializer.Deserialize[]>(jsonString, JsonLayoutServiceSerializer.GetDefaultSerializerOptions()); } catch (Exception ex) { From 2b3c82dbfbec7dc665b657441d11622ca9cdfba8 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 18 Jul 2025 11:21:00 +0530 Subject: [PATCH 25/60] Remove some unnecessary comments used in ImageTagHelper --- .../TagHelpers/Fields/ImageTagHelper.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 242b064..b8811bd 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -211,14 +211,13 @@ public override void Process(TagHelperContext context, TagHelperOutput output) { PropertyInfo[] properties = parameters.GetType().GetProperties(); - // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) + // Priority: w > mw > width > maxWidth width = TryParseParameter(parameters, "w", properties) ?? TryParseParameter(parameters, "mw", properties) ?? TryParseParameter(parameters, "width", properties) ?? TryParseParameter(parameters, "maxWidth", properties); } - // Validate width is positive if (width != null && int.TryParse(width, out int widthValue) && widthValue <= 0) { return null; @@ -229,7 +228,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) private static object[]? ParseJsonSrcSet(object srcSetValue) { - // If it's a JSON string, parse it if (srcSetValue is string jsonString) { try @@ -244,7 +242,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } } - // Single object - wrap in array return [srcSetValue]; } @@ -379,14 +376,12 @@ private string GenerateSrcSetAttribute(ImageField imageField) object[]? parsedSrcSet; - // If already an object array, use as-is if (SrcSet is object[] objectArray) { parsedSrcSet = objectArray; } else { - // Parse JSON string or wrap single object parsedSrcSet = ParseJsonSrcSet(SrcSet); } From b0887ee38b678d445b68a010d11439c61888cb04 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 21 Jul 2025 09:11:09 +0530 Subject: [PATCH 26/60] refactor: rename MergeParameters method parameters for clarity - Rename aseParams to imageParams to better reflect base image parameters - Rename overrideParams to srcSetParams to indicate srcset-specific parameters --- .../Extensions/SitecoreFieldExtensions.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a74bae4..fc813e4 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -56,17 +56,17 @@ public static partial class SitecoreFieldExtensions /// /// Merges base parameters with override parameters. /// - /// Base parameters. - /// Override parameters that take precedence. + /// Base image parameters. + /// SrcSet specific parameters that take precedence. /// Merged parameters as dictionary. - private static Dictionary MergeParameters(object? baseParams, object? overrideParams) + private static Dictionary MergeParameters(object? imageParams, object? srcSetParams) { Dictionary result = new Dictionary(StringComparer.OrdinalIgnoreCase); // Add base parameters first - if (baseParams != null) + if (imageParams != null) { - if (baseParams is Dictionary baseDict) + if (imageParams is Dictionary baseDict) { foreach (KeyValuePair kvp in baseDict) { @@ -75,18 +75,18 @@ public static partial class SitecoreFieldExtensions } else { - PropertyInfo[] baseProps = baseParams.GetType().GetProperties(); + PropertyInfo[] baseProps = imageParams.GetType().GetProperties(); foreach (PropertyInfo prop in baseProps) { - result[prop.Name] = prop.GetValue(baseParams); + result[prop.Name] = prop.GetValue(imageParams); } } } // Override with srcSet parameters - if (overrideParams != null) + if (srcSetParams != null) { - if (overrideParams is Dictionary overrideDict) + if (srcSetParams is Dictionary overrideDict) { foreach (KeyValuePair kvp in overrideDict) { @@ -95,10 +95,10 @@ public static partial class SitecoreFieldExtensions } else { - PropertyInfo[] overrideProps = overrideParams.GetType().GetProperties(); + PropertyInfo[] overrideProps = srcSetParams.GetType().GetProperties(); foreach (PropertyInfo prop in overrideProps) { - result[prop.Name] = prop.GetValue(overrideParams); + result[prop.Name] = prop.GetValue(srcSetParams); } } } From fe0a9fe66bc9def7ecc8e40a22499799e0f2325c Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 21 Jul 2025 09:22:42 +0530 Subject: [PATCH 27/60] refactor: extract duplicate parameter processing logic into reusable method - Create AddParametersToResult helper method to handle both Dictionary and reflection-based parameter processing - Add skipNullValues parameter to handle different null value handling requirements - Refactor MergeParameters to use the new helper method, eliminating code duplication - Update GetSitecoreMediaUriWithPreservation to use the helper method with skipNullValues: true --- .../Extensions/SitecoreFieldExtensions.cs | 78 +++++++------------ 1 file changed, 30 insertions(+), 48 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index fc813e4..88283b7 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -64,46 +64,49 @@ public static partial class SitecoreFieldExtensions Dictionary result = new Dictionary(StringComparer.OrdinalIgnoreCase); // Add base parameters first - if (imageParams != null) + AddParametersToResult(result, imageParams); + + // Override with srcSet parameters + AddParametersToResult(result, srcSetParams); + + return result; + } + + /// + /// Adds parameters from an object to the result dictionary. + /// + /// The result dictionary to add parameters to. + /// The parameters object (can be Dictionary or any object with properties). + /// Whether to skip null values when adding parameters. + private static void AddParametersToResult(Dictionary result, object? parameters, bool skipNullValues = false) + { + if (parameters == null) { - if (imageParams is Dictionary baseDict) - { - foreach (KeyValuePair kvp in baseDict) - { - result[kvp.Key] = kvp.Value; - } - } - else - { - PropertyInfo[] baseProps = imageParams.GetType().GetProperties(); - foreach (PropertyInfo prop in baseProps) - { - result[prop.Name] = prop.GetValue(imageParams); - } - } + return; } - // Override with srcSet parameters - if (srcSetParams != null) + if (parameters is Dictionary paramDict) { - if (srcSetParams is Dictionary overrideDict) + foreach (KeyValuePair kvp in paramDict) { - foreach (KeyValuePair kvp in overrideDict) + if (!skipNullValues || kvp.Value != null) { result[kvp.Key] = kvp.Value; } } - else + } + else + { + PropertyInfo[] properties = parameters.GetType().GetProperties(); + foreach (PropertyInfo prop in properties) { - PropertyInfo[] overrideProps = srcSetParams.GetType().GetProperties(); - foreach (PropertyInfo prop in overrideProps) + object? value = prop.GetValue(parameters); + if (!skipNullValues || value != null) { - result[prop.Name] = prop.GetValue(srcSetParams); + result[prop.Name] = value; } } } - - return result; } /// @@ -171,28 +174,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? } // Add new parameters (these will override existing ones) - if (parameters != null) - { - if (parameters is Dictionary paramDict) - { - foreach (KeyValuePair kvp in paramDict) - { - mergedParams[kvp.Key] = kvp.Value; - } - } - else - { - PropertyInfo[] properties = parameters.GetType().GetProperties(); - foreach (PropertyInfo prop in properties) - { - object? value = prop.GetValue(parameters); - if (value != null) - { - mergedParams[prop.Name] = value; - } - } - } - } + AddParametersToResult(mergedParams, parameters, skipNullValues: true); // Add query parameters foreach (KeyValuePair kvp in mergedParams) From d40864e364ed25dc8aef9fd0253133cf7991ced9 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 21 Jul 2025 09:28:18 +0530 Subject: [PATCH 28/60] refactor: extract URL parameter parsing into separate ParseUrlParams method - ParseUrlParams --- .../Extensions/SitecoreFieldExtensions.cs | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 88283b7..b2f931f 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -160,17 +160,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? url = parts[0]; string queryString = parts[1]; - string[] paramPairs = queryString.Split('&'); - foreach (string paramPair in paramPairs) - { - string[] keyValue = paramPair.Split('=', 2); - if (keyValue.Length == 2) - { - string key = HttpUtility.UrlDecode(keyValue[0]); - string value = HttpUtility.UrlDecode(keyValue[1]); - mergedParams[key] = value; - } - } + ParseUrlParams(queryString, mergedParams); } // Add new parameters (these will override existing ones) @@ -188,6 +178,26 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? return ApplyJssMediaUrlPrefix(url); } + /// + /// Parses URL query string parameters and adds them to the provided dictionary. + /// + /// The query string to parse. + /// The dictionary to add parsed parameters to. + private static void ParseUrlParams(string queryString, Dictionary parameters) + { + string[] paramPairs = queryString.Split('&'); + foreach (string paramPair in paramPairs) + { + string[] keyValue = paramPair.Split('=', 2); + if (keyValue.Length == 2) + { + string key = HttpUtility.UrlDecode(keyValue[0]); + string value = HttpUtility.UrlDecode(keyValue[1]); + parameters[key] = value; + } + } + } + /// /// Applies JSS media URL prefix replacement to the given URL. /// From b3407d6c77cb7890635cb31769fa3904a4cd600c Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 21 Jul 2025 09:48:14 +0530 Subject: [PATCH 29/60] refactor: add using System.Text.Json to remove fully qualified name - Add using System.Text.Json directive to imports --- .../TagHelpers/Fields/ImageTagHelper.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index b8811bd..d8d9453 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -1,4 +1,5 @@ using System.Reflection; +using System.Text.Json; using HtmlAgilityPack; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Rendering; @@ -233,7 +234,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) try { // We need to use Dictionary[] to ensure proper deserialization of JSON objects into dictionaries that our GetWidthDescriptor method can handle - return System.Text.Json.JsonSerializer.Deserialize[]>(jsonString, JsonLayoutServiceSerializer.GetDefaultSerializerOptions()); + return JsonSerializer.Deserialize[]>(jsonString, JsonLayoutServiceSerializer.GetDefaultSerializerOptions()); } catch (Exception ex) { From 1343dc7440247d09b5909afe4d5a79d4979c673d Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 22 Jul 2025 08:20:49 +0530 Subject: [PATCH 30/60] refactor: move complete URL parsing logic into ParseUrlParams method --- .../Extensions/SitecoreFieldExtensions.cs | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index b2f931f..bb83011 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -154,14 +154,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? // Parse existing query parameters and build merged parameters dictionary Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); - if (url.Contains('?')) - { - string[] parts = url.Split('?', 2); - url = parts[0]; - string queryString = parts[1]; - - ParseUrlParams(queryString, mergedParams); - } + url = ParseUrlParams(url, mergedParams); // Add new parameters (these will override existing ones) AddParametersToResult(mergedParams, parameters, skipNullValues: true); @@ -181,21 +174,33 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? /// /// Parses URL query string parameters and adds them to the provided dictionary. /// - /// The query string to parse. + /// The full URL with potential query parameters. /// The dictionary to add parsed parameters to. - private static void ParseUrlParams(string queryString, Dictionary parameters) + /// The URL without query parameters. + private static string ParseUrlParams(string url, Dictionary parameters) { - string[] paramPairs = queryString.Split('&'); - foreach (string paramPair in paramPairs) + if (url.Contains('?')) { - string[] keyValue = paramPair.Split('=', 2); - if (keyValue.Length == 2) + string[] parts = url.Split('?', 2); + string cleanUrl = parts[0]; + string queryString = parts[1]; + + string[] paramPairs = queryString.Split('&'); + foreach (string paramPair in paramPairs) { - string key = HttpUtility.UrlDecode(keyValue[0]); - string value = HttpUtility.UrlDecode(keyValue[1]); - parameters[key] = value; + string[] keyValue = paramPair.Split('=', 2); + if (keyValue.Length == 2) + { + string key = HttpUtility.UrlDecode(keyValue[0]); + string value = HttpUtility.UrlDecode(keyValue[1]); + parameters[key] = value; + } } + + return cleanUrl; } + + return url; } /// From 03938a635b633ad7d83436da6621aef9ea726b10 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 22 Jul 2025 12:52:47 +0530 Subject: [PATCH 31/60] refactor: move mergedParams declaration to ParseUrlParams call for cleaner code --- .../Extensions/SitecoreFieldExtensions.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index bb83011..4e132e1 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -150,11 +150,9 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? return urlStr; } - string url = urlStr; - // Parse existing query parameters and build merged parameters dictionary Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); - url = ParseUrlParams(url, mergedParams); + string url = ParseUrlParams(urlStr, mergedParams); // Add new parameters (these will override existing ones) AddParametersToResult(mergedParams, parameters, skipNullValues: true); From 09ad819bdd1c9c5e33a70571d63e4a890254e7c4 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 24 Jul 2025 14:39:38 +0530 Subject: [PATCH 32/60] Intergration tests for GeneratesProperSrcSetAttribute and SrcSetAttributeContainsCorrectUrls --- .../ComponentModels/ComponentWithImages.cs | 3 + .../TagHelpers/ImageFieldTagHelperFixture.cs | 61 +++++++++++++++++-- .../ComponentWithImages.cshtml | 1 + .../CannedResponses.cs | 4 ++ 4 files changed, 65 insertions(+), 4 deletions(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs index 85d60f3..87246e4 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs @@ -9,4 +9,7 @@ public class ComponentWithImages public ImageField? SecondImage { get; set; } public TextField? Heading { get; set; } + + public ImageField? ThirdImage { get; set; } + } \ No newline at end of file diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs index 95f9877..839358c 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs @@ -95,7 +95,7 @@ public async Task ImgTagHelper_GeneratesImageTags() // Assert // check that there is proper number of 'img' tags generated. - sectionNode.ChildNodes.Count(n => n.Name.Equals("img", StringComparison.OrdinalIgnoreCase)).Should().Be(2); + sectionNode.ChildNodes.Count(n => n.Name.Equals("img", StringComparison.OrdinalIgnoreCase)).Should().Be(3); } [Fact] @@ -140,12 +140,13 @@ public async Task ImgTagHelper_GeneratesProperImageUrlIncludingImageParams() HtmlDocument doc = new(); doc.LoadHtml(response); HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); - HtmlNode? lastImage = sectionNode.ChildNodes.Last(n => n.Name.Equals("img", StringComparison.OrdinalIgnoreCase)); + HtmlNode? secondImage = sectionNode.Descendants("img").ElementAtOrDefault(1); // Assert // check that image url contains mw and mh parameters - lastImage.Attributes.Should().Contain(a => a.Name == "src"); - lastImage.Attributes["src"].Value.Should().Contain("mw=100&mh=50"); + secondImage.Should().NotBeNull(); + secondImage.Attributes.Should().Contain(a => a.Name == "src"); + secondImage.Attributes["src"].Value.Should().Contain("mw=100&mh=50"); } [Fact] @@ -176,6 +177,58 @@ public async Task ImgTagHelper_GeneratesProperEditableImageMarkupWithCustomPrope sectionNode.InnerHtml.Should().Contain("src=\"/sitecore/shell/-/jssmedia/styleguide/data/media/img/sc_logo.png?mw=100&mh=50\""); } + [Fact] + public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() + { + // Arrange + _mockClientHandler.Responses.Push(new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent(Serializer.Serialize(CannedResponses.PageWithPreview)) + }); + + HttpClient client = _server.CreateClient(); + + // Act + string response = await client.GetStringAsync(new Uri("/", UriKind.Relative)); + HtmlDocument doc = new(); + doc.LoadHtml(response); + HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); + HtmlNode? imgNode = sectionNode.Descendants("img").ElementAt(2); + + // Assert + imgNode.Should().NotBeNull(); + imgNode.Attributes.Should().Contain(a => a.Name == "srcset"); + imgNode.Attributes["srcset"].Value.Should().Contain("400w"); + imgNode.Attributes["srcset"].Value.Should().Contain("200w"); + } + + [Fact] + public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrls() + { + // Arrange + _mockClientHandler.Responses.Push(new HttpResponseMessage + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent(Serializer.Serialize(CannedResponses.PageWithPreview)) + }); + + HttpClient client = _server.CreateClient(); + + // Act + string response = await client.GetStringAsync(new Uri("/", UriKind.Relative)); + HtmlDocument doc = new(); + doc.LoadHtml(response); + HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); + HtmlNode? imgNode = sectionNode.Descendants("img").ElementAt(2); + + // Assert + imgNode.Should().NotBeNull(); + var srcSet = imgNode.Attributes["srcset"].Value; + srcSet.Should().Contain("mw=400 400w"); + srcSet.Should().Contain("mw=200 200w"); + } + public void Dispose() { _mockClientHandler.Dispose(); diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml index c6d2dbc..9c3ba8a 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml @@ -4,4 +4,5 @@
+ \ No newline at end of file diff --git a/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs b/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs index 4740cb0..113cc07 100644 --- a/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs +++ b/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs @@ -2988,6 +2988,8 @@ public static class CannedResponses ["FirstImage"] = new ImageField(new Image { Alt = "sample", Src = "sample.png" }), ["SecondImage"] = new ImageField(new Image { Alt = "second", Src = "site/second.png" }), + ["ThirdImage"] = new ImageField(new Image + { Alt = "third", Src = "site/third.png" }), ["Heading"] = new TextField(TestConstants.TestFieldValue), } }, @@ -3894,6 +3896,8 @@ public static class CannedResponses ["FirstImage"] = new ImageField(new Image { Alt = "sample", Src = "sample.png" }), ["SecondImage"] = new ImageField(new Image { Alt = "second", Src = "site/second.png" }), + ["ThirdImage"] = new ImageField(new Image + { Alt = "third", Src = "site/third.png" }), ["Heading"] = new TextField(TestConstants.TestFieldValue), } }, From f6c22c044324fec87470cd5fba79cfe0b1133e45 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 25 Jul 2025 09:58:27 +0530 Subject: [PATCH 33/60] Added FourthImage and enhanced integration tests to assert srcset URLs and descriptors for both third and fourth images --- .../ComponentModels/ComponentWithImages.cs | 2 + .../TagHelpers/ImageFieldTagHelperFixture.cs | 44 +++++++++++++------ .../ComponentWithImages.cshtml | 1 + .../CannedResponses.cs | 4 ++ 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs index 87246e4..174ccab 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs @@ -12,4 +12,6 @@ public class ComponentWithImages public ImageField? ThirdImage { get; set; } + public ImageField? FourthImage { get; set; } + } \ No newline at end of file diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs index 839358c..f2033f3 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs @@ -95,7 +95,7 @@ public async Task ImgTagHelper_GeneratesImageTags() // Assert // check that there is proper number of 'img' tags generated. - sectionNode.ChildNodes.Count(n => n.Name.Equals("img", StringComparison.OrdinalIgnoreCase)).Should().Be(3); + sectionNode.ChildNodes.Count(n => n.Name.Equals("img", StringComparison.OrdinalIgnoreCase)).Should().Be(4); } [Fact] @@ -191,16 +191,26 @@ public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() // Act string response = await client.GetStringAsync(new Uri("/", UriKind.Relative)); - HtmlDocument doc = new(); + HtmlDocument doc = new HtmlDocument(); doc.LoadHtml(response); HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); - HtmlNode? imgNode = sectionNode.Descendants("img").ElementAt(2); // Assert - imgNode.Should().NotBeNull(); - imgNode.Attributes.Should().Contain(a => a.Name == "srcset"); - imgNode.Attributes["srcset"].Value.Should().Contain("400w"); - imgNode.Attributes["srcset"].Value.Should().Contain("200w"); + // Third image (index 2) + HtmlNode? thirdImg = sectionNode.Descendants("img").ElementAt(2); + thirdImg.Should().NotBeNull(); + thirdImg.Attributes.Should().Contain(a => a.Name == "srcset"); + string thirdSrcSet = thirdImg.Attributes["srcset"].Value; + thirdSrcSet.Should().Contain("site/third.png?mw=400 400w"); + thirdSrcSet.Should().Contain("site/third.png?mw=200 200w"); + + // Fourth image (index 3) + HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); + fourthImg.Should().NotBeNull(); + fourthImg.Attributes.Should().Contain(a => a.Name == "srcset"); + string fourthSrcSet = fourthImg.Attributes["srcset"].Value; + fourthSrcSet.Should().Contain("site/fourth.png?mw=800 800w"); + fourthSrcSet.Should().Contain("site/fourth.png?mw=400 400w"); } [Fact] @@ -217,16 +227,24 @@ public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrls() // Act string response = await client.GetStringAsync(new Uri("/", UriKind.Relative)); - HtmlDocument doc = new(); + HtmlDocument doc = new HtmlDocument(); doc.LoadHtml(response); HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); - HtmlNode? imgNode = sectionNode.Descendants("img").ElementAt(2); // Assert - imgNode.Should().NotBeNull(); - var srcSet = imgNode.Attributes["srcset"].Value; - srcSet.Should().Contain("mw=400 400w"); - srcSet.Should().Contain("mw=200 200w"); + // Third image (index 2) + HtmlNode? thirdImg = sectionNode.Descendants("img").ElementAt(2); + thirdImg.Should().NotBeNull(); + string thirdSrcSet = thirdImg.Attributes["srcset"].Value; + thirdSrcSet.Should().Contain("site/third.png?mw=400 400w"); + thirdSrcSet.Should().Contain("site/third.png?mw=200 200w"); + + // Fourth image (index 3) + HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); + fourthImg.Should().NotBeNull(); + string fourthSrcSet = fourthImg.Attributes["srcset"].Value; + fourthSrcSet.Should().Contain("site/fourth.png?mw=800 800w"); + fourthSrcSet.Should().Contain("site/fourth.png?mw=400 400w"); } public void Dispose() diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml index 9c3ba8a..6ef9560 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Views/Shared/Components/SitecoreComponent/ComponentWithImages.cshtml @@ -5,4 +5,5 @@
+ \ No newline at end of file diff --git a/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs b/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs index 113cc07..ef8a712 100644 --- a/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs +++ b/tests/data/Sitecore.AspNetCore.SDK.TestData/CannedResponses.cs @@ -2990,6 +2990,8 @@ public static class CannedResponses { Alt = "second", Src = "site/second.png" }), ["ThirdImage"] = new ImageField(new Image { Alt = "third", Src = "site/third.png" }), + ["FourthImage"] = new ImageField(new Image + { Alt = "fourth", Src = "site/fourth.png" }), ["Heading"] = new TextField(TestConstants.TestFieldValue), } }, @@ -3898,6 +3900,8 @@ public static class CannedResponses { Alt = "second", Src = "site/second.png" }), ["ThirdImage"] = new ImageField(new Image { Alt = "third", Src = "site/third.png" }), + ["FourthImage"] = new ImageField(new Image + { Alt = "fourth", Src = "site/fourth.png" }), ["Heading"] = new TextField(TestConstants.TestFieldValue), } }, From 041c70e37993f4c86b92c4bd3d4c9edab61f7f93 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 25 Jul 2025 10:29:05 +0530 Subject: [PATCH 34/60] update the testcase ImgTagHelper_SrcSetAttributeContainsCorrectUrls to ImgTagHelper_SrcSetAttributeContainsCorrectUrlsAndSizes and clear separation of checking generate srcSet and URL check --- .../TagHelpers/ImageFieldTagHelperFixture.cs | 34 ++++++++----------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs index f2033f3..1bbdaeb 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs @@ -145,8 +145,8 @@ public async Task ImgTagHelper_GeneratesProperImageUrlIncludingImageParams() // Assert // check that image url contains mw and mh parameters secondImage.Should().NotBeNull(); - secondImage.Attributes.Should().Contain(a => a.Name == "src"); - secondImage.Attributes["src"].Value.Should().Contain("mw=100&mh=50"); + secondImage?.Attributes.Should().Contain(a => a.Name == "src"); + secondImage?.Attributes["src"].Value.Should().Contain("mw=100&mh=50"); } [Fact] @@ -196,25 +196,21 @@ public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); // Assert - // Third image (index 2) + // Third image for (index 2) HtmlNode? thirdImg = sectionNode.Descendants("img").ElementAt(2); thirdImg.Should().NotBeNull(); thirdImg.Attributes.Should().Contain(a => a.Name == "srcset"); - string thirdSrcSet = thirdImg.Attributes["srcset"].Value; - thirdSrcSet.Should().Contain("site/third.png?mw=400 400w"); - thirdSrcSet.Should().Contain("site/third.png?mw=200 200w"); + thirdImg.Attributes.Should().Contain(a => a.Name == "sizes"); - // Fourth image (index 3) + // Fourth image for (index 3) HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); fourthImg.Should().NotBeNull(); fourthImg.Attributes.Should().Contain(a => a.Name == "srcset"); - string fourthSrcSet = fourthImg.Attributes["srcset"].Value; - fourthSrcSet.Should().Contain("site/fourth.png?mw=800 800w"); - fourthSrcSet.Should().Contain("site/fourth.png?mw=400 400w"); + fourthImg.Attributes.Should().Contain(a => a.Name == "sizes"); } [Fact] - public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrls() + public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrlsAndSizes() { // Arrange _mockClientHandler.Responses.Push(new HttpResponseMessage @@ -232,19 +228,19 @@ public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrls() HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); // Assert - // Third image (index 2) + // Third image for (index 2) HtmlNode? thirdImg = sectionNode.Descendants("img").ElementAt(2); thirdImg.Should().NotBeNull(); - string thirdSrcSet = thirdImg.Attributes["srcset"].Value; - thirdSrcSet.Should().Contain("site/third.png?mw=400 400w"); - thirdSrcSet.Should().Contain("site/third.png?mw=200 200w"); + thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=400 400w"); + thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=200 200w"); + thirdImg.Attributes["sizes"].Value.Should().Be("(min-width: 400px) 400px, 200px"); - // Fourth image (index 3) + // Fourth image for (index 3) HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); fourthImg.Should().NotBeNull(); - string fourthSrcSet = fourthImg.Attributes["srcset"].Value; - fourthSrcSet.Should().Contain("site/fourth.png?mw=800 800w"); - fourthSrcSet.Should().Contain("site/fourth.png?mw=400 400w"); + fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=800 800w"); + fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=400 400w"); + fourthImg.Attributes["sizes"].Value.Should().Be("(min-width: 800px) 800px, 400px"); } public void Dispose() From 5c4bed23dfd283f8d4aedd4d20ad67715afaa3e5 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 25 Jul 2025 10:58:32 +0530 Subject: [PATCH 35/60] Complete check of attribute contents in GenerateProperSrcSetAttribute --- .../Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs index 1bbdaeb..7b491e8 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs @@ -201,12 +201,18 @@ public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() thirdImg.Should().NotBeNull(); thirdImg.Attributes.Should().Contain(a => a.Name == "srcset"); thirdImg.Attributes.Should().Contain(a => a.Name == "sizes"); + thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=400 400w"); + thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=200 200w"); + thirdImg.Attributes["sizes"].Value.Should().Be("(min-width: 400px) 400px, 200px"); // Fourth image for (index 3) HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); fourthImg.Should().NotBeNull(); fourthImg.Attributes.Should().Contain(a => a.Name == "srcset"); fourthImg.Attributes.Should().Contain(a => a.Name == "sizes"); + fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=800 800w"); + fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=400 400w"); + fourthImg.Attributes["sizes"].Value.Should().Be("(min-width: 800px) 800px, 400px"); } [Fact] From e535930f84b3b36d4f88218f2a30697cbc0e05d1 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 25 Jul 2025 11:01:36 +0530 Subject: [PATCH 36/60] Remove ImgTagHelper_SrcSetAttributeContainsCorrectUrlsAndSizes and merge the tests to ImgTagHelper_GeneratesProperSrcSetAttributeWithCorrectUrlsAndSizes --- .../TagHelpers/ImageFieldTagHelperFixture.cs | 36 +------------------ 1 file changed, 1 insertion(+), 35 deletions(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs index 7b491e8..b0a00a5 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/Fixtures/TagHelpers/ImageFieldTagHelperFixture.cs @@ -178,7 +178,7 @@ public async Task ImgTagHelper_GeneratesProperEditableImageMarkupWithCustomPrope } [Fact] - public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() + public async Task ImgTagHelper_GeneratesProperSrcSetAttributeWithCorrectUrlsAndSizes() { // Arrange _mockClientHandler.Responses.Push(new HttpResponseMessage @@ -215,40 +215,6 @@ public async Task ImgTagHelper_GeneratesProperSrcSetAttribute() fourthImg.Attributes["sizes"].Value.Should().Be("(min-width: 800px) 800px, 400px"); } - [Fact] - public async Task ImgTagHelper_SrcSetAttributeContainsCorrectUrlsAndSizes() - { - // Arrange - _mockClientHandler.Responses.Push(new HttpResponseMessage - { - StatusCode = HttpStatusCode.OK, - Content = new StringContent(Serializer.Serialize(CannedResponses.PageWithPreview)) - }); - - HttpClient client = _server.CreateClient(); - - // Act - string response = await client.GetStringAsync(new Uri("/", UriKind.Relative)); - HtmlDocument doc = new HtmlDocument(); - doc.LoadHtml(response); - HtmlNode? sectionNode = doc.DocumentNode.ChildNodes.First(n => n.HasClass("component-with-images")); - - // Assert - // Third image for (index 2) - HtmlNode? thirdImg = sectionNode.Descendants("img").ElementAt(2); - thirdImg.Should().NotBeNull(); - thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=400 400w"); - thirdImg.Attributes["srcset"].Value.Should().Contain("site/third.png?mw=200 200w"); - thirdImg.Attributes["sizes"].Value.Should().Be("(min-width: 400px) 400px, 200px"); - - // Fourth image for (index 3) - HtmlNode? fourthImg = sectionNode.Descendants("img").ElementAt(3); - fourthImg.Should().NotBeNull(); - fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=800 800w"); - fourthImg.Attributes["srcset"].Value.Should().Contain("site/fourth.png?mw=400 400w"); - fourthImg.Attributes["sizes"].Value.Should().Be("(min-width: 800px) 800px, 400px"); - } - public void Dispose() { _mockClientHandler.Dispose(); From 75602010b61a905d9df60e687084b7ba2d444e6b Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 29 Jul 2025 08:51:54 +0530 Subject: [PATCH 37/60] Refactor AddParametersToResult to use RouteValueDictionary for object parameter extraction - SitecoreFieldExtension --- .../Extensions/SitecoreFieldExtensions.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 4e132e1..7281988 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -97,13 +97,12 @@ private static void AddParametersToResult(Dictionary result, ob } else { - PropertyInfo[] properties = parameters.GetType().GetProperties(); - foreach (PropertyInfo prop in properties) + RouteValueDictionary routeValues = new RouteValueDictionary(parameters); + foreach (KeyValuePair kvp in routeValues) { - object? value = prop.GetValue(parameters); - if (!skipNullValues || value != null) + if (!skipNullValues || kvp.Value != null) { - result[prop.Name] = value; + result[kvp.Key] = kvp.Value; } } } From c317da657c131581ee5618ae41073b30166a4c4e Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 29 Jul 2025 09:22:37 +0530 Subject: [PATCH 38/60] Updated ParseUrlParams to use QueryHelpers.ParseQuery for extracting query parameters from URLs --- .../Extensions/SitecoreFieldExtensions.cs | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 7281988..e9cc2db 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -176,28 +176,27 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? /// The URL without query parameters. private static string ParseUrlParams(string url, Dictionary parameters) { - if (url.Contains('?')) + if (string.IsNullOrEmpty(url)) { - string[] parts = url.Split('?', 2); - string cleanUrl = parts[0]; - string queryString = parts[1]; + return url; + } - string[] paramPairs = queryString.Split('&'); - foreach (string paramPair in paramPairs) - { - string[] keyValue = paramPair.Split('=', 2); - if (keyValue.Length == 2) - { - string key = HttpUtility.UrlDecode(keyValue[0]); - string value = HttpUtility.UrlDecode(keyValue[1]); - parameters[key] = value; - } - } + int queryIndex = url.IndexOf('?'); + if (queryIndex < 0) + { + return url; + } + + string cleanUrl = url.Substring(0, queryIndex); + string queryString = url.Substring(queryIndex); - return cleanUrl; + Dictionary parsedQuery = QueryHelpers.ParseQuery(queryString); + foreach (KeyValuePair kvp in parsedQuery) + { + parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } - return url; + return cleanUrl; } /// From 64bdd84a9c668a2bd5113de652ab7f4d417b9b49 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 29 Jul 2025 09:38:24 +0530 Subject: [PATCH 39/60] Refactor GetWidthDescriptor to use TryGetValue instead of ContainsKey for CA1854 compliance --- .../TagHelpers/Fields/ImageTagHelper.cs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index d8d9453..d0bdd8b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -191,21 +191,21 @@ public override void Process(TagHelperContext context, TagHelperOutput output) if (parameters is Dictionary dictionary) { // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - if (dictionary.ContainsKey("w")) + if (dictionary.TryGetValue("w", out var wValue)) { - width = dictionary["w"]?.ToString(); + width = wValue.ToString(); } - else if (dictionary.ContainsKey("mw")) + else if (dictionary.TryGetValue("mw", out var mwValue)) { - width = dictionary["mw"]?.ToString(); + width = mwValue.ToString(); } - else if (dictionary.ContainsKey("width")) + else if (dictionary.TryGetValue("width", out var widthValue)) { - width = dictionary["width"]?.ToString(); + width = widthValue.ToString(); } - else if (dictionary.ContainsKey("maxWidth")) + else if (dictionary.TryGetValue("maxWidth", out var maxWidthValue)) { - width = dictionary["maxWidth"]?.ToString(); + width = maxWidthValue.ToString(); } } else @@ -219,7 +219,7 @@ public override void Process(TagHelperContext context, TagHelperOutput output) ?? TryParseParameter(parameters, "maxWidth", properties); } - if (width != null && int.TryParse(width, out int widthValue) && widthValue <= 0) + if (width != null && int.TryParse(width, out int widthValueInt) && widthValueInt <= 0) { return null; } From 61610684eaf825edf9e20340a64575596c4243e0 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 29 Jul 2025 11:07:49 +0530 Subject: [PATCH 40/60] Remove JSON string support for SrcSet in ImageTagHelper and update tests - Removed support for passing SrcSet as a JSON string to ImageTagHelper; only object arrays (anonymous objects or dictionaries) are now supported. - Deleted ParseJsonSrcSet and all related JSON parsing logic from ImageTagHelper. - Removed test cases that expected JSON string support for SrcSet - tests for invalid JSON and JSON string srcset generation. --- .../TagHelpers/Fields/ImageTagHelper.cs | 34 +-------------- .../Fields/ImageTagHelperFixture.cs | 42 ------------------- 2 files changed, 2 insertions(+), 74 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index d0bdd8b..cd50d30 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -60,7 +60,7 @@ public class ImageTagHelper(IEditableChromeRenderer chromeRenderer) : TagHelper /// /// Gets or sets the srcset configurations for responsive images. - /// Supports: object[] (anonymous objects), Dictionary arrays, or JSON string. + /// Supports: object[] (anonymous objects) or Dictionary arrays. /// Each item should contain width parameters like { mw = 300 }, { w = 100 }. /// public object? SrcSet { get; set; } @@ -227,25 +227,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return width != null ? $"{width}w" : null; } - private static object[]? ParseJsonSrcSet(object srcSetValue) - { - if (srcSetValue is string jsonString) - { - try - { - // We need to use Dictionary[] to ensure proper deserialization of JSON objects into dictionaries that our GetWidthDescriptor method can handle - return JsonSerializer.Deserialize[]>(jsonString, JsonLayoutServiceSerializer.GetDefaultSerializerOptions()); - } - catch (Exception ex) - { - // JSON parsing failed - this is a programming error, invalid JSON was passed - throw new InvalidOperationException($"Failed to parse srcset JSON: {jsonString}", ex); - } - } - - return [srcSetValue]; - } - private TagBuilder GenerateImage(ImageField imageField, TagHelperOutput output) { Image image = imageField.Value; @@ -375,18 +356,7 @@ private string GenerateSrcSetAttribute(ImageField imageField) return string.Empty; } - object[]? parsedSrcSet; - - if (SrcSet is object[] objectArray) - { - parsedSrcSet = objectArray; - } - else - { - parsedSrcSet = ParseJsonSrcSet(SrcSet); - } - - if (parsedSrcSet == null || parsedSrcSet.Length == 0) + if (SrcSet is not object[] parsedSrcSet || parsedSrcSet.Length == 0) { return string.Empty; } diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index edba006..742d740 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -861,29 +861,6 @@ public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( sizesValue.Should().Be("(min-width: 768px) 600px, 300px"); } - [Theory] - [AutoNSubstituteData] - public void Process_SrcSetWithJsonString_GeneratesSrcSetAttribute( - ImageTagHelper sut, - TagHelperContext tagHelperContext, - TagHelperOutput tagHelperOutput) - { - // Arrange - tagHelperOutput.TagName = "img"; - sut.For = GetModelExpression(new ImageField(_image)); - sut.SrcSet = "[{\"mw\": 500}, {\"mw\": 250}]"; - - // Act - sut.Process(tagHelperContext, tagHelperOutput); - - // Assert - tagHelperOutput.Attributes.Should().ContainSingle(a => a.Name == "srcset"); - string srcSetValue = tagHelperOutput.Attributes["srcset"].Value.ToString()!; - - // Full string comparison - srcSetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=500 500w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=250 250w"); - } - [Theory] [AutoNSubstituteData] public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( @@ -915,25 +892,6 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( srcsetValue.Should().Be("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=800 800w, http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&mw=400 400w"); } - [Theory] - [AutoNSubstituteData] - public void Process_SrcSetWithInvalidJsonString_ThrowsInvalidOperationException( - ImageTagHelper sut, - TagHelperContext tagHelperContext, - TagHelperOutput tagHelperOutput) - { - // Arrange - tagHelperOutput.TagName = "img"; - sut.For = GetModelExpression(new ImageField(_image)); - sut.SrcSet = "invalid json string"; - - // Act & Assert - Action act = () => sut.Process(tagHelperContext, tagHelperOutput); - act.Should().Throw() - .WithMessage("Failed to parse srcset JSON: invalid json string*") - .WithInnerException(); - } - [Theory] [AutoNSubstituteData] public void Process_SrcSetWithImageParamsConflict_SrcSetParametersArePreferred( From ceef4be2257f585c42218bb7c974001792330f98 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Wed, 30 Jul 2025 11:14:52 +0530 Subject: [PATCH 41/60] Remove blank line to fix the warning - ComponentWithImage.cs --- .../ComponentModels/ComponentWithImages.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs index 174ccab..d0fccc2 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Integration.Tests/ComponentModels/ComponentWithImages.cs @@ -13,5 +13,4 @@ public class ComponentWithImages public ImageField? ThirdImage { get; set; } public ImageField? FourthImage { get; set; } - } \ No newline at end of file From 618f8c8e1f42dcb74be72b417ea040d78b66bfa7 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 31 Jul 2025 09:27:10 +0530 Subject: [PATCH 42/60] Refactor GetWidthDescriptor to use HtmlHelper.AnonymousObjectToHtmlAttributes for both anonymous objects and dictionaries --- .../TagHelpers/Fields/ImageTagHelper.cs | 56 ++++++------------- 1 file changed, 17 insertions(+), 39 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index cd50d30..6252a8b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -1,13 +1,10 @@ -using System.Reflection; -using System.Text.Json; -using HtmlAgilityPack; +using HtmlAgilityPack; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Razor.TagHelpers; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Properties; -using Sitecore.AspNetCore.SDK.LayoutService.Client.Serialization; using Sitecore.AspNetCore.SDK.RenderingEngine.Extensions; using Sitecore.AspNetCore.SDK.RenderingEngine.Rendering; @@ -173,12 +170,6 @@ public override void Process(TagHelperContext context, TagHelperOutput output) } } - private static string? TryParseParameter(object parameters, string paramName, PropertyInfo[] properties) - { - PropertyInfo? prop = properties.FirstOrDefault(p => p.Name.Equals(paramName, StringComparison.OrdinalIgnoreCase)); - return prop?.GetValue(parameters)?.ToString(); - } - private static string? GetWidthDescriptor(object? parameters) { if (parameters == null) @@ -186,37 +177,25 @@ public override void Process(TagHelperContext context, TagHelperOutput output) return null; } - string? width = null; + IDictionary dictionary = HtmlHelper.AnonymousObjectToHtmlAttributes(parameters); - if (parameters is Dictionary dictionary) + // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) + string? width = null; + if (dictionary.TryGetValue("w", out var wValue)) { - // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) - if (dictionary.TryGetValue("w", out var wValue)) - { - width = wValue.ToString(); - } - else if (dictionary.TryGetValue("mw", out var mwValue)) - { - width = mwValue.ToString(); - } - else if (dictionary.TryGetValue("width", out var widthValue)) - { - width = widthValue.ToString(); - } - else if (dictionary.TryGetValue("maxWidth", out var maxWidthValue)) - { - width = maxWidthValue.ToString(); - } + width = wValue.ToString(); } - else + else if (dictionary.TryGetValue("mw", out var mwValue)) { - PropertyInfo[] properties = parameters.GetType().GetProperties(); - - // Priority: w > mw > width > maxWidth - width = TryParseParameter(parameters, "w", properties) - ?? TryParseParameter(parameters, "mw", properties) - ?? TryParseParameter(parameters, "width", properties) - ?? TryParseParameter(parameters, "maxWidth", properties); + width = mwValue.ToString(); + } + else if (dictionary.TryGetValue("width", out var widthValue)) + { + width = widthValue.ToString(); + } + else if (dictionary.TryGetValue("maxWidth", out var maxWidthValue)) + { + width = maxWidthValue.ToString(); } if (width != null && int.TryParse(width, out int widthValueInt) && widthValueInt <= 0) @@ -363,8 +342,7 @@ private string GenerateSrcSetAttribute(ImageField imageField) List srcSetEntries = new(); - // filter out null items directly - foreach (object srcSetItem in parsedSrcSet.Where(item => item != null)) + foreach (object srcSetItem in parsedSrcSet) { // Get width descriptor first to check if this entry should be included string? descriptor = GetWidthDescriptor(srcSetItem); From 71413368ae0293d0abd7c95d1a6ba3fb78ed0868 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 31 Jul 2025 10:16:00 +0530 Subject: [PATCH 43/60] Refactor: Update ParseUrlParams to use Uri type and leverage Uri properties where possible --- .../Extensions/SitecoreFieldExtensions.cs | 62 +++++++++++++------ 1 file changed, 42 insertions(+), 20 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index e9cc2db..52a5167 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -1,6 +1,4 @@ -using System.Reflection; -using System.Text.RegularExpressions; -using System.Web; +using System.Text.RegularExpressions; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.WebUtilities; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; @@ -85,9 +83,9 @@ private static void AddParametersToResult(Dictionary result, ob return; } - if (parameters is Dictionary paramDict) + if (parameters is Dictionary paramDict) { - foreach (KeyValuePair kvp in paramDict) + foreach (KeyValuePair kvp in paramDict) { if (!skipNullValues || kvp.Value != null) { @@ -151,7 +149,13 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? // Parse existing query parameters and build merged parameters dictionary Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); - string url = ParseUrlParams(urlStr, mergedParams); + Uri? uri = null; + if (!string.IsNullOrEmpty(urlStr)) + { + Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out uri); + } + + string url = ParseUrlParams(uri, mergedParams); // Add new parameters (these will override existing ones) AddParametersToResult(mergedParams, parameters, skipNullValues: true); @@ -171,32 +175,50 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? /// /// Parses URL query string parameters and adds them to the provided dictionary. /// - /// The full URL with potential query parameters. + /// The Uri with potential query parameters. /// The dictionary to add parsed parameters to. /// The URL without query parameters. - private static string ParseUrlParams(string url, Dictionary parameters) + private static string ParseUrlParams(Uri? uri, Dictionary parameters) { - if (string.IsNullOrEmpty(url)) + if (uri == null) { - return url; + return string.Empty; } - int queryIndex = url.IndexOf('?'); - if (queryIndex < 0) + string urlWithoutQuery; + string? query = null; + + if (uri.IsAbsoluteUri) { - return url; + urlWithoutQuery = uri.GetLeftPart(UriPartial.Path); + query = uri.Query; + } + else + { + // For relative URIs, manually split on '?' + var original = uri.OriginalString; + int idx = original.IndexOf('?'); + if (idx >= 0) + { + urlWithoutQuery = original.Substring(0, idx); + query = original.Substring(idx); + } + else + { + urlWithoutQuery = original; + } } - string cleanUrl = url.Substring(0, queryIndex); - string queryString = url.Substring(queryIndex); - - Dictionary parsedQuery = QueryHelpers.ParseQuery(queryString); - foreach (KeyValuePair kvp in parsedQuery) + if (!string.IsNullOrEmpty(query)) { - parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; + var parsedQuery = QueryHelpers.ParseQuery(query); + foreach (var kvp in parsedQuery) + { + parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; + } } - return cleanUrl; + return urlWithoutQuery; } /// From 1816434f0ffb1067f049cfd679eeed41bb8245f8 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Thu, 31 Jul 2025 10:43:09 +0530 Subject: [PATCH 44/60] Refactor: Add using for Microsoft.Extensions.Primitives and use StringValues directly in ParseUrlParams --- .../Extensions/SitecoreFieldExtensions.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 52a5167..13f77e7 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -1,6 +1,7 @@ using System.Text.RegularExpressions; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.WebUtilities; +using Microsoft.Extensions.Primitives; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; namespace Sitecore.AspNetCore.SDK.RenderingEngine.Extensions; @@ -212,7 +213,7 @@ private static string ParseUrlParams(Uri? uri, Dictionary param if (!string.IsNullOrEmpty(query)) { var parsedQuery = QueryHelpers.ParseQuery(query); - foreach (var kvp in parsedQuery) + foreach (KeyValuePair kvp in parsedQuery) { parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } From b15b4c1a922027c5c2b3cc36a15262706bdeaa78 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Fri, 1 Aug 2025 10:09:32 +0530 Subject: [PATCH 45/60] Simplify ParseUrlParams to handle relative URIs without exceptions --- .../Extensions/SitecoreFieldExtensions.cs | 33 ++++--------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 13f77e7..de295b8 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -186,40 +186,21 @@ private static string ParseUrlParams(Uri? uri, Dictionary param return string.Empty; } - string urlWithoutQuery; - string? query = null; + string original = uri.OriginalString; + int queryIndex = original.IndexOf('?'); - if (uri.IsAbsoluteUri) - { - urlWithoutQuery = uri.GetLeftPart(UriPartial.Path); - query = uri.Query; - } - else - { - // For relative URIs, manually split on '?' - var original = uri.OriginalString; - int idx = original.IndexOf('?'); - if (idx >= 0) - { - urlWithoutQuery = original.Substring(0, idx); - query = original.Substring(idx); - } - else - { - urlWithoutQuery = original; - } - } - - if (!string.IsNullOrEmpty(query)) + if (queryIndex >= 0) { + string query = original.Substring(queryIndex); var parsedQuery = QueryHelpers.ParseQuery(query); - foreach (KeyValuePair kvp in parsedQuery) + foreach (var kvp in parsedQuery) { parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } + return original.Substring(0, queryIndex); } - return urlWithoutQuery; + return original; } /// From b3e5a589cc45ec22f8d2d19bef54bb40e8888af4 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 5 Aug 2025 14:07:17 +0530 Subject: [PATCH 46/60] refactor: ParseUrlParams - improve URI parsing in SitecoreFieldExtensions to handle absolute and relative URIs separately --- .../Extensions/SitecoreFieldExtensions.cs | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index de295b8..a245db4 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -1,7 +1,8 @@ -using System.Text.RegularExpressions; +using System.Collections.Specialized; +using System.Text.RegularExpressions; +using System.Web; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.WebUtilities; -using Microsoft.Extensions.Primitives; using Sitecore.AspNetCore.SDK.LayoutService.Client.Response.Model.Fields; namespace Sitecore.AspNetCore.SDK.RenderingEngine.Extensions; @@ -186,21 +187,40 @@ private static string ParseUrlParams(Uri? uri, Dictionary param return string.Empty; } - string original = uri.OriginalString; - int queryIndex = original.IndexOf('?'); - - if (queryIndex >= 0) + if (uri.IsAbsoluteUri) { - string query = original.Substring(queryIndex); - var parsedQuery = QueryHelpers.ParseQuery(query); - foreach (var kvp in parsedQuery) + string url = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}"; + NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query); + foreach (string? param in queryParams.AllKeys) { - parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; + if (!string.IsNullOrEmpty(param)) + { + parameters[param] = queryParams[param]; + } } - return original.Substring(0, queryIndex); + + return url; } + else + { + // For relative URIs, accessing Uri.Query throws InvalidOperationException, so we use string manipulation + string original = uri.OriginalString; + int queryIndex = original.IndexOf('?'); - return original; + if (queryIndex >= 0) + { + string query = original.Substring(queryIndex); + var parsedQuery = QueryHelpers.ParseQuery(query); + foreach (var kvp in parsedQuery) + { + parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; + } + + return original.Substring(0, queryIndex); + } + + return original; + } } /// From 03285bf0e680d203d90bead42850b0c4e1f356e4 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 10:37:55 +0530 Subject: [PATCH 47/60] Avoid multiple return statements and return null in GetMediaLink --- .../Extensions/SitecoreFieldExtensions.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a245db4..00afe4b 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -23,12 +23,13 @@ public static partial class SitecoreFieldExtensions ArgumentNullException.ThrowIfNull(imageField); string? urlStr = imageField.Value.Src; - if (urlStr == null) + string? result = null; + if (urlStr != null) { - return null; + result = GetSitecoreMediaUri(urlStr, imageParams); } - return GetSitecoreMediaUri(urlStr, imageParams); + return result; } /// From d22722331b5009d91876efe8e29089a621638e90 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 10:47:41 +0530 Subject: [PATCH 48/60] fix IDE0090 violations in the SitecoreFieldExtensions file by replacing the explicit type declarations with target-typed new expressions --- .../Extensions/SitecoreFieldExtensions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 00afe4b..8c9ecca 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -62,7 +62,7 @@ public static partial class SitecoreFieldExtensions /// Merged parameters as dictionary. private static Dictionary MergeParameters(object? imageParams, object? srcSetParams) { - Dictionary result = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary result = new(StringComparer.OrdinalIgnoreCase); // Add base parameters first AddParametersToResult(result, imageParams); @@ -98,7 +98,7 @@ private static void AddParametersToResult(Dictionary result, ob } else { - RouteValueDictionary routeValues = new RouteValueDictionary(parameters); + RouteValueDictionary routeValues = new(parameters); foreach (KeyValuePair kvp in routeValues) { if (!skipNullValues || kvp.Value != null) @@ -151,7 +151,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? } // Parse existing query parameters and build merged parameters dictionary - Dictionary mergedParams = new Dictionary(StringComparer.OrdinalIgnoreCase); + Dictionary mergedParams = new(StringComparer.OrdinalIgnoreCase); Uri? uri = null; if (!string.IsNullOrEmpty(urlStr)) { From d87bdfc5ccefcfd572197ae81f5f2d6b8072b970 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 10:56:32 +0530 Subject: [PATCH 49/60] Refactor AddParametersToResult using a switch expression pattern and LINQ to eliminate the early return statement --- .../Extensions/SitecoreFieldExtensions.cs | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 8c9ecca..6abb492 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -81,31 +81,23 @@ public static partial class SitecoreFieldExtensions /// Whether to skip null values when adding parameters. private static void AddParametersToResult(Dictionary result, object? parameters, bool skipNullValues = false) { - if (parameters == null) + switch (parameters) { - return; - } - - if (parameters is Dictionary paramDict) - { - foreach (KeyValuePair kvp in paramDict) - { - if (!skipNullValues || kvp.Value != null) + case null: + break; + case Dictionary paramDict: + foreach (KeyValuePair kvp in paramDict.Where(kvp => !skipNullValues || kvp.Value != null)) { result[kvp.Key] = kvp.Value; } - } - } - else - { - RouteValueDictionary routeValues = new(parameters); - foreach (KeyValuePair kvp in routeValues) - { - if (!skipNullValues || kvp.Value != null) + break; + default: + RouteValueDictionary routeValues = new(parameters); + foreach (KeyValuePair kvp in routeValues.Where(kvp => !skipNullValues || kvp.Value != null)) { result[kvp.Key] = kvp.Value; } - } + break; } } From 7916141554827488f41ee0d955e3976162225e39 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 10:58:45 +0530 Subject: [PATCH 50/60] set blank line after closing brace of following change --- .../Extensions/SitecoreFieldExtensions.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 6abb492..d7306ec 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -90,6 +90,7 @@ private static void AddParametersToResult(Dictionary result, ob { result[kvp.Key] = kvp.Value; } + break; default: RouteValueDictionary routeValues = new(parameters); @@ -97,6 +98,7 @@ private static void AddParametersToResult(Dictionary result, ob { result[kvp.Key] = kvp.Value; } + break; } } From 47c09bff3a7ec96be4e839537f72b665dcb35e4d Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 11:05:59 +0530 Subject: [PATCH 51/60] remove the redundant else block and consolidate to a single return statement in the ParseUrlParams method --- .../Extensions/SitecoreFieldExtensions.cs | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index d7306ec..5470ace 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -196,26 +196,24 @@ private static string ParseUrlParams(Uri? uri, Dictionary param return url; } - else - { - // For relative URIs, accessing Uri.Query throws InvalidOperationException, so we use string manipulation - string original = uri.OriginalString; - int queryIndex = original.IndexOf('?'); - if (queryIndex >= 0) - { - string query = original.Substring(queryIndex); - var parsedQuery = QueryHelpers.ParseQuery(query); - foreach (var kvp in parsedQuery) - { - parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; - } + // For relative URIs, accessing Uri.Query throws InvalidOperationException, so we use string manipulation + string original = uri.OriginalString; + int queryIndex = original.IndexOf('?'); - return original.Substring(0, queryIndex); + if (queryIndex >= 0) + { + string query = original.Substring(queryIndex); + Dictionary parsedQuery = QueryHelpers.ParseQuery(query); + foreach (KeyValuePair kvp in parsedQuery) + { + parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } - return original; + return original.Substring(0, queryIndex); } + + return original; } /// From 6b64d7a356e728beefb263e45f599186cd5aa294 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 11:11:39 +0530 Subject: [PATCH 52/60] uses the range indexer original[queryIndex..] in parseUrlParams --- .../Extensions/SitecoreFieldExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 5470ace..5aff903 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -203,7 +203,7 @@ private static string ParseUrlParams(Uri? uri, Dictionary param if (queryIndex >= 0) { - string query = original.Substring(queryIndex); + string query = original[queryIndex..]; Dictionary parsedQuery = QueryHelpers.ParseQuery(query); foreach (KeyValuePair kvp in parsedQuery) { From 8b709c5de8b571fa58bd47228401f6ab4ecae8f2 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 12:04:57 +0530 Subject: [PATCH 53/60] Remove var - use explicit types instead of var in changes --- .../TagHelpers/Fields/ImageTagHelper.cs | 8 ++++---- .../TagHelpers/Fields/ImageTagHelperFixture.cs | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 6252a8b..0c15974 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -181,19 +181,19 @@ public override void Process(TagHelperContext context, TagHelperOutput output) // Priority: w > mw > width > maxWidth (matching Content SDK behavior + legacy support) string? width = null; - if (dictionary.TryGetValue("w", out var wValue)) + if (dictionary.TryGetValue("w", out object? wValue)) { width = wValue.ToString(); } - else if (dictionary.TryGetValue("mw", out var mwValue)) + else if (dictionary.TryGetValue("mw", out object? mwValue)) { width = mwValue.ToString(); } - else if (dictionary.TryGetValue("width", out var widthValue)) + else if (dictionary.TryGetValue("width", out object? widthValue)) { width = widthValue.ToString(); } - else if (dictionary.TryGetValue("maxWidth", out var maxWidthValue)) + else if (dictionary.TryGetValue("maxWidth", out object? maxWidthValue)) { width = maxWidthValue.ToString(); } diff --git a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs index 742d740..a26a4bb 100644 --- a/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs +++ b/tests/Sitecore.AspNetCore.SDK.RenderingEngine.Tests/TagHelpers/Fields/ImageTagHelperFixture.cs @@ -757,7 +757,7 @@ public void Process_ScImgTagWithSrcSet_GeneratesSrcSetAttribute( // Assert string content = tagHelperOutput.Content.GetContent(); - var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + System.Text.RegularExpressions.Match srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); srcsetMatch.Success.Should().BeTrue("srcset attribute should be present in the HTML"); string srcsetValue = srcsetMatch.Groups[1].Value; @@ -766,7 +766,7 @@ public void Process_ScImgTagWithSrcSet_GeneratesSrcSetAttribute( srcsetValue.Should().Contain("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=400 400w"); srcsetValue.Should().Contain("http://styleguide/-/jssmedia/styleguide/data/media/img/sc_logo.png?iar=0&hash=F313AD90AE547CAB09277E42509E289B&w=200 200w"); - var entries = srcsetValue.Split(", "); + string[] entries = srcsetValue.Split(", "); entries.Should().HaveCount(2); } @@ -848,11 +848,11 @@ public void Process_EditableImageWithSrcSet_MergesSrcSetIntoEditableMarkup( string content = tagHelperOutput.Content.GetContent(); // Extract srcset and sizes using regex for full string comparison - var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + System.Text.RegularExpressions.Match srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); srcsetMatch.Success.Should().BeTrue(); string srcsetValue = srcsetMatch.Groups[1].Value; - var sizesMatch = System.Text.RegularExpressions.Regex.Match(content, "sizes=\"([^\"]*)\""); + System.Text.RegularExpressions.Match sizesMatch = System.Text.RegularExpressions.Regex.Match(content, "sizes=\"([^\"]*)\""); sizesMatch.Success.Should().BeTrue(); string sizesValue = sizesMatch.Groups[1].Value; @@ -884,7 +884,7 @@ public void Process_SrcSetWithMixedParameterTypes_GeneratesSrcSetAttribute( string content = tagHelperOutput.Content.GetContent(); // Extract srcset using regex for full string comparison - var srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); + System.Text.RegularExpressions.Match srcsetMatch = System.Text.RegularExpressions.Regex.Match(content, "srcset=\"([^\"]*)\""); srcsetMatch.Success.Should().BeTrue(); string srcsetValue = srcsetMatch.Groups[1].Value; From c5e6a177f3b879c2266af9736693df17336f085d Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 12:09:16 +0530 Subject: [PATCH 54/60] use original[..queryIndex]; in ParseUrlParams --- .../Extensions/SitecoreFieldExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 5aff903..a83df20 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -210,7 +210,7 @@ private static string ParseUrlParams(Uri? uri, Dictionary param parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } - return original.Substring(0, queryIndex); + return original[..queryIndex]; } return original; From e0652bc86e5fb499f9f506d575b3f8d0ebfcde7e Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 14:02:06 +0530 Subject: [PATCH 55/60] Remove unnessasary if branch in GenerateSrcSetAttribute - ImageTagHelpers --- .../TagHelpers/Fields/ImageTagHelper.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index 0c15974..af26828 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -330,11 +330,6 @@ private HtmlString MergeEditableMarkupWithCustomAttributes(string editableMarkUp private string GenerateSrcSetAttribute(ImageField imageField) { - if (SrcSet == null) - { - return string.Empty; - } - if (SrcSet is not object[] parsedSrcSet || parsedSrcSet.Length == 0) { return string.Empty; From 50f70511e3fb7d4bf36b594632942b4308b185b3 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Tue, 19 Aug 2025 14:07:42 +0530 Subject: [PATCH 56/60] use collection initializer syntax to create the list in GenerateSrcSetAttribute - ImageTagHelper --- .../TagHelpers/Fields/ImageTagHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs index af26828..ea517ee 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/TagHelpers/Fields/ImageTagHelper.cs @@ -335,7 +335,7 @@ private string GenerateSrcSetAttribute(ImageField imageField) return string.Empty; } - List srcSetEntries = new(); + List srcSetEntries = []; foreach (object srcSetItem in parsedSrcSet) { From 40e565b1bf394d63e09d9d234e63703566bac2f1 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 8 Sep 2025 10:53:29 +0530 Subject: [PATCH 57/60] Remove multiple return statements - GetMediaLinkForSrcSet --- .../Extensions/SitecoreFieldExtensions.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index a83df20..cd7c1ab 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -45,13 +45,14 @@ public static partial class SitecoreFieldExtensions ArgumentNullException.ThrowIfNull(imageField); string? urlStr = imageField.Value.Src; - if (urlStr == null) + string? result = null; + if (urlStr != null) { - return null; + Dictionary mergedParams = MergeParameters(imageParams, srcSetParams); + result = GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); } - Dictionary mergedParams = MergeParameters(imageParams, srcSetParams); - return GetSitecoreMediaUriWithPreservation(urlStr, mergedParams); + return result; } /// From 07f972733387e3a599dd90dabd2999b089a7ff5d Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 8 Sep 2025 13:18:06 +0530 Subject: [PATCH 58/60] Refactor URL parsing to single-return style - Extract ParseAbsoluteUriParams and ParseRelativeUriParams; make ParseUrlParams a single-return wrapper. - Convert ParseRelativeUriParams and GetSitecoreMediaUriWithPreservation to single-return style. - Preserve existing parsing behavior for absolute and relative URIs. --- .../Extensions/SitecoreFieldExtensions.cs | 78 ++++++++++++------- 1 file changed, 49 insertions(+), 29 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index cd7c1ab..5ddf9ea 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -140,30 +140,30 @@ private static string GetSitecoreMediaUri(string url, object? imageParams) /// Modified URL string. private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? parameters) { + string url; + if (string.IsNullOrEmpty(urlStr)) { - return urlStr; + url = string.Empty; } - - // Parse existing query parameters and build merged parameters dictionary - Dictionary mergedParams = new(StringComparer.OrdinalIgnoreCase); - Uri? uri = null; - if (!string.IsNullOrEmpty(urlStr)) + else { - Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out uri); - } + // Parse existing query parameters and build merged parameters dictionary + Dictionary mergedParams = new(StringComparer.OrdinalIgnoreCase); + Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out Uri? uri); - string url = ParseUrlParams(uri, mergedParams); + url = ParseUrlParams(uri, mergedParams); - // Add new parameters (these will override existing ones) - AddParametersToResult(mergedParams, parameters, skipNullValues: true); + // Add new parameters (these will override existing ones) + AddParametersToResult(mergedParams, parameters, skipNullValues: true); - // Add query parameters - foreach (KeyValuePair kvp in mergedParams) - { - if (kvp.Value != null) + // Add query parameters + foreach (KeyValuePair kvp in mergedParams) { - url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty); + if (kvp.Value != null) + { + url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty); + } } } @@ -178,29 +178,45 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? /// The URL without query parameters. private static string ParseUrlParams(Uri? uri, Dictionary parameters) { + string result; + if (uri == null) { - return string.Empty; + result = string.Empty; + } + else if (uri.IsAbsoluteUri) + { + result = ParseAbsoluteUriParams(uri, parameters); + } + else + { + result = ParseRelativeUriParams(uri, parameters); } - if (uri.IsAbsoluteUri) + return result; + } + + private static string ParseAbsoluteUriParams(Uri uri, Dictionary parameters) + { + string url = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}"; + NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query); + foreach (string? param in queryParams.AllKeys) { - string url = $"{uri.Scheme}://{uri.Host}{uri.AbsolutePath}"; - NameValueCollection queryParams = HttpUtility.ParseQueryString(uri.Query); - foreach (string? param in queryParams.AllKeys) + if (!string.IsNullOrEmpty(param)) { - if (!string.IsNullOrEmpty(param)) - { - parameters[param] = queryParams[param]; - } + parameters[param] = queryParams[param]; } - - return url; } + return url; + } + + private static string ParseRelativeUriParams(Uri uri, Dictionary parameters) + { // For relative URIs, accessing Uri.Query throws InvalidOperationException, so we use string manipulation string original = uri.OriginalString; int queryIndex = original.IndexOf('?'); + string result; if (queryIndex >= 0) { @@ -211,10 +227,14 @@ private static string ParseUrlParams(Uri? uri, Dictionary param parameters[kvp.Key] = kvp.Value.Count > 0 ? kvp.Value[0] : null; } - return original[..queryIndex]; + result = original[..queryIndex]; + } + else + { + result = original; } - return original; + return result; } /// From be019875fb46aa9b87fcac7df4ff85be152d6254 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 8 Sep 2025 13:40:49 +0530 Subject: [PATCH 59/60] Keep null semantics and the explicit IsNullOrEmpty guard, and use a single return --- .../Extensions/SitecoreFieldExtensions.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 5ddf9ea..4ecea37 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -138,11 +138,15 @@ private static string GetSitecoreMediaUri(string url, object? imageParams) /// The URL string. /// Parameters to merge. /// Modified URL string. - private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? parameters) + private static string? GetSitecoreMediaUriWithPreservation(string? urlStr, object? parameters) { - string url; + string? url; - if (string.IsNullOrEmpty(urlStr)) + if (urlStr == null) + { + url = null; + } + else if (string.IsNullOrEmpty(urlStr)) { url = string.Empty; } @@ -167,7 +171,7 @@ private static string GetSitecoreMediaUriWithPreservation(string urlStr, object? } } - return ApplyJssMediaUrlPrefix(url); + return url == null ? null : ApplyJssMediaUrlPrefix(url); } /// From 31843709cd33dadeab6398af3957bb6c474214b8 Mon Sep 17 00:00:00 2001 From: krishanthaudayakumara Date: Mon, 8 Sep 2025 13:56:23 +0530 Subject: [PATCH 60/60] Ensure we only drop the path when URI parsing succeedsin GetSitecoreMediaUriWithPreservation --- .../Extensions/SitecoreFieldExtensions.cs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs index 4ecea37..2cc16ff 100644 --- a/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs +++ b/src/Sitecore.AspNetCore.SDK.RenderingEngine/Extensions/SitecoreFieldExtensions.cs @@ -142,36 +142,39 @@ private static string GetSitecoreMediaUri(string url, object? imageParams) { string? url; - if (urlStr == null) + if (string.IsNullOrEmpty(urlStr)) { - url = null; - } - else if (string.IsNullOrEmpty(urlStr)) - { - url = string.Empty; + url = urlStr; } else { // Parse existing query parameters and build merged parameters dictionary Dictionary mergedParams = new(StringComparer.OrdinalIgnoreCase); - Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out Uri? uri); - url = ParseUrlParams(uri, mergedParams); + if (!Uri.TryCreate(urlStr, UriKind.RelativeOrAbsolute, out Uri? uri)) + { + url = urlStr; + } + else + { + url = ParseUrlParams(uri, mergedParams); - // Add new parameters (these will override existing ones) - AddParametersToResult(mergedParams, parameters, skipNullValues: true); + // Add new parameters (these will override existing ones) + AddParametersToResult(mergedParams, parameters, skipNullValues: true); - // Add query parameters - foreach (KeyValuePair kvp in mergedParams) - { - if (kvp.Value != null) + // Add query parameters + foreach (KeyValuePair kvp in mergedParams) { - url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty); + if (kvp.Value != null) + { + url = QueryHelpers.AddQueryString(url, kvp.Key, kvp.Value.ToString() ?? string.Empty); + } } } } - return url == null ? null : ApplyJssMediaUrlPrefix(url); + string? result = url == null ? null : ApplyJssMediaUrlPrefix(url); + return result; } ///