Remove no-op behaviour for no-content `<label></label>` elements

- #6638

src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/DefaultHtmlGenerator.cs

@@ -419,11 +419,6 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
                 }
             }
 
-            if (string.IsNullOrEmpty(resolvedLabelText))
-            {
-                return null;
-            }
-
             var tagBuilder = new TagBuilder("label");
             var fullName = NameAndIdProvider.GetFullHtmlFieldName(viewContext, expression);
             var idString = NameAndIdProvider.CreateSanitizedId(viewContext, fullName, IdAttributeDotReplacement);

src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs

@@ -977,6 +977,25 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures
                 return HtmlString.Empty;
             }
 
+            // Do not generate an empty <label> element unless user passed string.Empty for the label text. This is
+            // primarily done for back-compatibility. (Note HtmlContentBuilder ignores (no-ops) an attempt to add
+            // string.Empty. So tagBuilder.HasInnerHtml isn't sufficient here.)
+            if (!tagBuilder.HasInnerHtml && labelText == null)
+            {
+                if (tagBuilder.Attributes.Count == 0)
+                {
+                    // Element has no content and no attributes.
+                    return HtmlString.Empty;
+                }
+                else if (tagBuilder.Attributes.Count == 1 &&
+                    tagBuilder.Attributes.TryGetValue("for", out var forAttribute) &&
+                    string.IsNullOrEmpty(forAttribute))
+                {
+                    // Element has no content and only an empty (therefore useless) "for" attribute.
+                    return HtmlString.Empty;
+                }
+            }
+
             return tagBuilder;
         }
 

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

@@ -3,10 +3,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
-using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.AspNetCore.Mvc.TestCommon;
 using Microsoft.AspNetCore.Mvc.ViewFeatures;
 using Microsoft.AspNetCore.Razor.TagHelpers;
