ASP.NET MVC Html.RadioButton Generates Invalid XHTML

It was pointed out to me by a colleague that the Html.RadioButton(...) extension methods produce invalid XHTML. The output is invalid because, by default, the method will duplicate html "id" attributes. For example, let's say I have a Colours View Model...

public enum Colours
{
    Red,
    Blue,
    Green
}

public class ColoursViewModel
{
    public Colours ChosenColour
    {
        get;
        set;
    }
}

Then I could create a simple action like this ...

public class TestController
{
    public ActionResult SelectColour()
    {
        return View(new ColoursViewModel());
    }
}

... that rendered the following view ...

<%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<ColoursViewModel>" %>

<%= Html.RadioButton("ChosenColour", "Red"); %>
<%= Html.RadioButton("ChosenColour", "Green"); %>
<%= Html.RadioButton("ChosenColour", "Blue"); %>

The MVC framework will automatically bind my view model's "ChosenColour" property to the radio button collection so that I can post the results back like this...

public class TestController
{
    [AcceptVerbs(HttpVerbs.Post)]
    public string SelectColour(ColoursViewModel viewModel)
    {
        return "You chose " + viewModel.ChosenColour.ToString();
    }
}

The problem here is that the generated html looks like this (notice the duplicate id attributes)...

This means that we cannot use the "for" attribute on any labels associated with the individual radio buttons nor can we use javascript to select the radio button when that associated label is clicked.

This is how you fix the problem...

<%@ Control Language="C#" Inherits="System.Web.Mvc.ViewUserControl<ColoursViewModel>" %>

<%= Html.RadioButton("ChosenColour", "Red", new { id = "chooseRed" }); %>
<%= Html.RadioButton("ChosenColour", "Green", new { id = "chooseGreen" }); %>
<%= Html.RadioButton("ChosenColour", "Blue", new { id = "chooseBlue" }); %>

... which will generate this html fragment...

The point I'm trying to make here is that the id attribute should be a first class parameter of the RadioButton extension method. I shouldn't have to override it using the htmlAttributes parameter. This is what the official extension methods look like...

public static string RadioButton(this HtmlHelper htmlHelper, string name, object value);
public static string RadioButton(this HtmlHelper htmlHelper, string name, object value, object htmlAttributes);
public static string RadioButton(this HtmlHelper htmlHelper, string name, object value, IDictionary<string, object> htmlAttributes);
public static string RadioButton(this HtmlHelper htmlHelper, string name, object value, bool isChecked);
public static string RadioButton(this HtmlHelper htmlHelper, string name, object value, bool isChecked, object htmlAttributes);
public static string RadioButton(this HtmlHelper htmlHelper, string name, object value, bool isChecked, IDictionary<string, object> htmlAttributes);

You could consider making a change to the MVC framework libraries to include the id, thus decoupling it from the name. For example..

public static string RadioButton(this HtmlHelper htmlHelper, string name, string id, object value);

So anyway, the take-away from this blog post is "Html.RadioButton(...) will produce invalid XHTML unless you pay attention."

Question: When is a non-static class static?

Answer: When it's a Provider.

OK, quite an obtuse question there so let me explain...

In our ASP.NET MVC application we have a custom Membership Provider and a custom Role Provider. Here's a (vastly simplified) example of the membership provider...

public class CustomMembershipProvider : MembershipProvider
{
    private SqlConnection _connection;
    private string _userName;
    private string _password;

    public CustomMembershipProvider()
    {
    }

    public override bool ValidateUser(string username, string password)
    {
        _userName = username;
        _password = password;

        bool success = false;

        try
        {
            // Sets the _cn member...
            OpenConnection();

            success = ValidateUser();
        }
        finally
        {
            CloseConnection();
        }

        return success;
    }
}

So, pretty straight forward. Nothing wrong with that, is there?

Well, actually, yes. The problem is that you may write this class with the expectation that it will be treated as a  POIO (Plain Old Instance Object) but ASP.NET has other ideas. One, and only one, of these objects is instantiated (during the first request to you site). The big issue here is that we now have private fields on a singleton class - not good.

We ran into problems with SqlDataReaders because the runtime complained about multiple readers on the same connection. We could have implemented MARS, but that still didn't deal with the underlying problem. Worse still is the possibility of an unauthorised user actually being authorised! You only need user1's time-slice to end and user2's to begin midway through that ValidateUser method and you've got serious problems.

This is a summarised suggestion of how we solved this problem...

public class CustomMembershipProvider : MembershipProvider
{
    public CustomMembershipProvider()
    {
    }

    public override bool ValidateUser(string username, string password)
    {
        bool success = false;
        
        using (var cn = OpenConnection())
        {
            var validater = new UserValidater(cn, username, password);

            success = validater.IsValid();
        }

        return success;
    }

    private class UserValidater
    {
        private SqlConnection _connection;
        private string _userName;
        private string _password;

        public UserValidater(SqlConnection cn, string userName, string password)
        {
            _connection = connection;
            _userName = username;
            _password = password;
        }

        public bool IsValid()
        {
            // Logic removed for brevity...
        }
    }
}

Automatically Redirect Http Requests to Https on IIS 7

Our main website application used to have a structure that would allow the user to land on an http page and then login to a secure area. This has now changed so that the entire site has to be under SSL. The result of this is that we can no longer 'code' for this redirect. JPPinto has written a great blog entry that describes how to do it outside of your application, just using IIS.

http://www.jppinto.com/2009/04/automatically-redirect-http-requests-to-https-iis-7/