jekyll / jekyll-redirect-from

:twisted_rightwards_arrows: Seamlessly specify multiple redirections URLs for your pages and posts.
MIT License
781 stars 112 forks source link

regression bug redirect_to stop working with relative paths on 0.12 #149

Closed bcardiff closed 6 years ago

bcardiff commented 7 years ago

With jekyll-redirect-from-0.11.0 redirect_to was not working for absolute paths as described in #130.

After jekyll-redirect-from-0.12.0 (and 0.12.1), redirect_to works with absolute paths but stop working with relative paths.

I extend the sample repo for #130 for testing redirects in the same directory https://github.com/bcardiff/issue-redirect-to/commit/5e87a14dda558de650a29a9a8f8eaca03bd36ac7 and across different directories https://github.com/bcardiff/issue-redirect-to/commit/950dfd74692bad04a4e1cf8e266def452f6d9940 .

I think that if the redirect_to is relative ./ or ../ the path could remain unchanged. Probably here but I am not jekyll-wise.

These are the generated outputs for 0.11:

_site/path/redirect-abs.html
<script>location="/nested/path1/original.html"</script>
_site/path/redirect-rel.html
<script>location="./original.html"</script>
_site/nested/path2/redirect-abs.html
<script>location="/nested/path1/original.html"</script>
_site/nested/path2/redirect-rel.html
<script>location="../path1/original.html"</script>

These are the generated output for 0.12:

_site/path/redirect-abs.html
<script>location="http://localhost:4000/issue-redirect-to/path/original.html"</script>
_site/path/redirect-rel.html
<script>location="http://localhost:4000/issue-redirect-to/original.html"</script>
_site/nested/path2/redirect-abs.html
<script>location="http://localhost:4000/issue-redirect-to/nested/path1/original.html"</script>
_site/nested/path2/redirect-rel.html
<script>location="http://localhost:4000/path1/original.html"</script>
benbalter commented 7 years ago

@bcardiff Thanks for the great but report. You're right, it appears to be the case (based on the code) that we need to resolve relative links before making them absolute. Is the same true for redirect_from?

bcardiff commented 7 years ago

Regarding redirect_from both 0.11 0.12 seems to behaves in the same buggy way. The only difference is in blank spaces.

I added https://github.com/bcardiff/issue-redirect-to/commit/a140b6f740944a56ba604dd27adfd87ba387aff1

# file: path/original.md
---
redirect_from:
  - ./with-from-rel.html
  - /path/with-from-abs.html
# file: nested/path1/original.md
---
redirect_from:
  - ../path2/with-from-rel-nested.html
  - nested/path2/with-from-abs-nested.html
benbalter commented 7 years ago

@bcardiff I don't think this plugin should support relative redirect_from or redirect_to paths. There's two reasons for this:

  1. AFAIK, Jekyll doesn't support relative paths in any other context. If you have /foo/bar/baz.html and give it a permalink of ../page.html, it'll be outputted at /page.html.

  2. Things get really confusing for end users really quickly. If I have redirect_from: ../foo.html, we need to build that off the page's permalink, not the page's path, meaning if there's a custom permalink set, or if pretty permalinks are enabled, you may end up in the case where a file in /bar.html with permalink: /foo/bar/ (which is really /foo/bar/index.html) with a redirect of ../foo.html really outputs to /foo/foo.html.

Is there a strong reason why the redirects can't be relative to the site source, rather than the file's URL?

bcardiff commented 7 years ago

No strong reason here. After I first posted #130, while the fix reach the stable version my workaround was to use relative paths. And they worked until 0.12. Since I was using them back then I discover the regression.

I am able to use just absolute paths and fully understand if this ends up out of scope and was working without explicit intent.

If this ends up in out of scope, maybe a proper error should apear to the user.

jekyllbot commented 7 years ago

This issue has been automatically marked as stale because it has not been commented on for at least two months.

The resources of the Jekyll team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, please consider whether it can be accomplished in another way. If it cannot, please elaborate on why it is core to this project and why you feel more than 80% of users would find this beneficial.

This issue will automatically be closed in two months if no further activity occurs. Thank you for all your contributions.

ndarville commented 6 years ago

This has tripped me up a bunch of time, and I think an error is the best way to go since a localhost redirect in production doesn’t do anyone any good.