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.

Sign up for GitHub

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

Jump to bottom

Fix author links in the proposals lists #1997

Merged
merged 3 commits into from
Nov 20, 2024
Merged

Fix author links in the proposals lists #1997

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d1cf7c
Fix the proposals lists
zecakeh Nov 14, 2024
e6ab4c6
Add changelog
zecakeh Nov 14, 2024
f39c44c
Remove HTML sanitizing
zecakeh Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/internal/newsfragments/1997.clarification
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix relative URLs when serving the specification with a custom `baseURL`.
5 changes: 3 additions & 2 deletions layouts/shortcodes/proposal-tables.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
{{ end }}
{{ $docs_links := delimit $docs_links_list ", " }}

{{ $authors_list := apply .authors "htmlEscape" "." }}
{{ $authors_list := apply .authors "printf" "<a href=\"https://github.com/%s\">@%s</a>" "." "." }}
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

{{ $authors := delimit $authors_list ", " }}

Expand All @@ -68,8 +69,8 @@
<td>{{ .title }}</td>
<td>{{ .created_at }}</td>
<td>{{ .updated_at }}</td>
<td>{{ with $docs_links }}{{ $docs_links }}{{ end }}</td>
<td>{{ $authors }}</td>
<td>{{ with $docs_links }}{{ $docs_links | safeHTML }}{{ end }}</td>
<td>{{ $authors | safeHTML }}</td>
<td>{{ with .shepherd }}<a href="https://github.com/{{ . }}">@{{ . }}</a>{{ end }}</td>
</tr>
{{ end }}
Expand Down