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

Sitemap page links are broken when application is hosted as sub application #222

Merged
merged 1 commit into from
Aug 30, 2013
Merged

Sitemap page links are broken when application is hosted as sub application #222

merged 1 commit into from
Aug 30, 2013

Conversation

uhaciogullari
Copy link
Contributor

When sitemap is broken into pages, page URLs are not generated correctly. They are just added to the end of domain. These links do not work when application is hosted inside another website.

sb1
sb2

I fixed this by injecting IUrlPath into XmlSiteMapResult and using it to generate page links. It fixes the problem but you may have a better solution.

NightOwl888 added a commit that referenced this pull request Aug 30, 2013
Sitemap page links are broken when application is hosted as sub application
@NightOwl888 NightOwl888 merged commit d14fbba into maartenba:dev Aug 30, 2013
@NightOwl888
Copy link
Collaborator

Thanks for the elegantly done fix.

@NightOwl888
Copy link
Collaborator

I modified this a bit in v4.2.0. Rather than passing the urlPath to the XmlSiteMapResult I modified the DefaultBaseUrl of XmlSiteMapResultFactory to return the result of urlPath.MakeRelativeurlAbsolute("~/") minus the trailing slash.

I also did some much needed consolidation of the display logic of XmlSiteMapResult.

Take a look and let me know if it is still working for you.

@uhaciogullari
Copy link
Contributor Author

It works, thanks.

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 We will have to reopen this one because it messed up the regular links now.
untitled

This happens when you create DynamicNode instances with a route.

@NightOwl888
Copy link
Collaborator

Ok, I take it the expected behavior is for the URLs in your example to be:

http://sitemapsample.com/subapp/Store/Browse/34741
http://sitemapsample.com/subapp/Store/Browse/34742

Correct?

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 That's correct.

NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this pull request Sep 20, 2013
@NightOwl888
Copy link
Collaborator

Ok, I have added the patch here: https://github.com/NightOwl888/MvcSiteMapProvider/tree/dev

Please verify it is working the way you need it to. It is almost exactly like you had it the first time, except is uses the baseUrl parameter in conjunction with urlPath.MakeVirtualPathAppAbsolute(), which is exactly what the urlPath.MakeRelativeUrlAbsolute() does internally.

I have also made the overloads that don't accept a page number obsolete because it enables a scenario where the sitemap will stop working (switch to an index page) when the site reaches 35,000 nodes.

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 Nope, doesn't work. It's still duplicated.

@NightOwl888
Copy link
Collaborator

Just so we are on the same page, please verify you have the dev branch. The master branch is the same as v4.3.0, which is definitely broken.

When you nest your application, do you set the virtual directory as an application in IIS? It has been a long time since I attempted something like this and I don't think I ever got it working. But I would like to set up a test so I can try to determine what is going on. Please give step by step instructions (or a link to them) on how to set this up.

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 Yes, I tested it on dev branch.

Just right click on a website on IIS, click Add Application, set the path as the MvcMusicStore's folder and see if you can see the homepage. You may need to remove Razor section from the Web.config in Views folder.

@NightOwl888
Copy link
Collaborator

I set up MvcMusicStore as a sub-application as you described, but I am not getting the duplicated sub application name.

image

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 It happens when sitemap is broken into pages. Create a dynamic node provider that returns more than 35,000 links.

@NightOwl888
Copy link
Collaborator

Is it happening on the index page, or the link pages?

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 Link pages.

@NightOwl888
Copy link
Collaborator

I added a dynamic node provider with 35,000 more nodes. Still unable to duplicate the issue. So either it is something in your environment, or the setup instructions weren't enough to reproduce your same conditions.

image

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 I specified a route while creating DynamicNode instances. How did you create yours?

for (int i = 0; i < 50000; i++)
{
    yield return new DynamicNode { Route = "Browse", Action = "Browse", Controller = "Store", RouteValues = new Dictionary<string, object> { { "genre", i } } };
}

@NightOwl888
Copy link
Collaborator

I wasn't adding the route, but I tried that as well and I still can't reproduce what you are seeing. I was able to reproduce it using the code from the original dev branch, though. That is good enough for me - I have verified the problem no longer happens using the current code.

NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this pull request Sep 21, 2013
NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this pull request Sep 21, 2013
NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this pull request Sep 21, 2013
@NightOwl888
Copy link
Collaborator

Okay, the fix is now in v4.4.0, which is also available on NuGet.

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 This fixed the regular links but page links are broken now :)
Same as the first image I posted.

NightOwl888 added a commit to NightOwl888/MvcSiteMapProvider that referenced this pull request Sep 23, 2013
@NightOwl888
Copy link
Collaborator

So we have come full circle.

I have confirmed the index page is broken and added a patch here. Please confirm it works on your end before I make the patch into an official release.

@uhaciogullari
Copy link
Contributor Author

@NightOwl888 Looks clear :)

@NightOwl888
Copy link
Collaborator

Ok, the patch is in v4.4.1.

@uhaciogullari
Copy link
Contributor Author

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants