React to https://github.com/aspnet/Razor/pull/683

src/Microsoft.AspNetCore.Mvc.Razor.Host/MvcRazorHost.cs

@@ -112,7 +112,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor
                     TagHelperContentGetContentMethodName = nameof(TagHelperContent.GetContent),
                     TagHelperOutputIsContentModifiedPropertyName = nameof(TagHelperOutput.IsContentModified),
                     TagHelperOutputContentPropertyName = nameof(TagHelperOutput.Content),
-                    TagHelperOutputGetChildContentAsyncMethodName = nameof(TagHelperExecutionContext.GetChildContentAsync)
+                    TagHelperOutputGetChildContentAsyncMethodName = nameof(TagHelperOutput.GetChildContentAsync)
                 })
             {
                 BeginContextMethodName = "BeginContext",

src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs

@@ -245,9 +245,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                 return;
             }
 
-            // NOTE: Values in TagHelperOutput.Attributes may already be HTML-encoded.
-            var attributes = new TagHelperAttributeList(output.Attributes);
-
             if (AppendVersion == true)
             {
                 EnsureFileVersionProvider();
@@ -264,7 +261,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             var builder = new DefaultTagHelperContent();
             if (mode == Mode.GlobbedHref || mode == Mode.Fallback && !string.IsNullOrEmpty(HrefInclude))
             {
-                BuildGlobbedLinkTags(attributes, builder);
+                BuildGlobbedLinkTags(output.Attributes, builder);
                 if (string.IsNullOrEmpty(Href))
                 {
                     // Only HrefInclude is specified. Don't render the original tag.
@@ -306,8 +303,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                     continue;
                 }
 
-                attributes.SetAttribute(HrefAttributeName, url);
+                BuildLinkTag(url, attributes, builder);
-                BuildLinkTag(attributes, builder);
             }
         }
 
@@ -402,38 +398,56 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             }
         }
 
-        private void BuildLinkTag(TagHelperAttributeList attributes, TagHelperContent builder)
+        private void BuildLinkTag(string href, TagHelperAttributeList attributes, TagHelperContent builder)
         {
             builder.AppendHtml("<link ");
 
+            var addHref = true;
+
             // Perf: Avoid allocating enumerator
             for (var i = 0; i < attributes.Count; i++)
             {
                 var attribute = attributes[i];
-                var attributeValue = attribute.Value;
-                if (AppendVersion == true &&
-                    string.Equals(attribute.Name, HrefAttributeName, StringComparison.OrdinalIgnoreCase))
-                {
-                    // "href" values come from bound attributes and globbing. So anything but a non-null string is
-                    // unexpected but could happen if another helper targeting the same element does something odd.
-                    // Pass through existing value in that case.
-                    var attributeStringValue = attributeValue as string;
-                    if (attributeStringValue != null)
-                    {
-                        attributeValue = _fileVersionProvider.AddFileVersionToPath(attributeStringValue);
-                    }
-                }
 
-                builder
+                if (string.Equals(attribute.Name, HrefAttributeName, StringComparison.OrdinalIgnoreCase))
-                    .AppendHtml(attribute.Name)
+                {
-                    .AppendHtml("=\"")
+                    addHref = false;
-                    .Append(HtmlEncoder, attributeValue)
+
-                    .AppendHtml("\" ");
+                    AppendVersionedHref(attribute.Name, href, builder);
+                }
+                else
+                {
+                    AppendAttribute(attribute.Name, attribute.Value, builder);
+                }
+            }
+
+            if (addHref)
+            {
+                AppendVersionedHref(HrefAttributeName, href, builder);
             }
 
             builder.AppendHtml("/>");
         }
 
+        private void AppendVersionedHref(string hrefName, string hrefValue, TagHelperContent builder)
+        {
+            if (AppendVersion == true)
+            {
+                hrefValue = _fileVersionProvider.AddFileVersionToPath(hrefValue);
+            }
+
+            AppendAttribute(hrefName, hrefValue, builder);
+        }
+
+        private void AppendAttribute(string key, object value, TagHelperContent builder)
+        {
+            builder
+                .AppendHtml(key)
+                .AppendHtml("=\"")
+                .Append(HtmlEncoder, value)
+                .AppendHtml("\" ");
+        }
+
         private enum Mode
         {
             /// <summary>

src/Microsoft.AspNetCore.Mvc.TagHelpers/ScriptTagHelper.cs

@@ -208,9 +208,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                 return;
             }
 
