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

Author: N. Taylor Mullen

src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperOutputExtensions.cs

@@ -23,30 +23,47 @@ public static class TagHelperOutputExtensions
         /// <param name="tagHelperOutput">The <see cref="TagHelperOutput"/> this method extends.</param>
         /// <param name="attributeName">The name of the bound attribute.</param>
         /// <param name="context">The <see cref="TagHelperContext"/>.</param>
-        /// <remarks>Only copies the attribute if <paramref name="tagHelperOutput"/>'s
+        /// <remarks>
+        /// <para>
+        /// Only copies the attribute if <paramref name="tagHelperOutput"/>'s
         /// <see cref="TagHelperOutput.Attributes"/> does not contain an attribute with the given
-        /// <paramref name="attributeName"/>.</remarks>
+        /// <paramref name="attributeName"/>.
+        /// </para>
+        /// <para>
+        /// Duplicate attributes same name in <paramref name="context"/>'s <see cref="TagHelperContext.AllAttributes"/>
+        /// or <paramref name="tagHelperOutput"/>'s <see cref="TagHelperOutput.Attributes"/> may result in copied
+        /// attribute order not being maintained.
+        /// </para></remarks>
         public static void CopyHtmlAttribute(
             [NotNull] this TagHelperOutput tagHelperOutput,
             [NotNull] string attributeName,
             [NotNull] TagHelperContext context)
         {
             if (!tagHelperOutput.Attributes.ContainsName(attributeName))
             {
-                IEnumerable<IReadOnlyTagHelperAttribute> 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--)
                 {
-                    throw new ArgumentException(
-                        Resources.FormatTagHelperOutput_AttributeDoesNotExist(attributeName, nameof(TagHelperContext)),
-                        nameof(attributeName));
+                    // 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;
+                    }
                 }
 
-                foreach (var entry in entries)
+                if (!copiedAttribute)
                 {
-                    tagHelperOutput.Attributes.Add(entry.Name, entry.Value);
+                    throw new ArgumentException(
+                        Resources.FormatTagHelperOutput_AttributeDoesNotExist(attributeName, nameof(TagHelperContext)),
+                        nameof(attributeName));
                 }
             }
         }
@@ -100,5 +117,61 @@ public static void RemoveRange(
                 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;
+        }
     }
 }

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html

@@ -3,7 +3,7 @@
     <form action="/Customer/HtmlGeneration_Customer" method="post">
         <div>
             <label class="order" for="Number">Number</label>
-            <input class="form-control input-validation-error" type="number" data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." id="Number" name="Number" value="" />
+            <input type="number" class="form-control input-validation-error" data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." id="Number" name="Number" value="" />
             <span class="field-validation-error" data-valmsg-for="Number" data-valmsg-replace="true">The value &#x27;&#x27; is invalid.</span>
         </div>
         <div>
@@ -21,7 +21,7 @@
         </div>
         <div>
             <label class="order" for="Password">Password</label>
-            <input class="form-control input-validation-error" type="password" data-val="true" data-val-required="The Password field is required." id="Password" name="Password" />
+            <input type="password" class="form-control input-validation-error" data-val="true" data-val-required="The Password field is required." id="Password" name="Password" />
             <span class="field-validation-error" data-valmsg-for="Password" data-valmsg-replace="true">The Password field is required.</span>
         </div>
         <div>

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.CreateWarehouse.html

@@ -2,7 +2,7 @@
 <body>
 <form action="/HtmlGeneration_Home/CreateWarehouse" method="post">        <div>
             <label class="warehouse" for="City">City</label>
-            <input size="50" type="text" data-val="true" data-val-minlength="The field City must be a string or array type with a minimum length of &#x27;2&#x27;." data-val-minlength-min="2" id="City" name="City" value="" />
+            <input type="text" size="50" data-val="true" data-val-minlength="The field City must be a string or array type with a minimum length of &#x27;2&#x27;." data-val-minlength-min="2" id="City" name="City" value="" />
             <span class="field-validation-valid" data-valmsg-for="City" data-valmsg-replace="true"></span>
         </div>
         <div>

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Customer.html

@@ -3,7 +3,7 @@
     <form action="/Customer/HtmlGeneration_Customer" method="post">
         <div>
             <label class="order" for="Number">Number</label>
-            <input class="form-control" type="number" data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." id="Number" name="Number" value="" />
+            <input type="number" class="form-control" data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." id="Number" name="Number" value="" />
             <span class="field-validation-valid" data-valmsg-for="Number" data-valmsg-replace="true"></span>
         </div>
         <div>
@@ -21,7 +21,7 @@
         </div>
         <div>
             <label class="order" for="Password">Password</label>
-            <input class="form-control" type="password" data-val="true" data-val-required="The Password field is required." id="Password" name="Password" />
+            <input type="password" class="form-control" data-val="true" data-val-required="The Password field is required." id="Password" name="Password" />
             <span class="field-validation-valid" data-valmsg-for="Password" data-valmsg-replace="true"></span>
         </div>
         <div>

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.EmployeeList.html

@@ -4,7 +4,7 @@
 <div>
     <label for="z0__Number">Number</label>
     <input data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." disabled="disabled" id="z0__Number" name="[0].Number" readonly="readonly" type="text" value="0" />
-    <input class="form-control" type="number" id="z0__Number" name="[0].Number" value="0" />
+    <input type="number" class="form-control" id="z0__Number" name="[0].Number" value="0" />
 </div>
 <div>
     <label class="employee" for="z0__Name">Name</label>
@@ -37,7 +37,7 @@
 <div>
     <label for="z1__Number">Number</label>
     <input data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." disabled="disabled" id="z1__Number" name="[1].Number" readonly="readonly" type="text" value="1" />
-    <input class="form-control" type="number" id="z1__Number" name="[1].Number" value="1" />
+    <input type="number" class="form-control" id="z1__Number" name="[1].Number" value="1" />
 </div>
 <div>
     <label class="employee" for="z1__Name">Name</label>
@@ -70,7 +70,7 @@
 <div>
     <label for="z2__Number">Number</label>
     <input data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" data-val-required="The Number field is required." disabled="disabled" id="z2__Number" name="[2].Number" readonly="readonly" type="text" value="2" />
-    <input class="form-control" type="number" id="z2__Number" name="[2].Number" value="2" />
+    <input type="number" class="form-control" id="z2__Number" name="[2].Number" value="2" />
 </div>
 <div>
     <label class="employee" for="z2__Name">Name</label>

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Image.html

@@ -15,7 +15,7 @@ <h2>Image Tag Helper Test</h2>
     <img alt="Red versioned" title="Red versioned" src="/images/red.png?v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk" />
 
     <!-- Plain image tag with file version set to false -->
-    <img alt="Red explicitly not versioned" title="Red versioned" src="/images/red.png">
+    <img src="/images/red.png" alt="Red explicitly not versioned" title="Red versioned">
 
     <!-- Plain image tag with absolute path and file version -->
     <img alt="Absolute path versioned" src="http://contoso.com/hello/world">