Using the `_html` suffix in an i18n key is the same as using `html_safe`
on it, which means that translation could potentially be used for XSS
attacks.
Using the `_html` suffix automatically marks texts as HTML safe, so
doing so on sanitized texts is redundant.
Note flash texts are not sanitized the moment they are generated, but
are sanitized when displayed in the view.
Using `<%==` is the same as using `raw`. I'm not sure if we meant
`sanitize` in this case, or it's just a typo. I'm assuming the latter
since we don't use anything similar in any other places.
They do the exact same thing; however `html_safe` might confuse
developers into thinking it will make the HTML safe. Using `raw` makes
it clear that we're inserting the text without escaping it.
The difference is `html_safe` allows every HTML tag, including the
`<script>` tag, while `sanitize` only allows tags which are considered
safe. In this case, we want to allow a `<span>` tag in a translation,
and links inside flash messages.
Sometimes we're interpolating a link inside a translation, and marking
the whole translations as HTML safe.
However, some translations added by admins to the database or through
crowdin are not entirely under our control.
Although AFAIK crowdin checks for potential cross-site scripting
attacks, it's a good practice to sanitize parts of a string potentially
out of our control before marking the string as HTML safe.
There's a slight chance an attribute like an author's name might contain
an attempt to perform XSS attacks. So, instead of marking the whole text
as HTML safe, we can sanitize it.
Also note I'm removing the `_html` suffix in the i18n key, since it's
got the same effect as using `html_safe`.
The name `safe_html_with_links` was confusing and could make you think
it takes care of making the HTML safe. So I've renamed it in a way that
makes it a bit more intuitive that it expects its input to be already
sanitized.
I've changed `text_with_links` as well so now the two method names
complement each other.
There's a case where we would face a Cross-Site Scripting attack. An
attacker could use the browser's developer tools to add (on their
browser) a `<code>` tag with a `<script>` tag inside in the text of the
draft version. After doing so, commenting on that text would result in
the attacker's JavaScript being executed.
It's possible to create a newsletter or a proposed action with
<script> tags by filling in the body using a textarea instead of a
CKEditor. While we trust our administrators not to do so, it's better to
completely eliminate that possibility.
This way we can simplify the way we generate form fields. In some cases,
we also use the human attribute in table headers, which IMHO makes
sense.
I haven't moved all of them: for example, sometimes a label is
different depending on whether it's shown to administrators, valuators,
or users. And I haven't touched the ones related to devise, since I
wasn't sure about possible side effects.
Note I've also removed placeholders when they had the same text as their
labels, since they weren't helpful. On the contrary, the added redundant
text to the form, potentially distracting users.
I'm not sure why it isn't already done by foundation's form builder. It
doesn't make any sense to change an ID of a form field without changing
the `for` attribute of its label.
Using the block syntax to generate the label with a <span> tag inside
isn't necessary after upgrading foundation_rails_helpers. Before the
upgrade, we couldn't do so because the <span> tag was escaped.
Moderate legislation proposals
- added a controller for moderation/legislation
- updated view to appropriate link + added route
- added a spec
- Feature test
- test for faded
- javascripts for visual effects
This way we write the tests from the user's point of view: users can see
(for example) a proposal with the title "Make everything awesome", but
they don't see a proposal with a certain ID.
There are probably dozens, if not hundreds, of places where we could
write tests this way. However, it's very hard to filter which ones are
safe to edit, since not many of them have an HTML class we can use in
the tests, and adding a class might generate conflicts with CSS styles.
So, for now, I'm only changing the ones allowing us to cleanly remove
useless assignements while maintaining the code vertically aligned.