-            // NOTE: Values in TagHelperOutput.Attributes may already be HTML-encoded.
-            var attributes = new TagHelperAttributeList(output.Attributes);
-
             if (AppendVersion == true)
             {
                 EnsureFileVersionProvider();
@@ -228,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
 
             if (mode == Mode.GlobbedSrc || mode == Mode.Fallback && !string.IsNullOrEmpty(SrcInclude))
             {
-                BuildGlobbedScriptTags(attributes, builder);
+                BuildGlobbedScriptTags(output.Attributes, builder);
                 if (string.IsNullOrEmpty(Src))
                 {
                     // Only SrcInclude is specified. Don't render the original tag.
@@ -245,7 +242,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                     FallbackSrc = resolvedUrl;
                 }
 
-                BuildFallbackBlock(attributes, builder);
+                BuildFallbackBlock(output.Attributes, builder);
             }
 
             output.PostElement.SetContent(builder);
@@ -270,8 +267,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                     continue;
                 }
 
-                attributes.SetAttribute(SrcAttributeName, url);
+                BuildScriptTag(url, attributes, builder);
-                BuildScriptTag(attributes, builder);
             }
         }
 
@@ -288,13 +284,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                        .AppendHtml(FallbackTestExpression)
                        .AppendHtml("||document.write(\"");
 
-                // May have no "src" attribute in the dictionary e.g. if Src and SrcInclude were not bound.
-                if (!attributes.ContainsName(SrcAttributeName))
-                {
-                    // Need this entry to place each fallback source.
-                    attributes.Add(new TagHelperAttribute(SrcAttributeName, value: null));
-                }
-
                 foreach (var src in fallbackSrcs)
                 {
                     // Fallback "src" values come from bound attributes and globbing. Must always be non-null.
@@ -302,6 +291,8 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
 
                     builder.AppendHtml("<script");
 
+                    var addSrc = true;
+
                     // Perf: Avoid allocating enumerator
                     for (var i = 0; i < attributes.Count; i++)
                     {
@@ -316,20 +307,16 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                         }
                         else
                         {
-                            // Ignore attribute.Value; use src instead.
+                            addSrc = false;
-                            var attributeValue = src;
+                            AppendEncodedVersionedSrc(attribute.Name, src, builder, generateForDocumentWrite: true);
-                            if (AppendVersion == true)
-                            {
-                                attributeValue = _fileVersionProvider.AddFileVersionToPath(attributeValue);
-                            }
-
-                            // attribute.Key ("src") does not need to be JavaScript-encoded.
-                            var encodedValue = JavaScriptEncoder.Encode(attributeValue);
-
-                            AppendAttribute(builder, attribute.Name, encodedValue, escapeQuotes: true);
                         }
                     }
 
+                    if (addSrc)
+                    {
+                        AppendEncodedVersionedSrc(SrcAttributeName, src, builder, generateForDocumentWrite: true);
+                    }
+
                     builder.AppendHtml("><\\/script>");
                 }
 
@@ -337,6 +324,25 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             }
         }
 
