-
Notifications
You must be signed in to change notification settings - Fork 33
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
Improve 404 behavior for MultiViews=true|false #164
Comments
Here's a rough idea for comment; this is not a PR :) This hack works for our use case in that it redirects old style URLs for file views and revision comparisons. It's not well-tested nor does it consider other reasons a person might see a 404 here. Seems the URL detection and URL construction logic should be centralized but that'd require more time. |
Could you please provide some concrete examples for URLs you have not working with MultiViews? Additionally make clear how those would need to look like to work with MultiViews, corresponding to your suggestions of automatic redirects. Maybe you could add some kind of table with existing URL, result with MultiView enabled/disabled and how a redirect should look like?
Redirecting automatically if possible at all seems to make sense to me. Though one should keep in mind that web servers might handle redirecting "wrong" URLs themself already, e.g. using |
My original request was written before looking at the code. The rough patch for comment (not a PR!) shows an idea of how it'd work and it functions for both file view and file comparisons. I did not edit for other operations. The bug isn't about MV, it's about how non-MV URLs are handled with MV enabled (I did not consider the reverse situation). URLs like so: With MV: websvn/browse/path/to/file.xml (works) The desire is for both to styles to work at the same time. With MV enabled, my rough patch adds the functioning MV URL to the 404 page when non-MV URLs are encountered. Redirection here works with header("Location:...) but I left it as a 404 as, in theory, people will update their bookmarks. I plan to make it automatically redirect after a month or so passes. |
And that's the whole downside of this approach, different URL types need to be handled in multiple files. I would try to look at something more generic, rewriting URLs at one place if possible, that's why I brought Some if* checking if MultiViews is enabled and then providing multiple RewriteRules matching non-MV URLs seems like a good start to me. Though, I didn't find yet how to check if |
Thanks, the limiting factor (for me) is figuring out the URL structure for both multiview and non-multiview styles, and doing so without trial-and-error. But after looking closer today, getURL() and getURLParts() offer big hints as does the $op switch in multiviews.php although that $op switch has fewer specific options than getURLParts(). The 2nd limiting factor (for me) is figuring out where all these variables come from and why they may or may not exist, such as $rep. There are many. Once figured out, a goal is to analyze the URL and make both styles known to all; and it appears setup.php would be the home (potentially via WebSvnConfig as it contains getURL() today). From there, performing the action would be easy. So it appears both URL formats could function simultaneously if the logic were centralized and I'll look closer as time permits. The action (e.g., notification of change, or redirect, ...) is the easier part. I'd prefer not adding another requirement and simply do this all in PHP. |
A feature request to improve 404 behavior as it relates to MultiViews support.
Three options exist for this feature request, either:
All options would provide a smoother transition from one method to another. Today I attempted a move to mutiViews=true but we have many old links (in old bug reports, for example) that turn into blank 404 pages so we feel stuck keeping it false.
The contents of these 404 pages (as of WebSVN v2.7.0) for a standard file view are:
Important: the "this path" does not provide a desirable path; instead it's a static value to the root page.
Ideally each 404 page would link to a functioning URL for the desired content (allowing options (A) and (B)) but (C) would be an okay but potentially more complicated option.
All hints for implementing the simpler (A) option are welcome! There are a lot of renderTemplate404() references and contexts; and thus far I got it working for basic file viewing to replace 'WebSVN 404 NOREP' but wonder if there's a simpler way. Would moving operation logic out of multiviews.php make sense for this?
The text was updated successfully, but these errors were encountered: