From 9e0fb54ea51fd43fd01e03e3ab71f62e851560c2 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 23 Jul 2024 10:25:54 +0200 Subject: [PATCH] Removed Type attribute from parsed local links (#16780) * Removed type attribute from processed local links improved code readabilty by using multi return type to private class * Removed type attribute from processed locallinks in delivery api * Removed type attribute from expected output regarding locallink parsing * Cleanup * Fixed spacing bug * Added 2 more edge test cases --------- Co-authored-by: Elitsa --- .../Templates/HtmlLocalLinkParser.cs | 79 +++++++++++++------ .../DeliveryApi/ApiRichTextMarkupParser.cs | 1 + .../Templates/HtmlLocalLinkParserTests.cs | 12 ++- .../ApiRichTextMarkupParserTests.cs | 4 +- 4 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs index 74105da511..c79506fb5f 100644 --- a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs +++ b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs @@ -35,11 +35,11 @@ public sealed class HtmlLocalLinkParser public IEnumerable FindUdisFromLocalLinks(string text) { - foreach ((var intId, GuidUdi? udi, var tagValue) in FindLocalLinkIds(text)) + foreach (LocalLinkTag tagData in FindLocalLinkIds(text)) { - if (udi is not null) + if (tagData.Udi is not null) { - yield return udi; // In v8, we only care abuot UDIs + yield return tagData.Udi; // In v8, we only care about UDIs } } } @@ -80,38 +80,41 @@ public sealed class HtmlLocalLinkParser throw new InvalidOperationException("Could not parse internal links, there is no current UmbracoContext"); } - foreach ((var intId, GuidUdi? udi, var tagValue) in FindLocalLinkIds(text)) + foreach (LocalLinkTag tagData in FindLocalLinkIds(text)) { - if (udi is not null) + if (tagData.Udi is not null) { var newLink = "#"; - if (udi?.EntityType == Constants.UdiEntityType.Document) + if (tagData.Udi?.EntityType == Constants.UdiEntityType.Document) { - newLink = _publishedUrlProvider.GetUrl(udi.Guid); + newLink = _publishedUrlProvider.GetUrl(tagData.Udi.Guid); } - else if (udi?.EntityType == Constants.UdiEntityType.Media) + else if (tagData.Udi?.EntityType == Constants.UdiEntityType.Media) { - newLink = _publishedUrlProvider.GetMediaUrl(udi.Guid); + newLink = _publishedUrlProvider.GetMediaUrl(tagData.Udi.Guid); } - if (newLink == null) - { - newLink = "#"; - } - text = text.Replace(tagValue, "href=\"" + newLink); + text = StripTypeAttributeFromTag(text, tagData.Udi!.EntityType); + text = text.Replace(tagData.TagHref, "href=\"" + newLink); } - else if (intId.HasValue) + else if (tagData.IntId.HasValue) { - var newLink = _publishedUrlProvider.GetUrl(intId.Value); - text = text.Replace(tagValue, "href=\"" + newLink); + var newLink = _publishedUrlProvider.GetUrl(tagData.IntId.Value); + text = text.Replace(tagData.TagHref, "href=\"" + newLink); } } return text; } - private IEnumerable<(int? intId, GuidUdi? udi, string tagValue)> FindLocalLinkIds(string text) + // under normal circumstances, the type attribute is preceded by a space + // to cover the rare occasion where it isn't, we first replace with a a space and then without. + private string StripTypeAttributeFromTag(string tag, string type) => + tag.Replace($" type=\"{type}\"", string.Empty) + .Replace($"type=\"{type}\"", string.Empty); + + private IEnumerable FindLocalLinkIds(string text) { MatchCollection localLinkTagMatches = LocalLinkTagPattern.Matches(text); foreach (Match linkTag in localLinkTagMatches) @@ -126,18 +129,22 @@ public sealed class HtmlLocalLinkParser continue; } - yield return (null, new GuidUdi(linkTag.Groups["type"].Value, guid), linkTag.Groups["locallink"].Value); + yield return new LocalLinkTag( + null, + new GuidUdi(linkTag.Groups["type"].Value, guid), + linkTag.Groups["locallink"].Value, + linkTag.Value); } // also return legacy results for values that have not been migrated - foreach ((int? intId, GuidUdi? udi, string tagValue) legacyResult in FindLegacyLocalLinkIds(text)) + foreach (LocalLinkTag legacyResult in FindLegacyLocalLinkIds(text)) { yield return legacyResult; } } // todo remove at some point? - private IEnumerable<(int? intId, GuidUdi? udi, string tagValue)> FindLegacyLocalLinkIds(string text) + private IEnumerable FindLegacyLocalLinkIds(string text) { // Parse internal links MatchCollection tags = LocalLinkPattern.Matches(text); @@ -153,15 +160,41 @@ public sealed class HtmlLocalLinkParser var guidUdi = udi as GuidUdi; if (guidUdi is not null) { - yield return (null, guidUdi, tag.Value); + yield return new LocalLinkTag(null, guidUdi, tag.Value, null); } } if (int.TryParse(id, NumberStyles.Integer, CultureInfo.InvariantCulture, out var intId)) { - yield return (intId, null, tag.Value); + yield return new LocalLinkTag (intId, null, tag.Value, null); } } } } + + private class LocalLinkTag + { + public LocalLinkTag(int? intId, GuidUdi? udi, string tagHref) + { + IntId = intId; + Udi = udi; + TagHref = tagHref; + } + + public LocalLinkTag(int? intId, GuidUdi? udi, string tagHref, string? fullTag) + { + IntId = intId; + Udi = udi; + TagHref = tagHref; + FullTag = fullTag; + } + + public int? IntId { get; } + + public GuidUdi? Udi { get; } + + public string TagHref { get; } + + public string? FullTag { get; } + } } diff --git a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs index 338966dc39..434a6ef16a 100644 --- a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs +++ b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs @@ -60,6 +60,7 @@ internal sealed class ApiRichTextMarkupParser : ApiRichTextParserBase, IApiRichT link.SetAttributeValue("href", route.Path); link.SetAttributeValue("data-start-item-path", route.StartItem.Path); link.SetAttributeValue("data-start-item-id", route.StartItem.Id.ToString("D")); + link.Attributes["type"]?.Remove(); }, url => link.SetAttributeValue("href", url), () => link.Attributes.Remove("href")); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs index 93b0d5bba3..6ef0f74e2f 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Umbraco. // See LICENSE for more details. -using System.Linq; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; @@ -13,7 +12,6 @@ using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Templates; using Umbraco.Cms.Tests.Common; using Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects; -using Umbraco.Extensions; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Templates; @@ -111,10 +109,16 @@ public class HtmlLocalLinkParserTests // current [TestCase( "world", - "world")] + "world")] [TestCase( "world", - "world")] + "world")] + [TestCase( + "world", + "world")] + [TestCase( + "world", + "world")] // legacy [TestCase( "hello href=\"{localLink:1234}\" world ", diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs index 43ec2136f7..d9a7309577 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs @@ -72,8 +72,8 @@ public class ApiRichTextMarkupParserTests

and to the other page

"; var expectedOutput = - @"

Rich text outside of the blocks with a link to itself

-

and to the other page

"; + @"

Rich text outside of the blocks with a link to itself

+

and to the other page

"; var parsedHtml = parser.Parse(html);