locked
DropDownListFor modifying passed SelectListItems RRS feed

  • Question

  • User-661793839 posted

    Whoops, tried to edit this post on my phone and it lost all formatting!

    I've come across what I believe to be a bug in DropDownListFor and would like to get some community feedback.

    Let's say I have a view model with a an IEnumerable<SelectListItem> called Users and would like to use it as the source for multiple dropdown lists:

    @Html.DropDownListFor(x => x.FirstUser, Model.Users)
    @Html.DropDownListFor(x => x.SecondUser, Model.Users)

    If, on initial page rendering, the view model property FirstUser is set to "Jason" and the property SecondUser is set to null, I would expect two drop down lists to be rendered, one with "Jason" preselected and one with either a blank entry or just the first item in the list preselected.

    Instead what I get is two drop down lists, both with "Jason" preselected. It seems that the html helper is actually modifying the existing select list items to set the item matching "Jason" as selected. When the second list is rendered, this value is then used.

    However, it looks like the code that does this in MVC actually tries to avoid this situation by making a copy of the select list when it sets the selected property of the selectlistitem.

    In SelectExtensions.cs:

        if (defaultValue != null) {
            IEnumerable defaultValues = (allowMultiple) ? defaultValue as IEnumerable : new[] { defaultValue };
            IEnumerable<string> values = from object value in defaultValues select Convert.ToString(value, CultureInfo.CurrentCulture);
            HashSet<string> selectedValues = new HashSet<string>(values, StringComparer.OrdinalIgnoreCase);
            List<SelectListItem> newSelectList = new List<SelectListItem>();
    
            foreach (SelectListItem item in selectList) {
                item.Selected = (item.Value != null) ? selectedValues.Contains(item.Value) : selectedValues.Contains(item.Text);
                newSelectList.Add(item);
            }
            selectList = newSelectList;
        }

    I'm fairly certain that the creation and use of newSelectList was intended to avoid this exact situation, but is failing to do so because it modifies the existing SelectListItem rather than creating a new one.

    Does anyone with more information or exposure have any thoughts?

    Tuesday, April 30, 2013 11:56 AM

All replies

  • User-1657171777 posted

    Is there any particular reason why you want to use a DropDownListFor?  It's probably easier to use @Html.DropDownList instead

    @Html.DropDownList("myList", new SelectList(Model, "FirstName", "FirstName", "Jason"))


    Which would make Jason the selected item (FirstName would be a property of the List's model)

    If you post the code for your model, it would probably help greatly, as DropDownListFor needs a property to bind as the expression.

    Tuesday, April 30, 2013 12:23 PM
  • User-661793839 posted

    Only because it's nicer to be strongly typed. I do have a workaround for this, and I imagine your solution would work as well, but my greater concern here is that this may be a bug in the framework.

    Here is an example of the view model, not much to it.

    public class CorporateDelegatesViewModel
    {
    
    public IEnumerable<SelectListItem> Users { get; set; }
    
    public string FirstUser { get; set; }
    public string SecondUser { get; set; }
    
    }

    Here's an example of the population of the view model:

    var viewModel = new CorporateDelegatesViewModel();
    viewModel.Users = new List<SelectListItem> { new SelectListItem { Text = "Jason", Value = "Jason" },
                                                 new SelectListItem { Text = "Select a user", Value = ""} };
    viewModel.FirstUser = "Jason";
    viewModel.SecondUser = null;
    
    return View(viewModel)




    Tuesday, April 30, 2013 12:58 PM
  • User-1657171777 posted

    Don't you want it to be...

    @Html.DropDownListFor(m => m.FirstUser, new SelectList(Model.Users, "Value", "Text", Model.FirstUser))



    Tuesday, April 30, 2013 2:23 PM
  • User-661793839 posted

    I imagine that would work around the problem by creating a new list each time, but I'm unsure as to if the framework is built with that solution in mind. Given that the signature of DropDownListFor takes an IEnumerable<SelectListItem> and not a SelectList, I think it's safe to assume that what I did is valid.

    There's also the issue of the MVC source itself (that I quoted in my original post) doing what looks like an attempted but failed workaround to the problem. Of course if there were comments there, I'd know for sure ;-)

    Tuesday, April 30, 2013 2:32 PM
  • User-1657171777 posted

    JackStrife17

    DropDownListFor takes an IEnumerable<SelectListItem>

    It takes IEnumerable<SelectListItem> SelectList.  The SelectListItem is the model of the object, and the SelectList is the type of data.  You've created a SelectList using SelectListItem as the model, so the way I've presented it (and the way dropdownlistfor accept it) is valid and correct.

    For instance, let's say we have a model named User.  this model contains an int Id, string firstname, and string lastname.  It's the model of the User object.  We want to create a list-collection of User objects.  We would declare it as IEnumerable<User> SelectList. 

    The model "SelectListItem" is a generic object when you don't have a strongly typed model.  You are basically creating the model when you delcare the value and text properties of the SelectListItem.

    Tuesday, April 30, 2013 2:44 PM
  • User-661793839 posted

    I wasn't intending to dispute that using a SelectList is valid - the issue is when you create it and how many instances you have. If you create a new select list for each dropdown, the helper will still modify the SelectList, you just won't see it for the next dropdown you use because you'll be creating a new instance next time as well.

    If the framework required a difference instance of a SelectList (or IEnumerable<SelectListItem>) each time, I would have expected to see documentation of that. It does not seem unreasonable to expect that an end user may want to create multiple dropdowns using the same set of possible values.

    Again, the framework looks to explicitly try to handle this situation (and fails to do so) in code. I'm hoping for feedback from someone who's familiar with the source code I'm referring to that can confirm or deny this, so I can know if I should be writing a bug report or just always using new list instances for each dropdown.

    Tuesday, April 30, 2013 2:57 PM
  • User1256974442 posted

    I think JackStrife17 has a perfectly valid question here.I don't see any reason why the DropdownListFor html helper should change the collection of SelectListItem passed.
    It should be treated as a read-only collection for the sole purpose of populating the dropdown list and possibly determining the default selected value based on which SelectListItem has its "Selected" property set to true; but there is no reason to modify the collection for the simple basic reason that we could very well want to re-use the collection for other dropdown lists and want to keep the "default selected value" as is without modification.

    I was surprised by this issue just today and I'm leaning to creating an issue report on codeplex.

    Tuesday, May 6, 2014 5:24 AM
  • User2035305786 posted

    This problem still exists. I've submitted an issue for it in the AspNetWebStack github repository:
    GetSelectListWithDefaultValue Modifies IEnumerable<SelectListItem> selectList · Issue #271 · aspnet/AspNetWebStack

    If anything, the documentation should be updated to note this effect. Resolving this bug could be a breaking change—I imagine there is existing code that depends on this behavior.

    Friday, March 6, 2020 3:41 PM
  • User-474980206 posted

    the real issue is that select list is used to build the options, and because multiselect is allowed, the selected property of the SelectListItem is used to set the selected property of the option. also all variances of the dropdown and listbox call the same render routine, and the selectlist is the key input. the the dropdown and dropdownfor act as helper methods to set the selected options in the list.   

    I agree that an immutable list would be safer, but it would require a clone of the select list items (not the list). but C# has poor support for immutable, and you should not assume passed objects are not mutated.

     

    Friday, March 6, 2020 9:07 PM