@@ -17,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
     public class LabelTagHelperTest
     {
         // Model (List<Model> or Model instance), container type (Model or NestModel), model accessor,
-        // property path, TagHelperOutput.Content values.
+        // property path, TagHelperOutput values. All accessors should end at a Text property.
         public static TheoryData<object, Type, Func<object>, string, TagHelperOutputContent> TestDataSet
         {
             get
@@ -101,8 +99,6 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                     { modelWithText, typeof(NestedModel), () => modelWithText.NestedModel.Text, "NestedModel.Text",
                         new TagHelperOutputContent("Hello World1", "Hello World2", "Hello World2", "NestedModel_Text") },
 
-                    // Note: Tests cases below here will not work in practice due to current limitations on indexing
-                    // into ModelExpressions. Will be fixed in https://github.com/aspnet/Mvc/issues/1345.
                     { models, typeof(Model), () => models[0].Text, "[0].Text",
                         new TagHelperOutputContent(Environment.NewLine, string.Empty, "HtmlEncode[[Text]]", "z0__Text") },
                     { models, typeof(Model), () => models[0].Text, "[0].Text",
@@ -234,6 +230,109 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             Assert.Equal(expectedTagName, output.TagName);
         }
 
+        // Display name, original child content, HTML field prefix, expected child content, and expected ID.
+        // Uses TagHelperOutputContent.OriginalContent to pass HtmlFieldPrefix values.
+        public static TheoryData<string, string, string, string, string> DisplayNameDataSet
+        {
+            get
+            {
+                return new TheoryData<string, string, string, string, string>
+                {
+                    {
+                        null, string.Empty, string.Empty, $"HtmlEncode[[{nameof(NestedModel.Text)}]]", nameof(NestedModel.Text)
+                    },
+                    {
+                        string.Empty, string.Empty, string.Empty, string.Empty, nameof(NestedModel.Text)
+                    },
+                    {
+                        "a label", string.Empty, string.Empty, "HtmlEncode[[a label]]", nameof(NestedModel.Text)
+                    },
+                    {
+                        null, "original label", string.Empty, "original label", nameof(NestedModel.Text)
+                    },
+                    {
+                        string.Empty, "original label", string.Empty, "original label", nameof(NestedModel.Text)
+                    },
+                    {
+                        "a label", "original label", string.Empty, "original label", nameof(NestedModel.Text)
+                    },
+                    {
+                        null, string.Empty, "prefix", $"HtmlEncode[[{nameof(NestedModel.Text)}]]", $"prefix_{nameof(NestedModel.Text)}"
+                    },
+                    {
+                        string.Empty, string.Empty, "prefix", string.Empty, $"prefix_{nameof(NestedModel.Text)}"
+                    },
+                    {
+                        "a label", string.Empty, "prefix", "HtmlEncode[[a label]]", $"prefix_{nameof(NestedModel.Text)}"
+                    },
+                };
+            }
+        }
+
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing in this test when displayName was empty.
+        [Theory]
+        [MemberData(nameof(DisplayNameDataSet))]
+        public async Task ProcessAsync_GeneratesExpectedOutput_WithDisplayName(
+            string displayName,
+            string originalChildContent,
+            string htmlFieldPrefix,
+            string expectedContent,
+            string expectedId)
+        {
+            // Arrange
+            var expectedAttributes = new TagHelperAttributeList
+            {
+                { "for", expectedId }
+            };
+
+            var name = nameof(NestedModel.Text);
+            var metadataProvider = new TestModelMetadataProvider();
+            metadataProvider
+                .ForProperty<NestedModel>(name)
+                .DisplayDetails(metadata => metadata.DisplayName = () => displayName);
+
+            var htmlGenerator = new TestableHtmlGenerator(metadataProvider);
+            var viewContext = TestableHtmlGenerator.GetViewContext(
+                model: null,
+                htmlGenerator: htmlGenerator,
+                metadataProvider: metadataProvider);
+
+            var viewData = new ViewDataDictionary<NestedModel>(metadataProvider, viewContext.ModelState);
+            viewData.TemplateInfo.HtmlFieldPrefix = htmlFieldPrefix;
+            viewContext.ViewData = viewData;
+
+            var containerExplorer = metadataProvider.GetModelExplorerForType(typeof(NestedModel), model: null);
+            var modelExplorer = containerExplorer.GetExplorerForProperty(name);
+            var modelExpression = new ModelExpression(name, modelExplorer);
+            var tagHelper = new LabelTagHelper(htmlGenerator)
+            {
+                For = modelExpression,
+                ViewContext = viewContext,
+            };
+
+            var tagHelperContext = new TagHelperContext(
+                tagName: "label",
+                allAttributes: new TagHelperAttributeList(),
+                items: new Dictionary<object, object>(),
+                uniqueId: "test");
+            var output = new TagHelperOutput(
+                "label",
+                new TagHelperAttributeList(),
+                getChildContentAsync: (useCachedResult, encoder) =>
+                {
+                    var tagHelperContent = new DefaultTagHelperContent();
+                    tagHelperContent.AppendHtml(originalChildContent);
+                    return Task.FromResult<TagHelperContent>(tagHelperContent);
+                });
+
+            // Act
+            await tagHelper.ProcessAsync(tagHelperContext, output);
+
+            // Assert
+            Assert.Equal(expectedAttributes, output.Attributes);
+            Assert.Equal(expectedContent, HtmlContentUtilities.HtmlContentToString(output.Content));
+        }
+
         public class TagHelperOutputContent
         {
             public TagHelperOutputContent(

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs

@@ -199,6 +199,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             Assert.Equal(expected, HtmlContentUtilities.HtmlContentToString(result));
         }
 
+        // Prior to aspnet/Mvc#6638 fix, helper did not generate Property1 <label> or containing <div> with this setup.
         // Expect almost the same HTML as in ObjectTemplateEditsSimplePropertiesOnObjectByDefault(). Only difference is
         // the <div class="editor-label">...</div> is not present for Property1.
         [Fact]
@@ -206,6 +207,8 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
         {
             // Arrange
             var expected =
+                "<div class=\"HtmlEncode[[editor-label]]\"><label for=\"HtmlEncode[[Property1]]\"></label></div>" +
+                Environment.NewLine +
                 "<div class=\"HtmlEncode[[editor-field]]\">Model = p1, ModelType = System.String, PropertyName = Property1, SimpleDisplayText = p1 " +
                 "<span class=\"HtmlEncode[[field-validation-valid]]\" data-valmsg-for=\"HtmlEncode[[Property1]]\" data-valmsg-replace=\"HtmlEncode[[true]]\">" +
                 "</span></div>" +

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperLabelExtensionsTest.cs

@@ -34,6 +34,47 @@ namespace Microsoft.AspNetCore.Mvc.Core
             Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelForModelResult));
         }
 
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForModel_WithLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"\">HtmlEncode[[a label]]</label>";
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper();
+
+            // Act
+            var labelResult = helper.Label(expression: string.Empty, labelText: "a label");
+            var labelNullResult = helper.Label(expression: null, labelText: "a label");
+            var labelForResult = helper.LabelFor(m => m, labelText: "a label");
+            var labelForModelResult = helper.LabelForModel(labelText: "a label");
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForModel_WithEmptyLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"\"></label>";
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper();
+
+            // Act
+            var labelResult = helper.Label(expression: string.Empty, labelText: string.Empty);
+            var labelNullResult = helper.Label(expression: null, labelText: string.Empty);
+            var labelForResult = helper.LabelFor(m => m, labelText: string.Empty);
+            var labelForModelResult = helper.LabelForModel(labelText: string.Empty);
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
         [Fact]
         public void LabelHelpers_DisplayPropertyName()
         {
@@ -107,6 +148,8 @@ namespace Microsoft.AspNetCore.Mvc.Core
             Assert.Equal("<label for=\"HtmlEncode[[value]]\">HtmlEncode[[value]]</label>", HtmlContentUtilities.HtmlContentToString(labelResult));
         }
 
+        // Following test is identical to LabelHelpers_ReturnEmptyForModel() from the HTML helpers' perspective. But,
+        // test confirms the added metadata does not change the behavior.
         [Fact]
         public void LabelHelpers_ReturnEmptyForModel_IfDisplayNameEmpty()
         {
@@ -131,6 +174,86 @@ namespace Microsoft.AspNetCore.Mvc.Core
             Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelForModelResult));
         }
 
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        // Following test mimics use of an identity expression in an editor template if invoked for an element in a
+        // collection. See also LabelHelpers_ReturnExpectedElementForProperty_IfDisplayNameEmptyAndNotTopLevel().
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForModel_IfDisplayNameEmptyAndNotTopLevel()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"HtmlEncode[[prefix]]\"></label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForType<DefaultTemplatesUtilities.ObjectTemplateModel>()
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+            helper.ViewData.TemplateInfo.HtmlFieldPrefix = "prefix";
+
+            // Act
+            var labelResult = helper.Label(expression: string.Empty);
+            var labelNullResult = helper.Label(expression: null);   // null is another alias for current model
+            var labelForResult = helper.LabelFor(m => m);
+            var labelForModelResult = helper.LabelForModel();
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForModel_IfDisplayNameEmpty_WithLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"\">HtmlEncode[[a label]]</label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForType<DefaultTemplatesUtilities.ObjectTemplateModel>()
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+
+            // Act
+            var labelResult = helper.Label(expression: string.Empty, labelText: "a label");
+            var labelNullResult = helper.Label(expression: null, labelText: "a label");
+            var labelForResult = helper.LabelFor(m => m, labelText: "a label");
+            var labelForModelResult = helper.LabelForModel(labelText: "a label");
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForModel_IfDisplayNameEmpty_WithEmptyLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"\"></label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForType<DefaultTemplatesUtilities.ObjectTemplateModel>()
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+
+            // Act
+            var labelResult = helper.Label(expression: string.Empty, labelText: string.Empty);
+            var labelNullResult = helper.Label(expression: null, labelText: string.Empty);
+            var labelForResult = helper.LabelFor(m => m, labelText: string.Empty);
+            var labelForModelResult = helper.LabelForModel(labelText: string.Empty);
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
         [Theory]
         [InlineData("DisplayName")]
         [InlineData("Custom display name from metadata")]
@@ -155,20 +278,24 @@ namespace Microsoft.AspNetCore.Mvc.Core
             Assert.Equal("<label for=\"\">HtmlEncode[[" + displayName + "]]</label>", HtmlContentUtilities.HtmlContentToString(labelForModelResult));
         }
 
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        // Following test mimics use of an identity expression in an editor template if invoked for a property. See
+        // also LabelHelpers_ReturnExpectedElementForModel_IfDisplayNameEmptyAndNotTopLevel().
         [Fact]
-        public void LabelHelpers_ReturnEmptyForProperty_IfDisplayNameEmpty()
+        public void LabelHelpers_ReturnExpectedElementForProperty_IfDisplayNameEmptyAndNotTopLevel()
         {
             // Arrange
+            var expectedLabel = "<label for=\"HtmlEncode[[Property1]]\"></label>";
             var provider = new TestModelMetadataProvider();
             provider
                 .ForProperty<DefaultTemplatesUtilities.ObjectTemplateModel>("Property1")
                 .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
 
-            var modelExplorer = provider
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper<string>(provider: provider);
+            helper.ViewData.ModelExplorer = provider
                 .GetModelExplorerForType(typeof(DefaultTemplatesUtilities.ObjectTemplateModel), model: null)
-                .GetExplorerForProperty("Property1");
+                .GetExplorerForProperty(nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1));
-
+            helper.ViewData.TemplateInfo.HtmlFieldPrefix = nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1);
-            var helper = DefaultTemplatesUtilities.GetHtmlHelper();
 
             // Act
             var labelResult = helper.Label(expression: string.Empty);
@@ -177,10 +304,79 @@ namespace Microsoft.AspNetCore.Mvc.Core
             var labelForModelResult = helper.LabelForModel();
 
             // Assert
-            Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
-            Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelNullResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelNullResult));
-            Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelForResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
-            Assert.Empty(HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForModelResult));
+        }
+
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForProperty_IfDisplayNameEmpty()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"HtmlEncode[[Property1]]\"></label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForProperty<DefaultTemplatesUtilities.ObjectTemplateModel>("Property1")
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+
+            // Act
+            var labelResult = helper.Label(expression: nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1));
+            var labelForResult = helper.LabelFor(m => m.Property1);
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+        }
+
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForProperty_IfDisplayNameEmpty_WithLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"HtmlEncode[[Property1]]\">HtmlEncode[[a label]]</label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForProperty<DefaultTemplatesUtilities.ObjectTemplateModel>("Property1")
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+
+            // Act
+            var labelResult = helper.Label(
+                expression: nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1),
+                labelText: "a label");
+            var labelForResult = helper.LabelFor(m => m.Property1, labelText: "a label");
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
+        }
+
+        // Prior to aspnet/Mvc#6638 fix, helpers generated nothing with this setup.
+        [Fact]
+        public void LabelHelpers_ReturnExpectedElementForProperty_IfDisplayNameEmpty_WithEmptyLabelText()
+        {
+            // Arrange
+            var expectedLabel = "<label for=\"HtmlEncode[[Property1]]\"></label>";
+            var provider = new TestModelMetadataProvider();
+            provider
+                .ForProperty<DefaultTemplatesUtilities.ObjectTemplateModel>("Property1")
+                .DisplayDetails(dd => dd.DisplayName = () => string.Empty);
+
+            var helper = DefaultTemplatesUtilities.GetHtmlHelper(provider: provider);
+
+            // Act
+            var labelResult = helper.Label(
+                expression: nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1),
+                labelText: string.Empty);
+            var labelForResult = helper.LabelFor(m => m.Property1, labelText: string.Empty);
+
+            // Assert
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelResult));
+            Assert.Equal(expectedLabel, HtmlContentUtilities.HtmlContentToString(labelForResult));
         }
 
         [Theory]