Change Script, Link and Image `TagHelper`s to work better with other `TagHelper`s.

- `ScriptTagHelper`, `LinkTagHelper` and `ImageTagHelper` now default to using `output.Attributes["href|src"]` if it's present when they run. This enables other `TagHelper`s to run prior and add those attributes.
- Added unit tests to validate this behavior.
- Updated `ImageTagHelper` functional test resources. Now that we're always defaulting to `output.Attributes["src"]` for `ImageTagHelper.Src` we're properly copying attributes back into the `output.Attributes` collection in the correct order (isntead of appending to the end).

#2902

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

@@ -75,23 +75,20 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
         /// <inheritdoc />
         public override void Process(TagHelperContext context, TagHelperOutput output)
         {
+            output.CopyHtmlAttribute(SrcAttributeName, context);
+            ProcessUrlAttribute(SrcAttributeName, output);
+
             if (AppendVersion)
             {
                 EnsureFileVersionProvider();
 
-                string resolvedUrl;
+                // Retrieve the TagHelperOutput variation of the "src" attribute in case other TagHelpers in the
-                if (TryResolveUrl(Src, encodeWebRoot: false, resolvedUrl: out resolvedUrl))
+                // pipeline have touched the value. If the value is already encoded this ImageTagHelper may
-                {
+                // not function properly.
-                    Src = resolvedUrl;
+                Src = output.Attributes[SrcAttributeName].Value as string;
-                }
+
                 output.Attributes[SrcAttributeName] = _fileVersionProvider.AddFileVersionToPath(Src);
             }
-            else
-            {
-                // Pass through attribute that is also a well-known HTML attribute.
-                output.CopyHtmlAttribute(SrcAttributeName, context);
-                ProcessUrlAttribute(SrcAttributeName, output);
-            }
         }
 
         private void EnsureFileVersionProvider()

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

@@ -211,16 +211,16 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             if (Href != null)
             {
                 output.CopyHtmlAttribute(HrefAttributeName, context);
-
-                // Resolve any application relative URLs (~/) now so they can be used in comparisons later.
-                if (TryResolveUrl(Href, encodeWebRoot: false, resolvedUrl: out resolvedUrl))
-                {
-                    Href = resolvedUrl;
-                }
-
-                ProcessUrlAttribute(HrefAttributeName, output);
             }
 
+            // If there's no "href" attribute in output.Attributes this will noop.
+            ProcessUrlAttribute(HrefAttributeName, output);
+
+            // Retrieve the TagHelperOutput variation of the "href" attribute in case other TagHelpers in the
+            // pipeline have touched the value. If the value is already encoded this LinkTagHelper may
+            // not function properly.
+            Href = output.Attributes[HrefAttributeName]?.Value as string;
+
             var modeResult = AttributeMatcher.DetermineMode(context, ModeDetails);
 
             modeResult.LogDetails(Logger, this, context.UniqueId, ViewContext.View.Path);
@@ -238,11 +238,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             {
                 EnsureFileVersionProvider();
 
-                var attributeStringValue = output.Attributes[HrefAttributeName]?.Value as string;
+                if (Href != null)
-                if (attributeStringValue != null)
                 {
-                    output.Attributes[HrefAttributeName].Value =
+                    output.Attributes[HrefAttributeName].Value = _fileVersionProvider.AddFileVersionToPath(Href);
-                        _fileVersionProvider.AddFileVersionToPath(attributeStringValue);
                 }
             }
 

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

@@ -179,15 +179,16 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             if (Src != null)
             {
                 output.CopyHtmlAttribute(SrcAttributeName, context);
-
-                if (TryResolveUrl(Src, encodeWebRoot: false, resolvedUrl: out resolvedUrl))
-                {
-                    Src = resolvedUrl;
-                }
-
-                ProcessUrlAttribute(SrcAttributeName, output);
             }
 
+            // If there's no "src" attribute in output.Attributes this will noop.
+            ProcessUrlAttribute(SrcAttributeName, output);
+
+            // Retrieve the TagHelperOutput variation of the "src" attribute in case other TagHelpers in the
+            // pipeline have touched the value. If the value is already encoded this ScriptTagHelper may
+            // not function properly.
+            Src = output.Attributes[SrcAttributeName]?.Value as string;
+
             var modeResult = AttributeMatcher.DetermineMode(context, ModeDetails);
 
             modeResult.LogDetails(Logger, this, context.UniqueId, ViewContext.View.Path);
