Return string from HtmlHelper properties - Id, Name, DisplayName and Value

[Kukkimonsuta]

I'd like to reopen issue on codeplex "DisplayName / DisplayText helpers should return string" from MVC 5. It was closed because it's breaking change and it was reported after the release. There is workaround specified involving rewriting TagBuilder, however that only solves one issue (see below). Now that new major version is on the horizont I believe it's time to resolve it.

Problem

Methods on HtmlHelper Id, Name, DisplayName and Value encode it's return value and return HtmlString

Detailed description

Code for reference: HtmlHelper.cs:334; HtmlHelper.cs:711; HtmlHelper.cs:841

The line I have problem with is return new HtmlString(Encode(fullName));. The pre-encoding is in my opinion unnecessary as the HtmlString instance will never contain html that is supposed to stay as html. Methods on HtmlHelper Id, Name, DisplayName and Value should return unencoded string as it's job for templating engine to take care of encoding these values.

In the original issue there was also DisplayText, however looking at the current source code it doesn't encode the data and interprets it as html in which case it's necessary to use HtmlString.

Proposed fix

Change methods on HtmlHelper Id, Name, DisplayName and Value to return string instead and let template engine / developer to take care of encoding them.

Use cases

Case 1 - no way to output raw value

<script>
    (function ($) {
        var title = "@Html.Raw(Html.DisplayNameFor(m => m.Foo))";
        alert(title);
    } (jQuery));
</script>

I think we all agree that we should stay away from inline javascript, however in the real world sometimes there is just no way around. Also there should be some kind of encoding going on anyway so that the script won't break when name contains ", but that's not really the point of this demo - the point is that we have no way of writing the raw value of these fields unless we decode them first.

Problem: developer can't use these apis to output raw value (unless we decode it first)

Workaround: add overload to HtmlHelper.Raw accepting IHtmlString which decodes the value.

Case 2 - TagBuilder will re-encode attributes

@Html.TextBoxFor(m => m.Bar, new { placeholder = Html.DisplayNameFor(m => m.Bar) })

Problem: placeholder will be encoded twice (unless we decode it first)

Workaround: allow TagBuilder to accept IHtmlString for attributes and not re-encode them.

Case 3 - you can't modify the raw string without decoding it first

public static HtmlString LabeledRadio<TModel, TProperty>(this HtmlHelper<TModel> helper, 
    Expression<Func<TModel, TProperty>> expression, string value, string title)
{
    var id = helper.IdFor(expression) + "$" + value;
    var name = helper.NameFor(expression).ToString();

    // todo: retrieve modelstate and select the current value

    var input = new TagBuilder("input");
    input.MergeAttribute("type", "radio");
    input.MergeAttribute("id", id);
    input.MergeAttribute("name", name);
    input.MergeAttribute("value", value);

    var label = new TagBuilder("label");
    label.MergeAttribute("for", id);
    label.SetInnerText(title);

    return new HtmlString(input.ToString(TagRenderMode.SelfClosing) + label.ToString());
}

Problem: part of id returned by IdFor will be encoded twice, name will be encoded twice (unless we decode them first)

Workaround: use different API, however for new users this won't be obvious and why should one reinvent the wheel when HtmlHelper is accessible. This applies especially to the Value helper.

[dougbu]

@Kukkimonsuta thank-you for your input. We appreciate your interest in MVC 6.0 and agree this is an issue we should address. We hope to fix the issue soon and make these helpers more generally usable.

[yishaigalatzer]

@dougbu please review with @GrabYourPitchforks when you are ready to PR

[dougbu]

👌 Have already had a conversation or two w/ him. No concerns so far...

[dougbu]

Fixed in commit 7845a8f along w/ a few other IHtmlHelper API issues.