From 3db0a803062df19e83bdd62b63eba47c016af384 Mon Sep 17 00:00:00 2001 From: dougbu Date: Sun, 3 Aug 2014 22:37:42 -0700 Subject: [PATCH] Remove HTML helpers with `IDictionary` parameters - see line 2 of #874 - focus on `TextBox[For]()` and `ValidationSummary()` related fixes included here: - `TextArea[For]()` documentation incorrectly indicated their `htmlAttributes` parameters were dictionaries - handle `htmlAttributes` parameters more consistently; create a dictionary only when necessary --- .../Rendering/Html/HtmlHelper.cs | 102 +++++++----------- .../Rendering/Html/HtmlHelperOfT.cs | 2 +- .../Rendering/HtmlHelperInputExtensions.cs | 21 +--- .../HtmlHelperValidationExtensions.cs | 43 ++------ .../Rendering/IHtmlHelper.cs | 16 +-- .../Rendering/IHtmlHelperOfT.cs | 10 +- 6 files changed, 62 insertions(+), 132 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs index 74e41c16ba..9a7d63143e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelper.cs @@ -115,7 +115,7 @@ namespace Microsoft.AspNet.Mvc.Rendering object htmlAttributes) { var url = _urlHelper.Action(actionName, controllerName, routeValues); - return GenerateLink(linkText, url, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); + return GenerateLink(linkText, url, GetHtmlAttributeDictionaryOrNull(htmlAttributes)); } /// @@ -186,13 +186,7 @@ namespace Microsoft.AspNet.Mvc.Rendering public MvcForm BeginForm(string actionName, string controllerName, object routeValues, FormMethod method, object htmlAttributes) { - // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. - IDictionary htmlAttributeDictionary = null; - if (htmlAttributes != null) - { - htmlAttributeDictionary = HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes); - } - + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); return GenerateForm(actionName, controllerName, routeValues, method, htmlAttributeDictionary); } @@ -427,7 +421,7 @@ namespace Microsoft.AspNet.Mvc.Rendering object htmlAttributes) { var url = _urlHelper.RouteUrl(routeName, routeValues, protocol, hostName, fragment); - return GenerateLink(linkText, url, HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); + return GenerateLink(linkText, url, GetHtmlAttributeDictionaryOrNull(htmlAttributes)); } /// @@ -437,12 +431,17 @@ namespace Microsoft.AspNet.Mvc.Rendering } /// - public HtmlString ValidationSummary(bool excludePropertyErrors, + public HtmlString ValidationSummary( + bool excludePropertyErrors, string message, - IDictionary htmlAttributes, + object htmlAttributes, string tag) { - return GenerateValidationSummary(excludePropertyErrors, message, htmlAttributes, tag); + return GenerateValidationSummary( + excludePropertyErrors, + message, + GetHtmlAttributeDictionaryOrNull(htmlAttributes), + tag); } /// @@ -476,7 +475,7 @@ namespace Microsoft.AspNet.Mvc.Rendering } /// - public HtmlString TextBox(string name, object value, string format, IDictionary htmlAttributes) + public HtmlString TextBox(string name, object value, string format, object htmlAttributes) { return GenerateTextBox(metadata: null, name: name, value: value, format: format, htmlAttributes: htmlAttributes); @@ -563,17 +562,7 @@ namespace Microsoft.AspNet.Mvc.Rendering } } - // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. - IDictionary htmlAttributeDictionary = null; - if (htmlAttributes != null) - { - htmlAttributeDictionary = htmlAttributes as IDictionary; - if (htmlAttributeDictionary == null) - { - htmlAttributeDictionary = AnonymousObjectToHtmlAttributes(htmlAttributes); - } - } - + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); var explicitValue = isChecked.HasValue; if (explicitValue && htmlAttributeDictionary != null) { @@ -691,17 +680,6 @@ namespace Microsoft.AspNet.Mvc.Rendering bool useViewData, object htmlAttributes) { - // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. - IDictionary htmlAttributeDictionary = null; - if (htmlAttributes != null) - { - htmlAttributeDictionary = htmlAttributes as IDictionary; - if (htmlAttributeDictionary == null) - { - htmlAttributeDictionary = AnonymousObjectToHtmlAttributes(htmlAttributes); - } - } - // Special-case opaque values and arbitrary binary data. var byteArrayValue = value as byte[]; if (byteArrayValue != null) @@ -709,6 +687,7 @@ namespace Microsoft.AspNet.Mvc.Rendering value = Convert.ToBase64String(byteArrayValue); } + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); return GenerateInput(InputType.Hidden, metadata, name, @@ -750,7 +729,7 @@ namespace Microsoft.AspNet.Mvc.Rendering ViewData.TemplateInfo.GetFullHtmlFieldName(htmlFieldName), IdAttributeDotReplacement)); tag.SetInnerText(resolvedLabelText); - tag.MergeAttributes(AnonymousObjectToHtmlAttributes(htmlAttributes), replaceExisting: true); + tag.MergeAttributes(GetHtmlAttributeDictionaryOrNull(htmlAttributes), replaceExisting: true); return tag.ToHtmlString(TagRenderMode.Normal); } @@ -794,17 +773,7 @@ namespace Microsoft.AspNet.Mvc.Rendering protected virtual HtmlString GeneratePassword(ModelMetadata metadata, string name, object value, object htmlAttributes) { - // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. - IDictionary htmlAttributeDictionary = null; - if (htmlAttributes != null) - { - htmlAttributeDictionary = htmlAttributes as IDictionary; - if (htmlAttributeDictionary == null) - { - htmlAttributeDictionary = AnonymousObjectToHtmlAttributes(htmlAttributes); - } - } - + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); return GenerateInput(InputType.Password, metadata, name, @@ -820,17 +789,7 @@ namespace Microsoft.AspNet.Mvc.Rendering protected virtual HtmlString GenerateRadioButton(ModelMetadata metadata, string name, object value, bool? isChecked, object htmlAttributes) { - // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. - IDictionary htmlAttributeDictionary = null; - if (htmlAttributes != null) - { - htmlAttributeDictionary = htmlAttributes as IDictionary; - if (htmlAttributeDictionary == null) - { - htmlAttributeDictionary = AnonymousObjectToHtmlAttributes(htmlAttributes); - } - } - + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); if (metadata == null) { // RadioButton() case. Do not override checked attribute if isChecked is implicit. @@ -941,7 +900,7 @@ namespace Microsoft.AspNet.Mvc.Rendering { InnerHtml = listItemBuilder.ToString() }; - tagBuilder.MergeAttributes(AnonymousObjectToHtmlAttributes(htmlAttributes)); + tagBuilder.MergeAttributes(GetHtmlAttributeDictionaryOrNull(htmlAttributes)); tagBuilder.MergeAttribute("name", fullName, true /* replaceExisting */); tagBuilder.GenerateId(fullName, IdAttributeDotReplacement); if (allowMultiple) @@ -998,7 +957,7 @@ namespace Microsoft.AspNet.Mvc.Rendering var tagBuilder = new TagBuilder("textarea"); tagBuilder.GenerateId(fullName, IdAttributeDotReplacement); - tagBuilder.MergeAttributes(AnonymousObjectToHtmlAttributes(htmlAttributes), true); + tagBuilder.MergeAttributes(GetHtmlAttributeDictionaryOrNull(htmlAttributes), true); if (rows > 0) { tagBuilder.MergeAttribute("rows", rows.ToString(CultureInfo.InvariantCulture), true); @@ -1026,8 +985,9 @@ namespace Microsoft.AspNet.Mvc.Rendering } protected virtual HtmlString GenerateTextBox(ModelMetadata metadata, string name, object value, string format, - IDictionary htmlAttributes) + object htmlAttributes) { + var htmlAttributeDictionary = GetHtmlAttributeDictionaryOrNull(htmlAttributes); return GenerateInput(InputType.Text, metadata, name, @@ -1037,7 +997,7 @@ namespace Microsoft.AspNet.Mvc.Rendering setId: true, isExplicitValue: true, format: format, - htmlAttributes: htmlAttributes); + htmlAttributes: htmlAttributeDictionary); } protected virtual HtmlString GenerateInput(InputType inputType, ModelMetadata metadata, string name, @@ -1189,7 +1149,7 @@ namespace Microsoft.AspNet.Mvc.Rendering tag = ViewContext.ValidationMessageElement; } var builder = new TagBuilder(tag); - builder.MergeAttributes(AnonymousObjectToHtmlAttributes(htmlAttributes)); + builder.MergeAttributes(GetHtmlAttributeDictionaryOrNull(htmlAttributes)); // Only the style of the span is changed according to the errors if message is null or empty. // Otherwise the content and style is handled by the client-side validation. @@ -1349,6 +1309,22 @@ namespace Microsoft.AspNet.Mvc.Rendering new ClientModelValidationContext(metadata, MetadataProvider))); } + // Only need a dictionary if htmlAttributes is non-null. TagBuilder.MergeAttributes() is fine with null. + private static IDictionary GetHtmlAttributeDictionaryOrNull(object htmlAttributes) + { + IDictionary htmlAttributeDictionary = null; + if (htmlAttributes != null) + { + htmlAttributeDictionary = htmlAttributes as IDictionary; + if (htmlAttributeDictionary == null) + { + htmlAttributeDictionary = AnonymousObjectToHtmlAttributes(htmlAttributes); + } + } + + return htmlAttributeDictionary; + } + private static string GetInputTypeString(InputType inputType) { switch (inputType) diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelperOfT.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelperOfT.cs index 195ff48697..814f2cc879 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelperOfT.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/Html/HtmlHelperOfT.cs @@ -206,7 +206,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// public HtmlString TextBoxFor([NotNull] Expression> expression, - string format, IDictionary htmlAttributes) + string format, object htmlAttributes) { var metadata = GetModelMetadata(expression); return GenerateTextBox(metadata, GetExpressionName(expression), metadata.Model, format, htmlAttributes); diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperInputExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperInputExtensions.cs index c5880c8510..6ec1e45a03 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperInputExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperInputExtensions.cs @@ -144,17 +144,7 @@ namespace Microsoft.AspNet.Mvc.Rendering string format, object htmlAttributes) { - return htmlHelper.TextBox(name, value, format, - HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); - } - - public static HtmlString TextBox( - [NotNull] this IHtmlHelper htmlHelper, - string name, - object value, - IDictionary htmlAttributes) - { - return htmlHelper.TextBox(name, value, format: null, htmlAttributes: htmlAttributes); + return htmlHelper.TextBox(name, value, format, htmlAttributes); } public static HtmlString TextBoxFor([NotNull] this IHtmlHelper htmlHelper, @@ -178,14 +168,7 @@ namespace Microsoft.AspNet.Mvc.Rendering public static HtmlString TextBoxFor([NotNull] this IHtmlHelper htmlHelper, [NotNull] Expression> expression, string format, object htmlAttributes) { - return htmlHelper.TextBoxFor(expression, format: format, - htmlAttributes: HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes)); - } - - public static HtmlString TextBoxFor([NotNull] this IHtmlHelper htmlHelper, - [NotNull] Expression> expression, IDictionary htmlAttributes) - { - return htmlHelper.TextBoxFor(expression, format: null, htmlAttributes: htmlAttributes); + return htmlHelper.TextBoxFor(expression, format: format, htmlAttributes: htmlAttributes); } public static HtmlString TextArea([NotNull] this IHtmlHelper htmlHelper, diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperValidationExtensions.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperValidationExtensions.cs index 0f751bfbc1..a9da75da06 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperValidationExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/HtmlHelperValidationExtensions.cs @@ -138,10 +138,8 @@ namespace Microsoft.AspNet.Mvc.Rendering string message, object htmlAttributes) { - return htmlHelper.ValidationSummary(excludePropertyErrors: false, - message: message, - htmlAttributes: HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), - tag: null); + return htmlHelper.ValidationSummary( + excludePropertyErrors: false, message: message, htmlAttributes: htmlAttributes, tag: null); } public static HtmlString ValidationSummary([NotNull] this IHtmlHelper htmlHelper, @@ -149,10 +147,8 @@ namespace Microsoft.AspNet.Mvc.Rendering object htmlAttributes, string tag) { - return htmlHelper.ValidationSummary(excludePropertyErrors: false, - message: message, - htmlAttributes: HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), - tag: tag); + return htmlHelper.ValidationSummary( + excludePropertyErrors: false, message: message, htmlAttributes: htmlAttributes, tag: tag); } public static HtmlString ValidationSummary([NotNull] this IHtmlHelper htmlHelper, @@ -171,10 +167,7 @@ namespace Microsoft.AspNet.Mvc.Rendering string message, object htmlAttributes) { - return htmlHelper.ValidationSummary(excludePropertyErrors, - message, - HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), - tag: null); + return htmlHelper.ValidationSummary(excludePropertyErrors, message, htmlAttributes, tag: null); } public static HtmlString ValidationSummary([NotNull] this IHtmlHelper htmlHelper, @@ -183,31 +176,7 @@ namespace Microsoft.AspNet.Mvc.Rendering object htmlAttributes, string tag) { - return htmlHelper.ValidationSummary(excludePropertyErrors, - message, - HtmlHelper.AnonymousObjectToHtmlAttributes(htmlAttributes), - tag); - } - - public static HtmlString ValidationSummary([NotNull] this IHtmlHelper htmlHelper, - string message, - IDictionary htmlAttributes) - { - return htmlHelper.ValidationSummary(excludePropertyErrors: false, - message: message, - htmlAttributes: htmlAttributes, - tag: null); - } - - public static HtmlString ValidationSummary([NotNull] this IHtmlHelper htmlHelper, - string message, - IDictionary htmlAttributes, - string tag) - { - return htmlHelper.ValidationSummary(excludePropertyErrors: false, - message: message, - htmlAttributes: htmlAttributes, - tag: tag); + return htmlHelper.ValidationSummary(excludePropertyErrors, message, htmlAttributes, tag); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelper.cs index 8c46d4c83b..6b23dfddb3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelper.cs @@ -414,8 +414,8 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// Number of rows in the textarea. /// Number of columns in the textarea. - /// - /// containing additional HTML attributes. + /// An object that contains the HTML attributes to set for the element. + /// Alternatively, an instance containing the HTML attributes. /// /// New containing the rendered HTML. HtmlString TextArea(string name, string value, int rows, int columns, object htmlAttributes); @@ -431,11 +431,11 @@ namespace Microsoft.AspNet.Mvc.Rendering /// If non-null, value to include in the element. Ignore if named value is found in submitted data. /// /// - /// - /// containing additional HTML attributes. + /// An object that contains the HTML attributes to set for the element. + /// Alternatively, an instance containing the HTML attributes. /// /// New containing the rendered HTML. - HtmlString TextBox(string name, object value, string format, IDictionary htmlAttributes); + HtmlString TextBox(string name, object value, string format, object htmlAttributes); /// /// Returns the validation message if an error exists in the object. @@ -458,7 +458,9 @@ namespace Microsoft.AspNet.Mvc.Rendering /// true to have the summary display model-level errors only, or false to /// have the summary display all errors. /// The message to display with the validation summary. - /// A dictionary that contains the HTML attributes for the element. + /// An object that contains the HTML attributes to set for the element. + /// Alternatively, an instance containing the HTML attributes. + /// /// The tag to wrap the in the generated HTML. /// Its default value is . /// An that contains an unordered list (ul element) of validation messages. @@ -466,7 +468,7 @@ namespace Microsoft.AspNet.Mvc.Rendering HtmlString ValidationSummary( bool excludePropertyErrors, string message, - IDictionary htmlAttributes, + object htmlAttributes, string tag); /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelperOfT.cs b/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelperOfT.cs index 081d371c8e..e42def7bb4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelperOfT.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Rendering/IHtmlHelperOfT.cs @@ -223,8 +223,8 @@ namespace Microsoft.AspNet.Mvc.Rendering /// An expression, relative to the current model. /// Number of rows in the textarea. /// Number of columns in the textarea. - /// - /// containing additional HTML attributes. + /// An object that contains the HTML attributes to set for the element. + /// Alternatively, an instance containing the HTML attributes. /// /// New containing the rendered HTML. HtmlString TextAreaFor([NotNull] Expression> expression, @@ -237,12 +237,12 @@ namespace Microsoft.AspNet.Mvc.Rendering /// An expression that identifies the object that contains the properties to render. /// /// - /// - /// containing additional HTML attributes. + /// An object that contains the HTML attributes to set for the element. + /// Alternatively, an instance containing the HTML attributes. /// /// New containing the rendered HTML. HtmlString TextBoxFor([NotNull] Expression> expression, string format, - IDictionary htmlAttributes); + object htmlAttributes); /// /// Returns the validation message for the specified expression