Remove safe and markdown filters from templates
#286
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We used this once when DOS1 was opening, but since then we've interated relentlessly and focused like crazy on user needs so that now we have a much simpler template. This one was ultimately too prescriptive to really be extended and so let's just get rid of it.
The markdown filter is becoming a bit dangerous and now that we have TemplateFields in the ContentLoader, we can stop using it.
Within the self-contained universe of the frontend toolkit, 'markdown' as a concept has been obliterated. It seemed like we had two options going forward: 1. Do some intermediary processing in the `generate_pages.py` file to run some of our question fields through markdown. This would reflects how these particular fields from the content will actually appear once rendered in front of users. 2. Remove all markdown from the examples in favour of straight HTML. This is consistent with how the templates itself act in isolation. We've gone with the latter approach because in general it seems safer to ignore the world outside of this app because it's less certain and subject to change.
If we pull in the content-loader, we don't need the markdown filter at all anymore. Means we can un-require the utils repo altogether.
It's better to pass in messages as Markup than to require all of the templates use the `safe` filter.
Doesn't appeear to be used anywhere, so it gets ✂️.
|
👍 |
|
Would be good to add a record to the changelog for the breaking version bump please :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So it turns out that using
safeandmarkdown(which is more permissive thansafe) isn't actually that safe, despite the deceptive name.This pull request removes all uses of
safeandmarkdownfrom our templates, as well as removes two outdated patterns that we have stopped using (namely,search-resultsandframework-notice).It's a pretty large breaking change because in quite a few places we're passing stuff into the templates that we expect will be escaped, but @allait and I are taking care of pulling them into the frontend apps.
Markdown formatted text in the content is still allowed, it's just going to be handled in the content loader now instead of here, so some of our examples have had to change to just using straight HTML rather than asterisks and dashes.