-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Fix author links in the proposals lists #1997
Conversation
Links were broken since the upgrade of the hugo version. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
{{ $authors_list := apply .authors "htmlEscape" "." }} | ||
{{ $authors_list := apply .authors "printf" "<a href=\"https://github.com/%s\">@%s</a>" "." "." }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand this. Isn't the new value immediately overrwritten?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, it's using the wrong variable here. The htmlEscape
is a only precaution anyway in case there is a special HTML character in the username of an author, but I'm not even sure GitHub would allow that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember now why I did that, even if GitHub doesn't allow special characters in the username, it's because the data from the MSC description is not validated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ad now I realized that for the URL, we should not use htmlEscape
but rather a function that converts to a URL-compatible format. Hugo has urlize
but it also transforms the string instead of just converting special characters.
Maybe we should consider sanitizing to be the job of the script that fetches the proposals?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the call to htmlEscape
, maybe it can be done separately, to see if we want to sanitize the input of the proposals.
Could you explain how #1984 broke this? |
I am not sure but there was a Hugo update, so I guess a fix on their part requires the use of |
Signed-off-by: Kévin Commaille <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Links were broken since the upgrade of the hugo version in #1984.
The broken state can be viewed at the preview from that PR.
It cannot be viewed at https://spec.matrix.org/proposals yet because it hasn't been updated since the upgrade (although it is broken on a different way because of the upgrade too).Now visible at https://spec.matrix.org/proposals/#work-in-progressPull Request Checklist
Preview: https://pr1997--matrix-spec-previews.netlify.app