From a0da6ec19f17dd0579ab0424044778608e483997 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 31 Jul 2015 14:50:11 -0700 Subject: [PATCH] Add `AddHtmlAttributeValues` for `TagHelper`s. - Refactored `WriteAttributeTo` to allow re-use of some of the core attribute writing logic. The refactoring was based on removing the `Begin`/`EndContext` for instrumentation bits which isn't valid in a `TagHelper` attribute scenario. - Added unit tests to validate attributes are properly added to `TagHelperExecutionContext`. - Added functional test to validate everything is output as expected with dynamic attributes (encoded and non-encoded). aspnet/Razor#247 --- src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs | 121 +++++++++--- .../TagHelpersTest.cs | 33 ++++ ...Home.UnboundDynamicAttributes.Encoded.html | 17 ++ ...WebSite.Home.UnboundDynamicAttributes.html | 17 ++ .../RazorPageTest.cs | 183 ++++++++++++++++++ .../Controllers/HomeController.cs | 5 + .../AddProcessedAttributeTagHelper.cs | 16 ++ .../Home/UnboundDynamicAttributes.cshtml | 23 +++ 8 files changed, 391 insertions(+), 24 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html create mode 100644 test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.html create mode 100644 test/WebSites/TagHelpersWebSite/TagHelpers/AddProcessedAttributeTagHelper.cs create mode 100644 test/WebSites/TagHelpersWebSite/Views/Home/UnboundDynamicAttributes.cshtml diff --git a/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs b/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs index 7530970435..5a0eb161b2 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs @@ -524,25 +524,26 @@ namespace Microsoft.AspNet.Mvc.Razor // Explicitly empty attribute, so write the prefix WritePositionTaggedLiteral(writer, prefix); } - else if (values.Length == 1 && values[0].Prefix == "" && - (values[0].Value.Value is bool || values[0].Value.Value == null)) + else if (IsSingleBoolFalseOrNullValue(values)) + { + // Value is either null or the bool 'false' with no prefix; don't render the attribute. + return; + } + else if (UseAttributeNameAsValue(values)) { - // Value is either null or a bool with no prefix. var attributeValue = values[0]; var positionTaggedAttributeValue = attributeValue.Value; - if (positionTaggedAttributeValue.Value == null || !(bool)positionTaggedAttributeValue.Value) - { - // The value is null or just the bool 'false', don't write anything. - return; - } - WritePositionTaggedLiteral(writer, prefix); var sourceLength = suffix.Position - positionTaggedAttributeValue.Position; + var nameAttributeValue = new AttributeValue( + attributeValue.Prefix, + new PositionTagged(name, attributeValue.Value.Position), + literal: attributeValue.Literal); // The value is just the bool 'true', write the attribute name instead of the string 'True'. - WriteAttributeValue(writer, attributeValue, name, sourceLength); + WriteAttributeValue(writer, nameAttributeValue, sourceLength); } else { @@ -568,15 +569,60 @@ namespace Microsoft.AspNet.Mvc.Razor // Calculate length of the source span by the position of the next value (or suffix) var sourceLength = next.Position - attributeValue.Value.Position; - var stringValue = positionTaggedAttributeValue.Value as string; - - WriteAttributeValue(writer, attributeValue, stringValue, sourceLength); + WriteAttributeValue(writer, attributeValue, sourceLength); } } WritePositionTaggedLiteral(writer, suffix); } + public void AddHtmlAttributeValues( + string attributeName, + TagHelperExecutionContext executionContext, + params AttributeValue[] values) + { + if (IsSingleBoolFalseOrNullValue(values)) + { + // The first value was 'null' or 'false' indicating that we shouldn't render the attribute. The + // attribute is treated as a TagHelper attribute so it's only available in + // TagHelperContext.AllAttributes for TagHelper authors to see (if they want to see why the attribute + // was removed from TagHelperOutput.Attributes). + executionContext.AddTagHelperAttribute( + attributeName, + values[0].Value.Value?.ToString() ?? string.Empty); + + return; + } + else if (UseAttributeNameAsValue(values)) + { + executionContext.AddHtmlAttribute(attributeName, attributeName); + } + else + { + var valueBuffer = new StringCollectionTextWriter(Output.Encoding); + + foreach (var value in values) + { + if (value.Value.Value == null) + { + // Skip null values + continue; + } + + if (!string.IsNullOrEmpty(value.Prefix)) + { + WriteLiteralTo(valueBuffer, value.Prefix); + } + + WriteUnprefixedAttributeValueTo(valueBuffer, value); + } + + var htmlString = new HtmlString(valueBuffer.ToString()); + + executionContext.AddHtmlAttribute(attributeName, htmlString); + } + } + public virtual string Href([NotNull] string contentPath) { if (_urlHelper == null) @@ -587,14 +633,8 @@ namespace Microsoft.AspNet.Mvc.Razor return _urlHelper.Content(contentPath); } - private void WriteAttributeValue( - TextWriter writer, - AttributeValue attributeValue, - string stringValue, - int sourceLength) + private void WriteAttributeValue(TextWriter writer, AttributeValue attributeValue, int sourceLength) { - var positionTaggedAttributeValue = attributeValue.Value; - if (!string.IsNullOrEmpty(attributeValue.Prefix)) { WritePositionTaggedLiteral(writer, attributeValue.Prefix); @@ -602,8 +642,17 @@ namespace Microsoft.AspNet.Mvc.Razor BeginContext(attributeValue.Value.Position, sourceLength, isLiteral: attributeValue.Literal); - // The extra branching here is to ensure that we call the Write*To(string) overload where - // possible. + WriteUnprefixedAttributeValueTo(writer, attributeValue); + + EndContext(); + } + + private void WriteUnprefixedAttributeValueTo(TextWriter writer, AttributeValue attributeValue) + { + var positionTaggedAttributeValue = attributeValue.Value; + var stringValue = positionTaggedAttributeValue.Value as string; + + // The extra branching here is to ensure that we call the Write*To(string) overload where possible. if (attributeValue.Literal && stringValue != null) { WriteLiteralTo(writer, stringValue); @@ -620,8 +669,6 @@ namespace Microsoft.AspNet.Mvc.Razor { WriteTo(writer, positionTaggedAttributeValue.Value); } - - EndContext(); } private void WritePositionTaggedLiteral(TextWriter writer, string value, int position) @@ -846,6 +893,32 @@ namespace Microsoft.AspNet.Mvc.Razor return HtmlString.Empty; } + private bool IsSingleBoolFalseOrNullValue(AttributeValue[] values) + { + if (values.Length == 1 && string.IsNullOrEmpty(values[0].Prefix) && + (values[0].Value.Value is bool || values[0].Value.Value == null)) + { + var attributeValue = values[0]; + var positionTaggedAttributeValue = attributeValue.Value; + + if (positionTaggedAttributeValue.Value == null || !(bool)positionTaggedAttributeValue.Value) + { + return true; + } + } + + return false; + } + + private bool UseAttributeNameAsValue(AttributeValue[] values) + { + // If the value is just the bool 'true', use the attribute name as the value. + return values.Length == 1 && + string.IsNullOrEmpty(values[0].Prefix) && + values[0].Value.Value is bool && + (bool)values[0].Value.Value; + } + private void EnsureMethodCanBeInvoked(string methodName) { if (PreviousSectionWriters == null) diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/TagHelpersTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/TagHelpersTest.cs index f4cc52b9a3..02608163ba 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/TagHelpersTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/TagHelpersTest.cs @@ -11,6 +11,7 @@ using System.Threading.Tasks; using BasicWebSite; using Microsoft.AspNet.Builder; using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.WebEncoders; using Xunit; namespace Microsoft.AspNet.Mvc.FunctionalTests @@ -32,6 +33,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests [InlineData("Index")] [InlineData("About")] [InlineData("Help")] + [InlineData("UnboundDynamicAttributes")] public async Task CanRenderViewsWithTagHelpers(string action) { // Arrange @@ -58,6 +60,37 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests #endif } + [Fact] + public async Task CanRenderViewsWithTagHelpersAndUnboundDynamicAttributes_Encoded() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, services => + { + _configureServices(services); + services.AddTransient(); + }); + var client = server.CreateClient(); + var expectedMediaType = MediaTypeHeaderValue.Parse("text/html; charset=utf-8"); + var outputFile = "compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html"; + var expectedContent = + await ResourceFile.ReadResourceAsync(_resourcesAssembly, outputFile, sourceFile: false); + + // Act + // The host is not important as everything runs in memory and tests are isolated from each other. + var response = await client.GetAsync("http://localhost/Home/UnboundDynamicAttributes"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(expectedMediaType, response.Content.Headers.ContentType); + + var responseContent = await response.Content.ReadAsStringAsync(); +#if GENERATE_BASELINES + ResourceFile.UpdateFile(_resourcesAssembly, outputFile, expectedContent, responseContent); +#else + Assert.Equal(expectedContent, responseContent, ignoreLineEndingDifferences: true); +#endif + } + public static TheoryData TagHelpersAreInheritedFromViewImportsPagesData { get diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html new file mode 100644 index 0000000000..fa611fbd9c --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.Encoded.html @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.html new file mode 100644 index 0000000000..ae8ec7457e --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Home.UnboundDynamicAttributes.html @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs index 073868e89a..d9830f7a37 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs @@ -742,6 +742,189 @@ namespace Microsoft.AspNet.Mvc.Razor context.Verify(); } + public static TheoryData AddHtmlAttributeValues_ValueData + { + get + { + // attributeValues, expectedValue + return new TheoryData + { + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged("", 9), + new PositionTagged("Hello", 9), + literal: true) + }, + "Hello" + }, + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged(" ", 9), + new PositionTagged("Hello", 10), + literal: true) + }, + " Hello" + }, + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged(" ", 9), + new PositionTagged(null, 10), + literal: false) + }, + "" + }, + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged(" ", 9), + new PositionTagged(false, 10), + literal: false) + }, + " HtmlEncode[[False]]" + }, + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged(" ", 9), + new PositionTagged(true, 11), + literal: false), + new AttributeValue( + new PositionTagged(" ", 15), + new PositionTagged("abcd", 17), + literal: true), + }, + " HtmlEncode[[True]] abcd" + }, + + { + new AttributeValue[] { + new AttributeValue( + new PositionTagged("", 9), + new PositionTagged("prefix", 9), + literal: true), + new AttributeValue( + new PositionTagged(" ", 15), + new PositionTagged(null, 17), + literal: false), + new AttributeValue( + new PositionTagged(" ", 21), + new PositionTagged("suffix", 22), + literal: false), + }, + "prefix HtmlEncode[[suffix]]" + }, + }; + } + } + + [Theory] + [MemberData(nameof(AddHtmlAttributeValues_ValueData))] + public void AddHtmlAttributeValues_AddsToHtmlAttributesAsExpected( + AttributeValue[] attributeValues, + string expectedValue) + { + // Arrange + var page = CreatePage(p => { }); + page.HtmlEncoder = new CommonTestEncoder(); + var executionContext = new TagHelperExecutionContext( + "p", + selfClosing: false, + items: null, + uniqueId: string.Empty, + executeChildContentAsync: () => Task.FromResult(result: true), + startTagHelperWritingScope: () => { }, + endTagHelperWritingScope: () => new DefaultTagHelperContent()); + + // Act + page.AddHtmlAttributeValues("someattr", executionContext, attributeValues); + + // Assert + var htmlAttribute = Assert.Single(executionContext.HTMLAttributes); + Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal); + Assert.IsType(htmlAttribute.Value); + Assert.Equal(expectedValue, htmlAttribute.Value.ToString(), StringComparer.Ordinal); + Assert.False(htmlAttribute.Minimized); + var allAttribute = Assert.Single(executionContext.AllAttributes); + Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal); + Assert.IsType(allAttribute.Value); + Assert.Equal(expectedValue, allAttribute.Value.ToString(), StringComparer.Ordinal); + Assert.False(allAttribute.Minimized); + } + + [Theory] + [InlineData(null, "")] + [InlineData(false, "False")] + public void AddHtmlAttributeValues_OnlyAddsToAllAttributesWhenAttributeRemoved( + object attributeValue, + string expectedValue) + { + // Arrange + var page = CreatePage(p => { }); + page.HtmlEncoder = new CommonTestEncoder(); + var executionContext = new TagHelperExecutionContext( + "p", + selfClosing: false, + items: null, + uniqueId: string.Empty, + executeChildContentAsync: () => Task.FromResult(result: true), + startTagHelperWritingScope: () => { }, + endTagHelperWritingScope: () => new DefaultTagHelperContent()); + + // Act + page.AddHtmlAttributeValues( + "someattr", + executionContext, + new AttributeValue( + prefix: new PositionTagged(string.Empty, 9), + value: new PositionTagged(attributeValue, 9), + literal: false)); + + // Assert + Assert.Empty(executionContext.HTMLAttributes); + var attribute = Assert.Single(executionContext.AllAttributes); + Assert.Equal("someattr", attribute.Name, StringComparer.Ordinal); + Assert.Equal(expectedValue, (string)attribute.Value, StringComparer.Ordinal); + Assert.False(attribute.Minimized); + } + + [Fact] + public void AddHtmlAttributeValues_AddsAttributeNameAsValueWhenValueIsUnprefixedTrue() + { + // Arrange + var page = CreatePage(p => { }); + page.HtmlEncoder = new CommonTestEncoder(); + var executionContext = new TagHelperExecutionContext( + "p", + selfClosing: false, + items: null, + uniqueId: string.Empty, + executeChildContentAsync: () => Task.FromResult(result: true), + startTagHelperWritingScope: () => { }, + endTagHelperWritingScope: () => new DefaultTagHelperContent()); + + // Act + page.AddHtmlAttributeValues( + "someattr", + executionContext, + new AttributeValue( + prefix: new PositionTagged(string.Empty, 9), + value: new PositionTagged(true, 9), + literal: false)); + + // Assert + var htmlAttribute = Assert.Single(executionContext.HTMLAttributes); + 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); + Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal); + Assert.Equal("someattr", (string)allAttribute.Value, StringComparer.Ordinal); + Assert.False(allAttribute.Minimized); + } + public static TheoryData WriteAttributeData { get diff --git a/test/WebSites/TagHelpersWebSite/Controllers/HomeController.cs b/test/WebSites/TagHelpersWebSite/Controllers/HomeController.cs index 3ee2955956..75f27e6f2f 100644 --- a/test/WebSites/TagHelpersWebSite/Controllers/HomeController.cs +++ b/test/WebSites/TagHelpersWebSite/Controllers/HomeController.cs @@ -30,6 +30,11 @@ namespace TagHelpersWebSite.Controllers return View(); } + public ViewResult UnboundDynamicAttributes() + { + return View(); + } + public ViewResult NestedViewImportsTagHelper() { return View(); diff --git a/test/WebSites/TagHelpersWebSite/TagHelpers/AddProcessedAttributeTagHelper.cs b/test/WebSites/TagHelpersWebSite/TagHelpers/AddProcessedAttributeTagHelper.cs new file mode 100644 index 0000000000..150b436b55 --- /dev/null +++ b/test/WebSites/TagHelpersWebSite/TagHelpers/AddProcessedAttributeTagHelper.cs @@ -0,0 +1,16 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Razor.Runtime.TagHelpers; + +namespace TagHelpersWebSite.TagHelpers +{ + [TargetElement("input")] + public class AddProcessedAttributeTagHelper : TagHelper + { + public override void Process(TagHelperContext context, TagHelperOutput output) + { + output.Attributes.Add(new TagHelperAttribute { Name = "processed", Minimized = true }); + } + } +} diff --git a/test/WebSites/TagHelpersWebSite/Views/Home/UnboundDynamicAttributes.cshtml b/test/WebSites/TagHelpersWebSite/Views/Home/UnboundDynamicAttributes.cshtml new file mode 100644 index 0000000000..0dec871399 --- /dev/null +++ b/test/WebSites/TagHelpersWebSite/Views/Home/UnboundDynamicAttributes.cshtml @@ -0,0 +1,23 @@ +@addTagHelper "TagHelpersWebSite.TagHelpers.AddProcessedAttributeTagHelper, TagHelpersWebSite" + +@{ + Layout = null; + var trueVar = true; + var falseVar = false; + var stringVar = "value"; + string nullVar = null; +} + + + + + + + + + + + + + + \ No newline at end of file