Closed GoogleCodeExporter closed 8 years ago
I've created a patch for the issue.
Original comment by codemu...@googlemail.com
on 28 Dec 2010 at 4:21
Attachments:
I took a look at the patch and there's one part I didn't understand and think
might be incorrect.
+ if(navigateUrl.EndsWith("default.aspx",
StringComparison.OrdinalIgnoreCase)
+ && Page.Request.Url.AbsolutePath == "/")
+ {
+ navigateUrl = "/";
+ }
What if the navigateUrl was set to "/foo/bar/default.aspx". It would be
highlighted when the root is highlighted according to this logic.
Original comment by haac...@gmail.com
on 29 Dec 2010 at 6:25
Also, we got rid of the Default.aspx file in the root. So you'll need to double
check this logic against the latest trunk. We're going to be moving towards
extensionless URLs.
Original comment by haac...@gmail.com
on 29 Dec 2010 at 6:28
Yes you're right. I didn't think of sub folders. Attached there's a new patch
that takes sub folders in account. In addition I've cleaned the if statement a
bit and moved the logic to two boolean functions.
I already thought of moving those new functions from the controls codebehind to
a helper class so that I could easily unit test them. In addition they could be
useful in other places, too. However I wasn't sure in wich helper to put them.
Maybe UrlHelper or HttpHelper?
Original comment by codemu...@googlemail.com
on 29 Dec 2010 at 10:57
Attachments:
Fixed in r4172.
Note that this code works now, but it may break when we move to extensionless
URLs. For example, IsDirectory assumes ending in slash means directory. But
/foo/bar is also a "directory" request.
To test out the extensionless pattern, change the following route in Routes.cs:
routes.MapControls("archives", "archives.aspx", null, new[] { "ArchivesPage" });
to
routes.MapControls("archives", "archives", null, new[] { "ArchivesPage" });
And you'll see the navigation link for Archives doesn't work in all skins.
Original comment by haac...@gmail.com
on 31 Dec 2010 at 6:26
Original issue reported on code.google.com by
codemu...@googlemail.com
on 28 Dec 2010 at 4:05