Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetSelectListWithDefaultValue Modifies IEnumerable<SelectListItem> selectList #271

Closed
Gabby-Paolucci opened this issue Mar 5, 2020 · 3 comments

Comments

@Gabby-Paolucci
Copy link

Gabby-Paolucci commented Mar 5, 2020

The GetSelectListWithDefaultValue method modifies the selectList given as a parameter. A copy of the list is made, but selectList is modified (specifically the Selected properties) as part of the process and since the object is passed by reference, the original object is modified. This is confusing behavior.

src/System.Web.Mvc/Html/SelectExtensions.cs

    private static IEnumerable<SelectListItem> GetSelectListWithDefaultValue(IEnumerable<SelectListItem> selectList, object defaultValue, bool allowMultiple)
    {
        ...
        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);
        }
        return newSelectList;
    }

Here's an example of where this can cause confusion:
@Html.DropDownListFor(model => model.Field1, Model.SelectListItems)
@Html.DropDownListFor(model => model.Field2, Model.SelectListItems)

If Field1 has a value set but Field2 is null, Field1 will still render with the value of Field1 set since Model.SelectListItems will have had Selected set to true for that SelectListItem.

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.

@Gabby-Paolucci
Copy link
Author

Gabby-Paolucci commented Mar 6, 2020

I'll also note, this was submitted as an issue for the project back in 2014 when it was hosted in CodePlex:
ASP.NET MVC / Web API / Web Pages - View Issue #1913: DropdownListFor modifies the SelectListItem collection passed to it (Wayback Machine Archive)

It was determined that the developers were "we are concerned that some users might have taken dependency on the behavior (although this is probably rare)."

And there was a post about this in 2013 in the ASP.NET Forums:
DropDownListFor modifying passed SelectListItems | The ASP.NET Forums

@mkArtakMSFT
Copy link
Member

Thanks for contacting us. @Rick-Anderson can this be clarified in the ASP.NET Web Stack docs?

@mkArtakMSFT
Copy link
Member

This issue was moved to dotnet/AspNetCore.Docs#18841

@aspnet aspnet locked and limited conversation to collaborators Jun 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants