diff --git a/Mvc.NoFun.sln b/Mvc.NoFun.sln index f67e635663..bdbfba511f 100644 --- a/Mvc.NoFun.sln +++ b/Mvc.NoFun.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 14 -VisualStudioVersion = 14.0.24720.0 +VisualStudioVersion = 14.0.25420.1 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{DAAE4C74-D06F-4874-A166-33305D2643CE}" EndProject diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs index a21dda1190..2b9d480c25 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/FormTagHelper.cs @@ -226,7 +226,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (tagBuilder != null) { output.MergeAttributes(tagBuilder); - output.PostContent.AppendHtml(tagBuilder.InnerHtml); + if (tagBuilder.HasInnerHtml) + { + output.PostContent.AppendHtml(tagBuilder.InnerHtml); + } } if (string.Equals(Method, "get", StringComparison.OrdinalIgnoreCase)) diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs index 1864c457b7..2e51db1fd2 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/InputTagHelper.cs @@ -218,10 +218,14 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (tagBuilder != null) { - // This TagBuilder contains the one element of interest. Since this is not the "checkbox" - // special-case, output is a self-closing element no longer guaranteed. + // This TagBuilder contains the one element of interest. output.MergeAttributes(tagBuilder); - output.Content.AppendHtml(tagBuilder.InnerHtml); + if (tagBuilder.HasInnerHtml) + { + // Since this is not the "checkbox" special-case, no guarantee that output is a self-closing + // element. A later tag helper targeting this element may change output.TagMode. + output.Content.AppendHtml(tagBuilder.InnerHtml); + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/LabelTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/LabelTagHelper.cs index a1127e9e5e..d8aefe3229 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/LabelTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/LabelTagHelper.cs @@ -72,17 +72,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { output.MergeAttributes(tagBuilder); - // We check for whitespace to detect scenarios such as: - // + // Do not update the content if another tag helper targeting this element has already done so. if (!output.IsContentModified) { + // We check for whitespace to detect scenarios such as: + // var childContent = await output.GetChildContentAsync(); - if (childContent.IsEmptyOrWhiteSpace) { - // Provide default label text since there was nothing useful in the Razor source. - output.Content.SetHtmlContent(tagBuilder.InnerHtml); + // Provide default label text (if any) since there was nothing useful in the Razor source. + if (tagBuilder.HasInnerHtml) + { + output.Content.SetHtmlContent(tagBuilder.InnerHtml); + } } else { diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/SelectTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/SelectTagHelper.cs index 794ee1a4ee..af12f5fe24 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/SelectTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/SelectTagHelper.cs @@ -147,7 +147,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (tagBuilder != null) { output.MergeAttributes(tagBuilder); - output.PostContent.AppendHtml(tagBuilder.InnerHtml); + if (tagBuilder.HasInnerHtml) + { + output.PostContent.AppendHtml(tagBuilder.InnerHtml); + } } } } diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/TextAreaTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/TextAreaTagHelper.cs index 5863780626..e8b6893b15 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/TextAreaTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/TextAreaTagHelper.cs @@ -70,10 +70,12 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (tagBuilder != null) { - // Overwrite current Content to ensure expression result round-trips correctly. - output.Content.SetHtmlContent(tagBuilder.InnerHtml); - output.MergeAttributes(tagBuilder); + if (tagBuilder.HasInnerHtml) + { + // Overwrite current Content to ensure expression result round-trips correctly. + output.Content.SetHtmlContent(tagBuilder.InnerHtml); + } } } } diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationMessageTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationMessageTagHelper.cs index fd4f02fcad..5d6ec1ea6d 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationMessageTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationMessageTagHelper.cs @@ -76,17 +76,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers { output.MergeAttributes(tagBuilder); - // We check for whitespace to detect scenarios such as: - // - // + // Do not update the content if another tag helper targeting this element has already done so. if (!output.IsContentModified) { + // We check for whitespace to detect scenarios such as: + // + // var childContent = await output.GetChildContentAsync(); - if (childContent.IsEmptyOrWhiteSpace) { - // Provide default label text since there was nothing useful in the Razor source. - output.Content.SetHtmlContent(tagBuilder.InnerHtml); + // Provide default message text (if any) since there was nothing useful in the Razor source. + if (tagBuilder.HasInnerHtml) + { + output.Content.SetHtmlContent(tagBuilder.InnerHtml); + } } else { diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationSummaryTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationSummaryTagHelper.cs index ddd5117cd7..3a68c793a1 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationSummaryTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ValidationSummaryTagHelper.cs @@ -112,7 +112,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } output.MergeAttributes(tagBuilder); - output.PostContent.AppendHtml(tagBuilder.InnerHtml); + if (tagBuilder.HasInnerHtml) + { + output.PostContent.AppendHtml(tagBuilder.InnerHtml); + } } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultDisplayTemplates.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultDisplayTemplates.cs index 96112c6390..b2c606cb7d 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultDisplayTemplates.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultDisplayTemplates.cs @@ -93,8 +93,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return HtmlString.Empty; } - var collection = model as IEnumerable; - if (collection == null) + var enumerable = model as IEnumerable; + if (enumerable == null) { // Only way we could reach here is if user passed templateName: "Collection" to a Display() overload. throw new InvalidOperationException(Resources.FormatTemplates_TypeMustImplementIEnumerable( @@ -119,12 +119,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { htmlHelper.ViewData.TemplateInfo.HtmlFieldPrefix = string.Empty; - var result = new HtmlContentBuilder(); + var collection = model as ICollection; + var result = collection == null ? new HtmlContentBuilder() : new HtmlContentBuilder(collection.Count); var viewEngine = serviceProvider.GetRequiredService(); var viewBufferScope = serviceProvider.GetRequiredService(); var index = 0; - foreach (var item in collection) + foreach (var item in enumerable) { var itemMetadata = elementMetadata; if (item != null && !typeInCollectionIsNullableValueType) @@ -224,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var viewEngine = serviceProvider.GetRequiredService(); var viewBufferScope = serviceProvider.GetRequiredService(); - var content = new HtmlContentBuilder(); + var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); foreach (var propertyExplorer in modelExplorer.Properties) { var propertyMetadata = propertyExplorer.Metadata; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs index 328975701f..1fce1dc8d5 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs @@ -60,8 +60,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal return HtmlString.Empty; } - var collection = model as IEnumerable; - if (collection == null) + var enumerable = model as IEnumerable; + if (enumerable == null) { // Only way we could reach here is if user passed templateName: "Collection" to an Editor() overload. throw new InvalidOperationException(Resources.FormatTemplates_TypeMustImplementIEnumerable( @@ -86,12 +86,13 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal { viewData.TemplateInfo.HtmlFieldPrefix = string.Empty; - var result = new HtmlContentBuilder(); + var collection = model as ICollection; + var result = collection == null ? new HtmlContentBuilder() : new HtmlContentBuilder(collection.Count); var viewEngine = serviceProvider.GetRequiredService(); var viewBufferScope = serviceProvider.GetRequiredService(); var index = 0; - foreach (var item in collection) + foreach (var item in enumerable) { var itemMetadata = elementMetadata; if (item != null && !typeInCollectionIsNullableValueType) @@ -143,24 +144,26 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var viewData = htmlHelper.ViewData; var model = viewData.Model; - var result = new HtmlContentBuilder(); - if (!viewData.ModelMetadata.HideSurroundingHtml) + IHtmlContent display; + if (viewData.ModelMetadata.HideSurroundingHtml) { - result.AppendHtml(DefaultDisplayTemplates.StringTemplate(htmlHelper)); + display = null; } - - // Special-case opaque values and arbitrary binary data. - var modelAsByteArray = model as byte[]; - if (modelAsByteArray != null) + else { - model = Convert.ToBase64String(modelAsByteArray); + display = DefaultDisplayTemplates.StringTemplate(htmlHelper); } var htmlAttributesObject = viewData[HtmlAttributeKey]; - var hiddenResult = htmlHelper.Hidden(expression: null, value: model, htmlAttributes: htmlAttributesObject); - result.AppendHtml(hiddenResult); + var hidden = htmlHelper.Hidden(expression: null, value: model, htmlAttributes: htmlAttributesObject); + if (viewData.ModelMetadata.HideSurroundingHtml) + { + return hidden; + } - return result; + return new HtmlContentBuilder(capacity: 2) + .AppendHtml(display) + .AppendHtml(hidden); } private static IDictionary CreateHtmlAttributes( @@ -250,7 +253,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var viewEngine = serviceProvider.GetRequiredService(); var viewBufferScope = serviceProvider.GetRequiredService(); - var content = new HtmlContentBuilder(); + var content = new HtmlContentBuilder(modelExplorer.Metadata.Properties.Count); foreach (var propertyExplorer in modelExplorer.Properties) { var propertyMetadata = propertyExplorer.Metadata; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/TagBuilder.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/TagBuilder.cs index 723f5a1cb1..eed02e76f4 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/TagBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Rendering/TagBuilder.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Rendering public class TagBuilder : IHtmlContent { private AttributeDictionary _attributes; + private HtmlContentBuilder _innerHtml; /// /// Creates a new HTML tag that has the specified tag name. @@ -34,7 +35,6 @@ namespace Microsoft.AspNetCore.Mvc.Rendering } TagName = tagName; - InnerHtml = new HtmlContentBuilder(); } /// @@ -57,7 +57,23 @@ namespace Microsoft.AspNetCore.Mvc.Rendering /// /// Gets the inner HTML content of the element. /// - public IHtmlContentBuilder InnerHtml { get; } + public IHtmlContentBuilder InnerHtml + { + get + { + if (_innerHtml == null) + { + _innerHtml = new HtmlContentBuilder(); + } + + return _innerHtml; + } + } + + /// + /// Gets an indication is not empty. + /// + public bool HasInnerHtml => _innerHtml?.Count > 0; /// /// Gets the tag name for this tag. @@ -291,7 +307,11 @@ namespace Microsoft.AspNetCore.Mvc.Rendering writer.Write(TagName); AppendAttributes(writer, encoder); writer.Write(">"); - InnerHtml.WriteTo(writer, encoder); + if (_innerHtml != null) + { + _innerHtml.WriteTo(writer, encoder); + } + writer.Write(""); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs index b7efa33a20..514cde5faf 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs @@ -881,21 +881,20 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures return null; } - var wrappedMessage = new HtmlContentBuilder(); - if (!string.IsNullOrEmpty(message)) + TagBuilder messageTag; + if (string.IsNullOrEmpty(message)) + { + messageTag = null; + } + else { if (string.IsNullOrEmpty(headerTag)) { headerTag = viewContext.ValidationSummaryMessageElement; } - var messageTag = new TagBuilder(headerTag); + messageTag = new TagBuilder(headerTag); messageTag.InnerHtml.SetContent(message); - wrappedMessage.AppendLine(messageTag); - } - else - { - wrappedMessage = null; } // If excludePropertyErrors is true, describe any validation issue with the current model in a single item. @@ -940,7 +939,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures tagBuilder.AddCssClass(HtmlHelper.ValidationSummaryCssClassName); } - tagBuilder.InnerHtml.AppendHtml(wrappedMessage); + if (messageTag != null) + { + tagBuilder.InnerHtml.AppendLine(messageTag); + } + tagBuilder.InnerHtml.AppendHtml(htmlSummary); if (viewContext.ClientValidationEnabled && !excludePropertyErrors) @@ -1518,7 +1521,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures IEnumerable selectList, ICollection currentValues) { - var listItemBuilder = new HtmlContentBuilder(); + var itemsList = selectList as IList; + if (itemsList == null) + { + itemsList = selectList.ToList(); + } + + var count = itemsList.Count; + if (optionLabel != null) + { + count++; + } + + // Short-circuit work below if there's nothing to add. + if (count == 0) + { + return HtmlString.Empty; + } + + var listItemBuilder = new HtmlContentBuilder(count); // Make optionLabel the first item that gets rendered. if (optionLabel != null) @@ -1533,12 +1554,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures currentValues: null)); } - var itemsList = selectList as IList; - if (itemsList == null) - { - itemsList = selectList.ToList(); - } - // Group items in the SelectList if requested. // The worst case complexity of this algorithm is O(number of groups*n). // If there aren't any groups, it is O(n) where n is number of items in the list. diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs index f1e9eaf475..666b6bef78 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs @@ -769,24 +769,21 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures isChecked, htmlAttributes); - var hiddenForCheckboxTag = _htmlGenerator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, expression); - if (checkbox == null || hiddenForCheckboxTag == null) + var hiddenForCheckbox = _htmlGenerator.GenerateHiddenForCheckbox(ViewContext, modelExplorer, expression); + if (checkbox == null || hiddenForCheckbox == null) { return HtmlString.Empty; } - var checkboxContent = new HtmlContentBuilder().AppendHtml(checkbox); - if (ViewContext.FormContext.CanRenderAtEndOfForm) { - ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckboxTag); - } - else - { - checkboxContent.AppendHtml(hiddenForCheckboxTag); + ViewContext.FormContext.EndOfFormContent.Add(hiddenForCheckbox); + return checkbox; } - return checkboxContent; + return new HtmlContentBuilder(capacity: 2) + .AppendHtml(checkbox) + .AppendHtml(hiddenForCheckbox); } protected virtual string GenerateDisplayName(ModelExplorer modelExplorer, string expression) diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/TagBuilderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/TagBuilderTest.cs index 87d0f7ff0e..55a37da7e4 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/TagBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/TagBuilderTest.cs @@ -117,7 +117,10 @@ namespace Microsoft.AspNetCore.Mvc.Core.Rendering [InlineData("attribute", "value", "

")] [InlineData("attribute", null, "

")] [InlineData("attribute", "", "

")] - public void WriteTo_WriteEmptyAttribute_WhenValueIsNullOrEmpty(string attributeKey, string attributeValue, string expectedOutput) + public void WriteTo_WriteEmptyAttribute_WhenValueIsNullOrEmpty( + string attributeKey, + string attributeValue, + string expectedOutput) { // Arrange var tagBuilder = new TagBuilder("p"); @@ -149,6 +152,22 @@ namespace Microsoft.AspNetCore.Mvc.Core.Rendering // Assert Assert.Equal("

HelloHtmlEncode[[, World!]]

", writer.ToString()); } + + Assert.True(tagBuilder.HasInnerHtml); + } + + [Fact] + public void ReadingInnerHtml_LeavesHasInnerHtmlFalse() + { + // Arrange + var tagBuilder = new TagBuilder("p"); + + // Act + var innerHtml = tagBuilder.InnerHtml; + + // Assert + Assert.False(tagBuilder.HasInnerHtml); + Assert.NotNull(innerHtml); } } } \ No newline at end of file