From c3e2e6fa0a8888374e3d05127185cea314622ffb Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Wed, 19 Aug 2015 17:06:15 -0700 Subject: [PATCH] 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 --- .../ImageTagHelper.cs | 19 +++---- .../LinkTagHelper.cs | 22 ++++---- .../ScriptTagHelper.cs | 21 ++++---- ...tionWebSite.HtmlGeneration_Home.Image.html | 12 ++--- .../ImageTagHelperTest.cs | 50 +++++++++++++++++ .../LinkTagHelperTest.cs | 54 +++++++++++++++++++ .../ScriptTagHelperTest.cs | 54 +++++++++++++++++++ 7 files changed, 192 insertions(+), 40 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/ImageTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/ImageTagHelper.cs index 47d7e365a2..3133a360ce 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/ImageTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/ImageTagHelper.cs @@ -75,23 +75,20 @@ namespace Microsoft.AspNet.Mvc.TagHelpers /// public override void Process(TagHelperContext context, TagHelperOutput output) { + output.CopyHtmlAttribute(SrcAttributeName, context); + ProcessUrlAttribute(SrcAttributeName, output); + if (AppendVersion) { EnsureFileVersionProvider(); - string resolvedUrl; - if (TryResolveUrl(Src, encodeWebRoot: false, resolvedUrl: out resolvedUrl)) - { - Src = resolvedUrl; - } + // 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 ImageTagHelper may + // not function properly. + 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() diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs index cf9069b3b2..ecd0365d2e 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs +++ b/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 (attributeStringValue != null) + if (Href != null) { - output.Attributes[HrefAttributeName].Value = - _fileVersionProvider.AddFileVersionToPath(attributeStringValue); + output.Attributes[HrefAttributeName].Value = _fileVersionProvider.AddFileVersionToPath(Href); } } diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs index 94ceb57985..76cda41b99 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs +++ b/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 (attributeStringValue != null) + if (Src != null) { - output.Attributes[SrcAttributeName].Value = - _fileVersionProvider.AddFileVersionToPath(attributeStringValue); + output.Attributes[SrcAttributeName].Value = _fileVersionProvider.AddFileVersionToPath(Src); } } 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 a8557e52a3..95b1559a10 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 @@ -12,24 +12,24 @@ Red block - Red versioned + Red versioned Red explicitly not versioned - Absolute path versioned + Absolute path versioned - Path to non existing file + Path to non existing file - Path with query string + Path with query string - Path with query string + Path with query string - Path linking to some action + Path linking to some action \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ImageTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ImageTagHelperTest.cs index 52f3b9a4cf..8da4eadcb2 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ImageTagHelperTest.cs +++ b/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(); + urlHelper + .Setup(urlhelper => urlhelper.Content(It.IsAny())) + .Returns(new Func(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() diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/LinkTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/LinkTagHelperTest.cs index f4cdbb419f..af3823a7f9 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/LinkTagHelperTest.cs +++ b/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>(); + var hostingEnvironment = MakeHostingEnvironment(); + var viewContext = MakeViewContext(); + var urlHelper = new Mock(); + urlHelper + .Setup(urlhelper => urlhelper.Content(It.IsAny())) + .Returns(new Func(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 diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs index 488014bc6a..26af3b02ea 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs +++ b/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>(); + var hostingEnvironment = MakeHostingEnvironment(); + var viewContext = MakeViewContext(); + var urlHelper = new Mock(); + urlHelper + .Setup(urlhelper => urlhelper.Content(It.IsAny())) + .Returns(new Func(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)