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

change safeURL to absLangURL in site-navbar menu for multi languages #56

Merged
merged 2 commits into from May 7, 2018
Merged

change safeURL to absLangURL in site-navbar menu for multi languages #56

merged 2 commits into from May 7, 2018

Conversation

ghost
Copy link

@ghost ghost commented May 5, 2018

After multi-language setting is enabled, once I change to another language, and click the site navbar menu, the site url may redirects to the default language menu.

The absolute URL function you use in the file layouts/partials/header.html is safeURL. Changing it to function absLangURL will solve the issue.

You can see the effect on my blog.

@Zebradil Zebradil requested review from xianmin and Zebradil May 5, 2018 22:09
@Zebradil
Copy link
Collaborator

Zebradil commented May 5, 2018

@MaxdSre Please, remove the last commit from this PR, because it's not related to the initial issue.

@Zebradil
Copy link
Collaborator

Zebradil commented May 5, 2018

As for your proposal about sitemap. I like to have an option for this in configuration. But now it could be a braking change for some users.

@Zebradil
Copy link
Collaborator

Zebradil commented May 5, 2018

Also, the first commit brakes links with non-standard schemas as it's explained in docs:

Without safeURL, only the URI schemes http:, https: and mailto: are considered safe by Go templates. If any other URI schemes (e.g., irc: and javascript:) are detected, the whole URL will be replaced with #ZgotmplZ. This is to “defang” any potential attack in the URL by rendering it useless.

@Zebradil
Copy link
Collaborator

Zebradil commented May 5, 2018

But in the following way we can handle it:

{{ .URL | absLangURL | safeURL}}

@ghost
Copy link
Author

ghost commented May 5, 2018

@Zebradil I have remove the last commit, and I also tested {{ .URL | absLangURL | safeURL }} in my blog, it still works well on my online blog.

Copy link
Collaborator

@Zebradil Zebradil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff. Let's wait for @xianmin.

@xianmin BTW, I'd take in consideration @MaxdSre's proposal about sitemap. It's not important for me, but argumentation was good and it can be useful.

Copy link
Owner

@xianmin xianmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, in the end, should we change to {{ .URL | absLangURL | safeURL}} ?
The code I checked is still {{ .URL | absLangURL }}.

@Zebradil
Copy link
Collaborator

Zebradil commented May 6, 2018

Yes, we need to change it. Yesterday I saw another version. Probably, force push was done to the source branch.

@ghost
Copy link
Author

ghost commented May 6, 2018

@Zebradil @xianmin I push a new commit to change it to {{ .URL | absLangURL | safeURL }}.

@xianmin xianmin merged commit 1cdd921 into xianmin:master May 7, 2018
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