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

Preserve search and hash in redirect #127

Closed
wants to merge 1 commit into from
Closed

Conversation

FokkeZB
Copy link

@FokkeZB FokkeZB commented Nov 18, 2016

Closes #123

  • Delays meta refresh with 1s to give JavaScript time to update it
  • Update meta refresh and link (canonical doesn't make any sense AFAIK) with search and hash
  • Use search and hash in scripted redirect

@FokkeZB
Copy link
Author

FokkeZB commented Nov 18, 2016

One job still fails but that seems to be a different issue:
https://travis-ci.org/jekyll/jekyll-redirect-from/jobs/176961793

@pathawks
Copy link
Member

What if the redirect URL already has a query?

I don't understand the motivation behind trying to modify <meta> tags with JS.

@FokkeZB
Copy link
Author

FokkeZB commented Nov 18, 2016

@pathawks we're talking a static generator right? I can't think of a situation where #{item_url} would have a query?

I agree modifying the refresh meta tag has not a ton of value, neither has modifying the link. If you have JavaScript, the location.replace() will do the job anyway. So I can remove that part.

But.. it do think it would should let the meta refresh at 1s to make sure JavaScript can handle te redirect before the browser does.

@pathawks
Copy link
Member

we're talking a static generator right?

Either it is important or it isn't.

A page could have been integrated into a section of another page. Perhaps the URL contains a hash of the section in the new page that contains the old content.

I feel like this greatly increases the complexity of the template, and I worry that there are some edge cases where problems could creep up.

@FokkeZB
Copy link
Author

FokkeZB commented Nov 18, 2016

What I meant is that the afaik #{item_url}always refers to the URL of a generated page as Jekyll constructs it and I don't think these ever come with a search or hash?

Sure, the it could be that the search and hash no longer serve a purpose on the new page, but in that case it wouldn't hearth anything either. It just wouldn't do anything.

In the meanwhile, if would still do anything... without this fix it doesn't let you.

@pathawks
Copy link
Member

What I mean is, if I had a page location.html and I remove it, and redirect it to about.html#location

With these new changes, if somebody lands on location.html?q=p#hash, it looks like they will be redirected to about.html#location?q=p#hash, which does not seem appropriate.

@FokkeZB
Copy link
Author

FokkeZB commented Nov 19, 2016

Ah, you mean when using redirect_to? Although afaik redirect_from is preferred when redirecting within the site and redirect_to is mostly used to redirect to external websites - I guess indeed you could use it internally as well and then potentially run into this issue.

I will make a change to accommodate.

@FokkeZB
Copy link
Author

FokkeZB commented Nov 19, 2016

All checks green, except still this odd one: https://travis-ci.org/jekyll/jekyll-redirect-from/jobs/177239657

@pathawks
Copy link
Member

Not your fault. Looks like it's time for us to stop testing against Ruby 1.9

@FokkeZB
Copy link
Author

FokkeZB commented Jan 18, 2017

@pathawks did you have a chance to have another look at this? @jekyllbot just staled #123 so I'd love to see this (updated and) merged.

@haniawni
Copy link

haniawni commented May 8, 2017

Hi! Random First-Time User here.
Ran into the exact problem by @pathawks here where redirect-to with Hashes wont connect.

Example is visible here:

Is there an existing workaround?

@FokkeZB FokkeZB closed this Jun 8, 2017
@FokkeZB FokkeZB deleted the patch-1 branch June 8, 2017 18:40
@mpchadwick
Copy link

Not sure what happened here, but I think the ability to preserve the hash and query string when redirecting is pretty important.

For example, I have a page at /projects which I'm thinking about changing to /work. I can link directly to a certain section of /projects with a hash /projects/#open-source-contributions. If I have backlinks directly to that section, I don't want them to lose the hash when redirecting.

That being said, I understand the concerns outline here.

Perhaps there could be an additional flag in the frontmatter to preserve these?

redirect_from:
   - /projects/
redirect_preserve:
   - hash
   - query

@pathawks
Copy link
Member

No, adding options would increase complexity, not reduce it.

Any solution would require JavaScript which is far more complex than our existing code. I’m not convinced it’s worth the trade.

@mpchadwick
Copy link

@pathawks i'm not arguing that it makes this less simple. all I'm saying is that it's a useful feature and something i would expect to be able to do with a tool that facilitates redirects. as you can see from the issues on the repo i'm not the only one who thinks this.

using a flag to enable the feature would prevent some of the potential drawbacks and edge-cases you've outlined as it would require users to specifically opt-in.

@pathawks
Copy link
Member

Thank you for the Pull Request. Code looks solid, but I don't think this feature is worth the complexity it adds to the plugin.

@jekyll jekyll locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve fragment and query string in redirect
5 participants