MvcSiteMapProvider icon indicating copy to clipboard operation
MvcSiteMapProvider copied to clipboard

Casing of RouteData.Values affects finding of CurrentNode

Open kimpenhaus opened this issue 10 years ago • 3 comments

I was trying to use MvcSitemapProvider together with T4MVC (and its extensions) and stumbled across a casing problem. That one is already mentioned on StackOverflow.

T4MVC is registering it's routes with capitalized keys.

2015-02-03_08-46-18

The routing mechanism from ASP.NET MVC itself works fine with that precisely because the Microsoft documentation of the RouteValueDictionary reads:

Represents a case-insensitive collection of key/value pairs that you use in various places in the routing framework, such as when you define the default values for a route or when you generate a URL that is based on a route.

The MvcSiteMapProvider fails to set the CurrentNode - from what I can see in the sources - the MatchRoute function inside the RouteValueDictionary is comparing keys not case-sensitive.

2015-02-03_09-30-29

2015-02-03_09-17-28

There is a MergeRouteValuesAndNamedQueryStringValues function which is correcting the casing of the query string values but not those of the route values which leads to no matching in the MatchRoute function mentioned above.

2015-02-03_09-26-06

One can have a discussion now, if all route values should be lower-cased by T4MVC or what the meaning of case-insensitive collection of key/value pairs from the MS documentation means. But I would like to see both systems (MVC and SiteMapProvider) behave the same way.

Thanks for taking your time! I really appreciate!

Cheers, Marcus

kimpenhaus avatar Feb 03 '15 08:02 kimpenhaus

Thanks for reporting that case of the route values matter. This helped me get my breadcrumbs to work!

I would cast a vote to towards having the SiteMapProvider matching be case insensitive to closer match the behaviour described in the MS documentation.

bounav avatar Jul 27 '16 10:07 bounav

This doesn't seem fixed yet but it's pretty severe.

brgrz avatar Aug 22 '17 12:08 brgrz

PR is welcomed if you want to dig in

maartenba avatar Aug 22 '17 12:08 maartenba