+        private void AppendEncodedVersionedSrc(
+            string srcName,
+            string srcValue,
+            TagHelperContent builder,
+            bool generateForDocumentWrite)
+        {
+            if (AppendVersion == true)
+            {
+                srcValue = _fileVersionProvider.AddFileVersionToPath(srcValue);
+            }
+
+            if (generateForDocumentWrite)
+            {
+                srcValue = JavaScriptEncoder.Encode(srcValue);
+            }
+
+            AppendAttribute(builder, srcName, srcValue, escapeQuotes: generateForDocumentWrite);
+        }
+
         private void EnsureGlobbingUrlBuilder()
         {
             if (GlobbingUrlBuilder == null)
@@ -360,30 +366,32 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
         }
 
         private void BuildScriptTag(
+            string src,
             TagHelperAttributeList attributes,
             TagHelperContent builder)
         {
             builder.AppendHtml("<script");
 
+            var addSrc = true;
+
             // Perf: Avoid allocating enumerator
             for (var i = 0; i < attributes.Count; i++)
             {
                 var attribute = attributes[i];
-                var attributeValue = attribute.Value;
+                if (!attribute.Name.Equals(SrcAttributeName, StringComparison.OrdinalIgnoreCase))
-                if (AppendVersion == true &&
-                    string.Equals(attribute.Name, SrcAttributeName, StringComparison.OrdinalIgnoreCase))
                 {
-                    // "src" values come from bound attributes and globbing. So anything but a non-null string is
+                    AppendAttribute(builder, attribute.Name, attribute.Value, escapeQuotes: false);
-                    // unexpected but could happen if another helper targeting the same element does something odd.
-                    // Pass through existing value in that case.
-                    var attributeStringValue = attributeValue as string;
-                    if (attributeStringValue != null)
-                    {
-                        attributeValue = _fileVersionProvider.AddFileVersionToPath(attributeStringValue);
-                    }
                 }
+                else
+                {
+                    addSrc = false;
+                    AppendEncodedVersionedSrc(attribute.Name, src, builder, generateForDocumentWrite: false);
+                }
+            }
 
-                AppendAttribute(builder, attribute.Name, attributeValue, escapeQuotes: false);
+            if (addSrc)
+            {
+                AppendEncodedVersionedSrc(SrcAttributeName, src, builder, generateForDocumentWrite: false);
             }
 
             builder.AppendHtml("></script>");

test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageTest.cs

@@ -978,13 +978,15 @@ namespace Microsoft.AspNetCore.Mvc.Razor
             page.EndAddHtmlAttributeValues(executionContext);
 
             // Assert
-            var htmlAttribute = Assert.Single(executionContext.HtmlAttributes);
+            var output = executionContext.CreateTagHelperOutput();
+            var htmlAttribute = Assert.Single(output.Attributes);
             Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal);
             var htmlContent = Assert.IsAssignableFrom<IHtmlContent>(htmlAttribute.Value);
             Assert.Equal(expectedValue, HtmlContentUtilities.HtmlContentToString(htmlContent), StringComparer.Ordinal);
             Assert.False(htmlAttribute.Minimized);
 
-            var allAttribute = Assert.Single(executionContext.AllAttributes);
+            var context = executionContext.CreateTagHelperContext();
+            var allAttribute = Assert.Single(context.AllAttributes);
             Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal);
             htmlContent = Assert.IsAssignableFrom<IHtmlContent>(allAttribute.Value);
             Assert.Equal(expectedValue, HtmlContentUtilities.HtmlContentToString(htmlContent), StringComparer.Ordinal);
@@ -1016,8 +1018,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor
             page.EndAddHtmlAttributeValues(executionContext);
 
             // Assert
-            Assert.Empty(executionContext.HtmlAttributes);
+            var output = executionContext.CreateTagHelperOutput();
-            var attribute = Assert.Single(executionContext.AllAttributes);
+            Assert.Empty(output.Attributes);
+            var context = executionContext.CreateTagHelperContext();
+            var attribute = Assert.Single(context.AllAttributes);
             Assert.Equal("someattr", attribute.Name, StringComparer.Ordinal);
             Assert.Equal(expectedValue, (string)attribute.Value, StringComparer.Ordinal);
             Assert.False(attribute.Minimized);
@@ -1044,11 +1048,13 @@ namespace Microsoft.AspNetCore.Mvc.Razor
             page.EndAddHtmlAttributeValues(executionContext);
 
             // Assert
-            var htmlAttribute = Assert.Single(executionContext.HtmlAttributes);
+            var output = executionContext.CreateTagHelperOutput();
+            var htmlAttribute = Assert.Single(output.Attributes);
             Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal);
             Assert.Equal("someattr", (string)htmlAttribute.Value, StringComparer.Ordinal);
             Assert.False(htmlAttribute.Minimized);
-            var allAttribute = Assert.Single(executionContext.AllAttributes);
+            var context = executionContext.CreateTagHelperContext();
+            var allAttribute = Assert.Single(context.AllAttributes);
             Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal);
             Assert.Equal("someattr", (string)allAttribute.Value, StringComparer.Ordinal);
             Assert.False(allAttribute.Minimized);

test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/RenderAtEndOfFormTagHelperTest.cs

@@ -63,7 +63,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                 });
 
             var tagHelperContext = new TagHelperContext(
-                Enumerable.Empty<TagHelperAttribute>(),
+                new TagHelperAttributeList(),
                 new Dictionary<object, object>(),
                 "someId");