diff --git a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ExpressionMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ExpressionMetadataProvider.cs index eeb93fe473..ca70ed5cb9 100644 --- a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ExpressionMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ExpressionMetadataProvider.cs @@ -71,7 +71,7 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions ModelMetadata metadata; if (propertyName == null) { - // Ex: + // Ex: // m => 5 (arbitrary expression) // m => foo (arbitrary expression) // m => m.Widgets[0] (expression ending with non-property-access) @@ -79,7 +79,7 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions } else { - // Ex: + // Ex: // m => m.Color (simple property access) // m => m.Color.Red (nested property access) // m => m.Widgets[0].Size (expression ending with property-access) @@ -89,22 +89,27 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions return viewData.ModelExplorer.GetExplorerForExpression(metadata, modelAccessor); } + /// + /// Gets for named in given + /// . + /// + /// Expression name, relative to viewData.Model. + /// + /// The that may contain the value. + /// + /// The . + /// + /// for named in given . + /// public static ModelExplorer FromStringExpression( string expression, [NotNull] ViewDataDictionary viewData, IModelMetadataProvider metadataProvider) { - if (string.IsNullOrEmpty(expression)) - { - // Empty string really means "ModelMetadata for the current model". - return FromModel(viewData, metadataProvider); - } - var viewDataInfo = ViewDataEvaluator.Eval(viewData, expression); - if (viewDataInfo == null) { - // Try getting a property from ModelMetadata if we couldn't find an answer in ViewData + // Try getting a property from ModelMetadata if we couldn't find an answer in ViewData var propertyExplorer = viewData.ModelExplorer.GetExplorerForProperty(expression); if (propertyExplorer != null) { @@ -114,11 +119,20 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions if (viewDataInfo != null) { + if (viewDataInfo.Container == viewData && + viewDataInfo.Value == viewData.Model && + string.IsNullOrEmpty(expression)) + { + // Nothing for empty expression in ViewData and ViewDataEvaluator just returned the model. Handle + // using FromModel() for its object special case. + return FromModel(viewData, metadataProvider); + } + ModelExplorer containerExplorer = viewData.ModelExplorer; if (viewDataInfo.Container != null) { containerExplorer = metadataProvider.GetModelExplorerForType( - viewDataInfo.Container.GetType(), + viewDataInfo.Container.GetType(), viewDataInfo.Container); } diff --git a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ViewDataEvaluator.cs b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ViewDataEvaluator.cs index fc5964d396..4f123cc7bf 100644 --- a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ViewDataEvaluator.cs +++ b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Expressions/ViewDataEvaluator.cs @@ -1,24 +1,34 @@ // 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 System; using System.Collections.Generic; using System.Reflection; -using Microsoft.AspNet.Mvc.Extensions; using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.Rendering.Expressions { public static class ViewDataEvaluator { + /// + /// Gets for named in given + /// . + /// + /// + /// The that may contain the value. + /// + /// Expression name, relative to viewData.Model. + /// + /// for named in given . + /// public static ViewDataInfo Eval([NotNull] ViewDataDictionary viewData, string expression) { - if (string.IsNullOrEmpty(expression)) - { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); - } + // While it is not valid to generate a field for the top-level model itself because the result is an + // unnamed input element, do not throw here if full name is null or empty. Support is needed for cases + // such as Html.Label() and Html.Value(), where the user's code is not creating a name attribute. Checks + // are in place at higher levels for the invalid cases. + var fullName = viewData.TemplateInfo.GetFullHtmlFieldName(expression); - // Given an expression "one.two.three.four" we look up the following (pseudocode): + // Given an expression "one.two.three.four" we look up the following (pseudo-code): // this["one.two.three.four"] // this["one.two.three"]["four"] // this["one.two"]["three.four] @@ -28,21 +38,60 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions // this["one"]["two"]["three.four"] // this["one"]["two"]["three"]["four"] - return EvalComplexExpression(viewData, expression); - } - - public static ViewDataInfo Eval(object indexableObject, string expression) - { - if (string.IsNullOrEmpty(expression)) + // Try to find a matching ViewData entry using the full expression name. If that fails, fall back to + // ViewData.Model using the expression's relative name. + var result = EvalComplexExpression(viewData, fullName); + if (result == null) { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); + if (string.IsNullOrEmpty(expression)) + { + // Null or empty expression name means current model even if that model is null. + result = new ViewDataInfo(container: viewData, value: viewData.Model); + } + else + { + result = EvalComplexExpression(viewData.Model, expression); + } } - // Run through same cases as other Eval() overload but allow a null container. - return (indexableObject == null) ? null : EvalComplexExpression(indexableObject, expression); + return result; + } + + /// + /// Gets for named in given + /// . + /// + /// + /// The that may contain the value. + /// + /// Expression name, relative to . + /// + /// for named in given + /// . + /// + public static ViewDataInfo Eval(object indexableObject, string expression) + { + // Run through many of the same cases as other Eval() overload. + return EvalComplexExpression(indexableObject, expression); } private static ViewDataInfo EvalComplexExpression(object indexableObject, string expression) + { + if (indexableObject == null) + { + return null; + } + + if (expression == null) + { + // In case a Dictionary indexableObject contains a "" entry, don't short-circuit the logic below. + expression = string.Empty; + } + + return InnerEvalComplexExpression(indexableObject, expression); + } + + private static ViewDataInfo InnerEvalComplexExpression(object indexableObject, string expression) { foreach (var expressionPair in GetRightToLeftExpressions(expression)) { @@ -59,7 +108,7 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions if (subTargetInfo.Value != null) { - var potential = EvalComplexExpression(subTargetInfo.Value, postExpression); + var potential = InnerEvalComplexExpression(subTargetInfo.Value, postExpression); if (potential != null) { return potential; @@ -71,12 +120,15 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions return null; } + // Produces an enumeration of combinations of property names given a complex expression in the following order: + // this["one.two.three.four"] + // this["one.two.three][four"] + // this["one.two][three.four"] + // this["one][two.three.four"] + // Recursion of InnerEvalComplexExpression() further sub-divides these cases to cover the full set of + // combinations shown in Eval(ViewDataDictionary, string) comments. private static IEnumerable GetRightToLeftExpressions(string expression) { - // Produces an enumeration of all the combinations of complex property names - // given a complex expression. See the list above for an example of the result - // of the enumeration. - yield return new ExpressionPair(expression, string.Empty); var lastDot = expression.LastIndexOf('.'); @@ -122,34 +174,24 @@ namespace Microsoft.AspNet.Mvc.Rendering.Expressions return null; } + // This method handles one "segment" of a complex property expression private static ViewDataInfo GetPropertyValue(object container, string propertyName) { - // This method handles one "segment" of a complex property expression - - // First, we try to evaluate the property based on its indexer + // First, try to evaluate the property based on its indexer. var value = GetIndexedPropertyValue(container, propertyName); if (value != null) { return value; } - // If the indexer didn't return anything useful, continue... - - // If the container is a ViewDataDictionary then treat its Model property - // as the container instead of the ViewDataDictionary itself. - var viewData = container as ViewDataDictionary; - if (viewData != null) - { - container = viewData.Model; - } - - // If the container is null, we're out of options - if (container == null) + // Do not attempt to find a property with an empty name and or of a ViewDataDictionary. + if (string.IsNullOrEmpty(propertyName) || container is ViewDataDictionary) { return null; } - // Finally try to use PropertyInfo and treat the expression as a property name + // If the indexer didn't return anything useful, try to use PropertyInfo and treat the expression + // as a property name. var propertyInfo = container.GetType().GetRuntimeProperty(propertyName); if (propertyInfo == null) { diff --git a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Html/DefaultHtmlGenerator.cs b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Html/DefaultHtmlGenerator.cs index bebf163fdc..aad2d90ddb 100644 --- a/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Html/DefaultHtmlGenerator.cs +++ b/src/Microsoft.AspNet.Mvc.Extensions/Rendering/Html/DefaultHtmlGenerator.cs @@ -322,11 +322,10 @@ namespace Microsoft.AspNet.Mvc.Rendering // isChecked not provided nor found in the given attributes; fall back to view data. var valueString = Convert.ToString(value, CultureInfo.CurrentCulture); - isChecked = !string.IsNullOrEmpty(expression) && - string.Equals( - EvalString(viewContext, expression), - valueString, - StringComparison.OrdinalIgnoreCase); + isChecked = string.Equals( + EvalString(viewContext, expression), + valueString, + StringComparison.OrdinalIgnoreCase); } } else @@ -419,14 +418,6 @@ namespace Microsoft.AspNet.Mvc.Rendering // If we got a null selectList, try to use ViewData to get the list of items. if (selectList == null) { - if (string.IsNullOrEmpty(expression)) - { - // Do not call ViewData.Eval(); that would return ViewData.Model, which is not correct here. - // Note this case has a simple workaround: users must pass a non-null selectList to use - // DropDownList() or ListBox() in a template, where a null or empty name has meaning. - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(expression)); - } - selectList = GetSelectListItems(viewContext, expression); } @@ -1012,7 +1003,7 @@ namespace Microsoft.AspNet.Mvc.Rendering if (!usedModelState && useViewData) { - isChecked = EvalBoolean(viewContext, fullName); + isChecked = EvalBoolean(viewContext, expression); } if (isChecked) @@ -1036,7 +1027,7 @@ namespace Microsoft.AspNet.Mvc.Rendering var attributeValue = (string)GetModelStateValue(viewContext, fullName, typeof(string)); if (attributeValue == null) { - attributeValue = useViewData ? EvalString(viewContext, fullName, format) : valueParameter; + attributeValue = useViewData ? EvalString(viewContext, expression, format) : valueParameter; } var addValue = true; @@ -1193,14 +1184,20 @@ namespace Microsoft.AspNet.Mvc.Rendering [NotNull] ViewContext viewContext, string expression) { + // Method is called only if user did not pass a select list in. They must provide select list items in the + // ViewData dictionary and definitely not as the Model. (Even if the Model datatype were correct, a + // +
- +EmployeeName_0
@@ -20,9 +18,7 @@ Name value that should not be seen.
- - - + @@ -40,15 +36,13 @@ Name value that should not be seen.
- - +
- +EmployeeName_1
@@ -57,9 +51,7 @@ Name value that should not be seen.
- - - + @@ -77,15 +69,13 @@ Name value that should not be seen.
- - +
- +EmployeeName_2
@@ -94,9 +84,7 @@ Name value that should not be seen.
- - - + diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html index b561698901..f6f761b888 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.Encoded.html @@ -69,8 +69,7 @@
Male - - Female + Female
  • diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html index db67181b13..3063e71bac 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.OrderUsingHtmlHelpers.html @@ -69,8 +69,7 @@
    Male - - Female + Female
    • diff --git a/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_HomeController.cs b/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_HomeController.cs index d753cb0443..7ce069c48e 100644 --- a/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_HomeController.cs +++ b/test/WebSites/HtmlGenerationWebSite/Controllers/HtmlGeneration_HomeController.cs @@ -124,8 +124,7 @@ namespace HtmlGenerationWebSite.Controllers }, }; - // Extra data that should be ignored within a template. But #1487 currently affects RadioButton and - // TextArea as well as ModelMetadata for
    - @* Due to #1487, text area will contain incorrect "Value that should not be seen." *@ @Html.TextArea(nameof(Model.Name))
    @@ -22,9 +20,7 @@ var genders = new SelectList(new string[] { "Male", "Female" }); } - @* Due to #1487, radio button will not be checked. Employee is Female but incorrect information does not match. *@ @Html.RadioButton(nameof(Model.Gender), "Female", htmlAttributes: new { disabled = "disabled", @readonly = "readonly" }) - @* Due to #1487,
    diff --git a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/EditorTemplates/GenderUsingHtmlHelpers.cshtml b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/EditorTemplates/GenderUsingHtmlHelpers.cshtml index 2fc2339bc8..ad7b40a779 100644 --- a/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/EditorTemplates/GenderUsingHtmlHelpers.cshtml +++ b/test/WebSites/HtmlGenerationWebSite/Views/HtmlGeneration_Home/EditorTemplates/GenderUsingHtmlHelpers.cshtml @@ -1,5 +1,4 @@ @using HtmlGenerationWebSite.Models @model Gender @Html.RadioButtonFor(m => m, value: "Male") Male -@* Due to #2662 radio button will not be checked because help ignores this expression." *@ @Html.RadioButton(expression: string.Empty, value: "Female") Female \ No newline at end of file