From f1eefdb650c5f7bd07ccc4499867ccaaf6dc6382 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Mon, 17 Aug 2015 16:56:58 -0700 Subject: [PATCH] Enable `CopyHtmlAttribute` to maintain copied attribute order. - Updated implementation to do a best guess at copying an attribute back into `TagHelperOutput.Attributes`. - Updated existing functional and unit tests to account for maintained attribute order. - Added a set of complex order based unit tests to validate `CopyHtmlAttribute` properly maintains order. #2639 --- .../TagHelperOutputExtensions.cs | 95 ++++- ...WebSite.HtmlGeneration_Customer.Index.html | 4 +- ...e.HtmlGeneration_Home.CreateWarehouse.html | 2 +- ...nWebSite.HtmlGeneration_Home.Customer.html | 4 +- ...Site.HtmlGeneration_Home.EmployeeList.html | 6 +- ...tionWebSite.HtmlGeneration_Home.Image.html | 2 +- ...Site.HtmlGeneration_Home.Link.Encoded.html | 42 +-- ...ationWebSite.HtmlGeneration_Home.Link.html | 42 +-- ...ite.HtmlGeneration_Home.Order.Encoded.html | 8 +- ...tionWebSite.HtmlGeneration_Home.Order.html | 8 +- ...e.HtmlGeneration_Home.Product.Encoded.html | 4 +- ...onWebSite.HtmlGeneration_Home.Product.html | 4 +- ...bSite.HtmlGeneration_Home.ProductList.html | 8 +- ...te.HtmlGeneration_Home.Script.Encoded.html | 8 +- ...ionWebSite.HtmlGeneration_Home.Script.html | 8 +- .../InputTagHelperTest.cs | 2 +- .../LinkTagHelperTest.cs | 4 +- .../ScriptTagHelperTest.cs | 4 +- .../TagHelperOutputExtensionsTest.cs | 334 +++++++++++++++++- 19 files changed, 497 insertions(+), 92 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs index be7410a15c..cb738ba070 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs @@ -23,9 +23,17 @@ namespace Microsoft.AspNet.Mvc.TagHelpers /// The this method extends. /// The name of the bound attribute. /// The . - /// Only copies the attribute if 's + /// + /// + /// Only copies the attribute if 's /// does not contain an attribute with the given - /// . + /// . + /// + /// + /// Duplicate attributes same name in 's + /// or 's may result in copied + /// attribute order not being maintained. + /// public static void CopyHtmlAttribute( [NotNull] this TagHelperOutput tagHelperOutput, [NotNull] string attributeName, @@ -33,21 +41,30 @@ namespace Microsoft.AspNet.Mvc.TagHelpers { if (!tagHelperOutput.Attributes.ContainsName(attributeName)) { - IEnumerable entries; + var copiedAttribute = false; - // We look for the original attribute so we can restore the exact attribute name the user typed. - // Approach also ignores changes made to tagHelperOutput[attributeName]. - if (!context.AllAttributes.TryGetAttributes(attributeName, out entries)) + // We iterate context.AllAttributes backwards since we prioritize TagHelperOutput values occurring + // before the current context.AllAttribtes[i]. + for (var i = context.AllAttributes.Count - 1; i >= 0; i--) + { + // We look for the original attribute so we can restore the exact attribute name the user typed in + // approximately the same position where the user wrote it in the Razor source. + if (string.Equals( + attributeName, + context.AllAttributes[i].Name, + StringComparison.OrdinalIgnoreCase)) + { + CopyHtmlAttribute(i, tagHelperOutput, context); + copiedAttribute = true; + } + } + + if (!copiedAttribute) { throw new ArgumentException( Resources.FormatTagHelperOutput_AttributeDoesNotExist(attributeName, nameof(TagHelperContext)), nameof(attributeName)); } - - foreach (var entry in entries) - { - tagHelperOutput.Attributes.Add(entry.Name, entry.Value); - } } } @@ -100,5 +117,61 @@ namespace Microsoft.AspNet.Mvc.TagHelpers tagHelperOutput.Attributes.Remove(attribute); } } + + private static void CopyHtmlAttribute( + int allAttributeIndex, + TagHelperOutput tagHelperOutput, + TagHelperContext context) + { + var existingAttribute = context.AllAttributes[allAttributeIndex]; + var copiedAttribute = new TagHelperAttribute + { + Name = existingAttribute.Name, + Value = existingAttribute.Value, + Minimized = existingAttribute.Minimized + }; + + // Move backwards through context.AllAttributes from the provided index until we find a familiar attribute + // in tagHelperOutput where we can insert the copied value after the familiar one. + for (var i = allAttributeIndex - 1; i >= 0; i--) + { + var previousName = context.AllAttributes[i].Name; + var index = IndexOfFirstMatch(previousName, tagHelperOutput.Attributes); + if (index != -1) + { + tagHelperOutput.Attributes.Insert(index + 1, copiedAttribute); + return; + } + } + + // Move forward through context.AllAttributes from the provided index until we find a familiar attribute in + // tagHelperOutput where we can insert the copied value. + for (var i = allAttributeIndex + 1; i < context.AllAttributes.Count; i++) + { + var nextName = context.AllAttributes[i].Name; + var index = IndexOfFirstMatch(nextName, tagHelperOutput.Attributes); + if (index != -1) + { + tagHelperOutput.Attributes.Insert(index, copiedAttribute); + return; + } + } + + // Couldn't determine the attribute's location, add it to the end. + tagHelperOutput.Attributes.Add(copiedAttribute); + } + + private static int IndexOfFirstMatch(string name, TagHelperAttributeList attributes) + { + for (var i = 0; i < attributes.Count; i++) + { + if (string.Equals(name, attributes[i].Name, StringComparison.OrdinalIgnoreCase)) + { + return i; + } + } + + return -1; + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html index 7d6cf27c87..9026b3bc0d 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html @@ -3,7 +3,7 @@
- + The value '' is invalid.
@@ -21,7 +21,7 @@
- + The Password field is required.
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.CreateWarehouse.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.CreateWarehouse.html index f5475cc0ab..9df160372c 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.CreateWarehouse.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.CreateWarehouse.html @@ -2,7 +2,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Customer.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Customer.html index b6b9557fca..a68479262d 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Customer.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Customer.html @@ -3,7 +3,7 @@
- +
@@ -21,7 +21,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.EmployeeList.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.EmployeeList.html index 531b893fc2..4239bd5a35 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.EmployeeList.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.EmployeeList.html @@ -4,7 +4,7 @@
- +
@@ -37,7 +37,7 @@ EmployeeName_0
- +
@@ -70,7 +70,7 @@ EmployeeName_1
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Image.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Image.html index c851ccf084..a8557e52a3 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Image.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Image.html @@ -15,7 +15,7 @@ Red versioned - Red explicitly not versioned + Red explicitly not versioned Absolute path versioned diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.Encoded.html index 401a11de37..642e4848b0 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.Encoded.html @@ -20,7 +20,7 @@ - + @@ -32,13 +32,13 @@ - + - + - + @@ -50,11 +50,11 @@ - + - + @@ -62,19 +62,19 @@ - + - + - + - + @@ -86,44 +86,44 @@ - + - + - + - + - + - + - + - + - + - + - + diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html index 15b5a344fe..a5c4e08137 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html @@ -20,7 +20,7 @@ - + @@ -32,13 +32,13 @@ - + - + - + @@ -50,11 +50,11 @@ - + - + @@ -62,19 +62,19 @@ - + - + - + - + @@ -86,44 +86,44 @@ - + - + - + - + - + - + - + - + - + - + - + diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html index 6bcfeecdc9..15249dd8d2 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.Encoded.html @@ -1,4 +1,4 @@ - + @@ -7,7 +7,7 @@
- +
@@ -45,7 +45,7 @@
- +
@@ -63,7 +63,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html index 69df8c30d2..6bd3544760 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Order.html @@ -1,4 +1,4 @@ - + @@ -7,7 +7,7 @@
- +
@@ -45,7 +45,7 @@
- +
@@ -63,7 +63,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html index 6019532ad0..3774684a26 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.Encoded.html @@ -1,4 +1,4 @@ - + @@ -7,7 +7,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.html index b6176ea4ae..a7e6d03314 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Product.html @@ -1,4 +1,4 @@ - + @@ -7,7 +7,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.ProductList.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.ProductList.html index 61526e7cdc..beb60a138c 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.ProductList.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.ProductList.html @@ -1,11 +1,11 @@ - +
- +
@@ -22,7 +22,7 @@
- +
@@ -39,7 +39,7 @@
- +
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html index 686f5cd516..40810ebd8f 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html @@ -10,15 +10,15 @@ // Regular script with comment in body, and extra properties. - - + - - + - - + - - +