@@ -205,11 +206,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
             {
                 EnsureFileVersionProvider();
 
-                var attributeStringValue = output.Attributes[SrcAttributeName]?.Value as string;
+                if (Src != null)
-                if (attributeStringValue != null)
                 {
-                    output.Attributes[SrcAttributeName].Value =
+                    output.Attributes[SrcAttributeName].Value = _fileVersionProvider.AddFileVersionToPath(Src);
-                        _fileVersionProvider.AddFileVersionToPath(attributeStringValue);
                 }
             }
 

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

@@ -12,24 +12,24 @@
     <img src="/images/red.png" alt="Red block" title="&lt;the title>">
 
     <!-- Plain image tag with file version -->
-    <img alt="Red versioned" title="Red versioned" src="/images/red.png?v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk" />
+    <img src="/images/red.png?v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk" alt="Red versioned" title="Red versioned" />
 
     <!-- Plain image tag with file version set to false -->
     <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">
+    <img src="http://contoso.com/hello/world" alt="Absolute path versioned">
 
     <!-- Plain image tag with file version and path to file that does not exist -->
-    <img alt="Path to non existing file" src="/images/fake.png" />
+    <img src="/images/fake.png" alt="Path to non existing file" />
 
     <!-- Plain image tag with file version and path containing query string -->
-    <img alt="Path with query string" src="/images/red.png?abc=def&amp;v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk">
+    <img src="/images/red.png?abc=def&amp;v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk" alt="Path with query string">
 
     <!-- Plain image tag with file version and path containing fragment -->
-    <img alt="Path with query string" src="/images/red.png?v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk#abc" />
+    <img src="/images/red.png?v=W2F5D366_nQ2fQqUk3URdgWy2ZekXjHzHJaY5yaiOOk#abc" alt="Path with query string" />
 
     <!-- Plain image tag with file version and path linking to some action -->
-    <img alt="Path linking to some action" src="/controller/action">
+    <img src="/controller/action" alt="Path linking to some action">
 </body>
 </html>

test/Microsoft.AspNet.Mvc.TagHelpers.Test/ImageTagHelperTest.cs

@@ -25,6 +25,56 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
 {
     public class ImageTagHelperTest
     {
+        [Theory]
+        [InlineData(null, "test.jpg", "test.jpg")]
+        [InlineData("abcd.jpg", "test.jpg", "test.jpg")]
+        [InlineData(null, "~/test.jpg", "/virtualRoot/test.jpg")]
+        [InlineData("abcd.jpg", "~/test.jpg", "/virtualRoot/test.jpg")]
+        public void Process_SrcDefaultsToTagHelperOutputSrcAttributeAddedByOtherTagHelper(
+            string src,
+            string srcOutput,
+            string expectedSrcPrefix)
+        {
+            // Arrange
+            var allAttributes = new TagHelperAttributeList(
+                new TagHelperAttributeList
+                {
+                    { "alt", new HtmlString("Testing") },
+                    { "asp-append-version", true },
+                });
+            var context = MakeTagHelperContext(allAttributes);
+            var outputAttributes = new TagHelperAttributeList
+                {
+                    { "alt", new HtmlString("Testing") },
+                    { "src", srcOutput },
+                };
+            var output = new TagHelperOutput("img", outputAttributes);
+            var hostingEnvironment = MakeHostingEnvironment();
+            var viewContext = MakeViewContext();
+            var urlHelper = new Mock<IUrlHelper>();
+            urlHelper
+                .Setup(urlhelper => urlhelper.Content(It.IsAny<string>()))
+                .Returns(new Func<string, string>(url => url.Replace("~/", "/virtualRoot/")));
+            var helper = new ImageTagHelper(
+                hostingEnvironment,
+                MakeCache(),
+                new CommonTestEncoder(),
+                urlHelper.Object)
+            {
+                ViewContext = viewContext,
+                AppendVersion = true,
+                Src = src,
+            };
+
+            // Act
+            helper.Process(context, output);
+
+            // Assert
+            Assert.Equal(
+                expectedSrcPrefix + "?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk",
+                (string)output.Attributes["src"].Value,
+                StringComparer.Ordinal);
+        }
 
         [Fact]
         public void PreservesOrderOfSourceAttributesWhenRun()

test/Microsoft.AspNet.Mvc.TagHelpers.Test/LinkTagHelperTest.cs

@@ -28,6 +28,60 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
 {
     public class LinkTagHelperTest
     {
+        [Theory]
+        [InlineData(null, "test.css", "test.css")]
+        [InlineData("abcd.css", "test.css", "test.css")]
+        [InlineData(null, "~/test.css", "/virtualRoot/test.css")]
+        [InlineData("abcd.css", "~/test.css", "/virtualRoot/test.css")]
+        public void Process_HrefDefaultsToTagHelperOutputHrefAttributeAddedByOtherTagHelper(
+            string href,
+            string hrefOutput,
+            string expectedHrefPrefix)
+        {
+            // Arrange
+            var allAttributes = new TagHelperAttributeList(
+                new TagHelperAttributeList
+                {
+                    { "rel", new HtmlString("stylesheet") },
+                    { "asp-append-version", true },
+                });
+            var context = MakeTagHelperContext(allAttributes);
+            var outputAttributes = new TagHelperAttributeList
+                {
+                    { "rel", new HtmlString("stylesheet") },
+                    { "href", hrefOutput },
+                };
+            var output = MakeTagHelperOutput("link", outputAttributes);
+            var logger = new Mock<ILogger<LinkTagHelper>>();
+            var hostingEnvironment = MakeHostingEnvironment();
+            var viewContext = MakeViewContext();
+            var urlHelper = new Mock<IUrlHelper>();
+            urlHelper
+                .Setup(urlhelper => urlhelper.Content(It.IsAny<string>()))
+                .Returns(new Func<string, string>(url => url.Replace("~/", "/virtualRoot/")));
+            var helper = new LinkTagHelper(
+                logger.Object,
+                hostingEnvironment,
+                MakeCache(),
+                new CommonTestEncoder(),
+                new CommonTestEncoder(),
+                urlHelper.Object)
+            {
+                ViewContext = viewContext,
+                AppendVersion = true,
+                Href = href,
+            };
+
+            // Act
+            helper.Process(context, output);
+
+            // Assert
+            Assert.Equal(
+                expectedHrefPrefix + "?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk",
+                (string)output.Attributes["href"].Value,
+                StringComparer.Ordinal);
+        }
+
         public static TheoryData MultiAttributeSameNameData
         {
             get

test/Microsoft.AspNet.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs

@@ -29,6 +29,60 @@ namespace Microsoft.AspNet.Mvc.TagHelpers
 {
     public class ScriptTagHelperTest
     {
+        [Theory]
+        [InlineData(null, "test.js", "test.js")]
+        [InlineData("abcd.js", "test.js", "test.js")]
+        [InlineData(null, "~/test.js", "/virtualRoot/test.js")]
+        [InlineData("abcd.js", "~/test.js", "/virtualRoot/test.js")]
+        public void Process_SrcDefaultsToTagHelperOutputSrcAttributeAddedByOtherTagHelper(
+            string src,
+            string srcOutput,
+            string expectedSrcPrefix)
+        {
+            // Arrange
+            var allAttributes = new TagHelperAttributeList(
+                new TagHelperAttributeList
+                {
+                    { "type", new HtmlString("text/javascript") },
+                    { "asp-append-version", true },
+                });
+            var context = MakeTagHelperContext(allAttributes);
+            var outputAttributes = new TagHelperAttributeList
+                {
+                    { "type", new HtmlString("text/javascript") },
+                    { "src", srcOutput },
+                };
+            var output = MakeTagHelperOutput("script", outputAttributes);
+            var logger = new Mock<ILogger<ScriptTagHelper>>();
+            var hostingEnvironment = MakeHostingEnvironment();
+            var viewContext = MakeViewContext();
+            var urlHelper = new Mock<IUrlHelper>();
+            urlHelper
+                .Setup(urlhelper => urlhelper.Content(It.IsAny<string>()))
+                .Returns(new Func<string, string>(url => url.Replace("~/", "/virtualRoot/")));
+            var helper = new ScriptTagHelper(
+                logger.Object,
+                hostingEnvironment,
+                MakeCache(),
+                new CommonTestEncoder(),
+                new CommonTestEncoder(),
+                urlHelper.Object)
+            {
+                ViewContext = viewContext,
+                AppendVersion = true,
+                Src = src,
+            };
+
+            // Act
+            helper.Process(context, output);
+
+            // Assert
+            Assert.Equal(
+                expectedSrcPrefix + "?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk",
+                (string)output.Attributes["src"].Value,
+                StringComparer.Ordinal);
+        }
+
         [Theory]
         [MemberData(nameof(LinkTagHelperTest.MultiAttributeSameNameData), MemberType = typeof(LinkTagHelperTest))]
         public async Task HandlesMultipleAttributesSameNameCorrectly(TagHelperAttributeList outputAttributes)