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

- #6638

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

@@ -419,11 +419,6 @@ public virtual TagBuilder GenerateLabel(
                 }
             }
 
-            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 @@ protected virtual IHtmlContent GenerateLabel(
                 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 @@ public static TheoryData<object, Type, Func<object>, string, TagHelperOutputCont
                     { 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 @@ public async Task ProcessAsync_GeneratesExpectedOutput(
             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,13 +199,16 @@ public void ObjectTemplateEditsSimplePropertiesOnObjectByDefault()
             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]
         public void ObjectTemplateSkipsLabel_IfDisplayNameIsEmpty()
         {
             // 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>" +