Support for IHtmlString and HtmlString

[CZEMacLeod]

Summary

Looking to add support for IHtmlString and HtmlString

Motivation and goals

I currently have some conditional compiles to target netcoreapp / net48 to use the aspnetcore IHtmlContent or IHtmlString as a result on an interface. I would prefer to have a consistent interface and not have to multitarget if possible.

In scope

Add System.Web.IHtmlString based on Microsoft.AspNetCore.Html.IHtmlContent with a default implementation of IHtmlContent.WriteTo
Add System.Web.HtmlString based on Microsoft.AspNetCore.Html.HtmlString and implement System.Web.IHtmlString

Risks / unknowns

One area that could be a problem (and I think is already a 'bug' in net6) is that System.Web.HttpUtilitity is part of net6.
The problem is that the code to detect IHtmlString in the public static string? HtmlEncode (object? value); method is effectively missing, and it doesn't have any reference to Microsoft.AspNetCore.Html.IHtmlContent either. Nor is there a way to replace the implementation used. This means that if you use this method, expecting the operation where it does not encode IHtmlString, you don't get what you expect.

https://github.com/dotnet/runtime/blob/04faf38fd7e8fee87b31d2698d6a566baf2ef372/src/libraries/System.Web.HttpUtility/src/System/Web/HttpUtility.cs#L145
vs
https://github.com/microsoft/referencesource/blob/5697c29004a34d80acdaf5742d7e699022c64ecd/System.Web/httpserverutility.cs#L1111

Examples

	public static string AsAttribute(this string value,
                                 string name,
                                 bool encode = true,
                                 bool includeLeadingSpace = true) => string.IsNullOrEmpty(value)
        	? string.Empty
        	: $"{(includeLeadingSpace ? " " : string.Empty)}{name}=\"{(encode ? System.Web.HttpUtility.HtmlAttributeEncode(value) : value)}\"";

	public static System.Web.IHtmlString ToTimeTag(this DateTime value, string format = "g", string css = "dateStamp") => 
		new System.Web.HtmlString($"<time{(value.ToString("O")).AsAttribute("datetime")}{css.AsAttribute("class")}>{value.ToString(format)}</time>");

This could then be used in Razor, WebForms, MVC or other ways to return a time tag for any DateTime. It would then be fine to add to an array of properties for display using HtmlEncode (except for the issue mentioned before).

Detailed design

It's often best not to fill this out until you get basic consensus about the above. When you do, consider adding an implementation proposal with the following headings:

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.Web;

/// <summary>
/// Represents an HTML-encoded string that should not be encoded again.
/// </summary>
public class HtmlString : Microsoft.AspNetCore.Html.HtmlString, IHtmlString
{
    public HtmlString(string? value) : base(value) { }

    public string ToHtmlString() => ToString();
}

/// <summary>
/// Represents an HTML-encoded string that should not be encoded again.
/// </summary>
public interface IHtmlString : IHtmlContent
{
    string ToHtmlString();
    void IHtmlContent.WriteTo(TextWriter writer, HtmlEncoder encoder) => writer.Write(ToHtmlString());
}

Unfortunately, I don't see a good way of correctly implementing HttpUtility.HtmlEncode(object?), other than to have it available via a different static object and/or as an extension method. Allowing changing HttpUtility.HtmlEncode(myobj) to be replaced with either HttpUtility2.HtmlEncode(myobj) or myobj.HtmlEncode(). Perhaps an analyser could catch this and apply code fixes?

public static class HttpUtility2
{
    [return: NotNullIfNotNull("value")]
    public static string? HtmlEncode(this object? value)
    {
        if (value is null) return null;
        if (value is IHtmlString ihtmlString) return ihtmlString.ToHtmlString();
        if (value is Microsoft.AspNetCore.Html.HtmlString htmlString) return htmlString.ToString();
        if (value is Microsoft.AspNetCore.Html.IHtmlContent htmlContent)
        {
            var writer = new StringWriter();
            htmlContent.WriteTo(writer, System.Text.Encodings.Web.HtmlEncoder.Default);
            var result = writer.ToString();
            return result;
        }
        return HtmlEncode(Convert.ToString(value, CultureInfo.CurrentCulture) ?? string.Empty);
    }

    [return: NotNullIfNotNull(nameof(s))]
    public static string? HtmlEncode(this string? s) => HttpUtility.HtmlEncode(s);

    public static void HtmlEncode(this string? s, TextWriter output) => HttpUtility.HtmlEncode(s, output);
}

[twsouthwick]

A couple of alternative approaches we could consider:

• Add IHtmlString to System.Web.HttpUtility to enable the same behavior
• Update Microsoft.AspNetCore.Html.IHtmlContent to derive from System.Web.IHtmlString by using default interface implementations

[CZEMacLeod]

@twsouthwick I like this approach, however it does affect some fairly fundamental bits of both dotnet runtime and aspnetcore - while it would probably be the best overall solution (and possibly addresses the issue with HtmlEncode too), I didn't feel it was within scope to request that level of changes - especially as it goes outside the scope of this project.

Should I raise an issue on dotnet/runtime to add IHtmlString and update HtmlEncode? And then a dependant issue on dotnet/aspnetcore?
I'm guessing something like this would not make it into the release versions for some time (net8?)

[twsouthwick]

I've created an API proposal at dotnet/runtime#83477 to see if we can get this added to System.Web.HttpUtility.dll

[joperezr]

Progress:

• Waiting on decision of [API Proposal]: Add System.Web.IHtmlString and System.Web.HtmlString to System.Web.HttpUtility  runtime#83477. Depending on if that's approved, then this issue can just be closed as external.

[twsouthwick]

Update:

• System.Web.IHtmlString was added to .NET 8 and HttpUtility.Encode was updated to account for it
• Now deciding what to do with IHtmlString in ASP.NET Core: Make Microsoft.AspNetCore.Html.HtmlString implement System.Web.IHtmlString aspnetcore#48037

[twsouthwick]

Update:

• The adapters will carry a .NET Standard version of IHtmlString and a common implementation of HtmlString
• ASP.NET Core will not (as of now) add IHtmlString to any types - if we see that causing issues, we can readdress it later

All the planned changes are in, so I'll close this issue. If new issues arise, please comment or raise a new one