Add safe templates documentation #892

robrap · 2016-02-25T15:09:26Z

DOC-2739

Description

Add safe template document to read the docs
Update i18n doc with new best practices
Relates to TNL-4160

Date Needed

This will be used for a coding bug bash in mid-March.

Reviewers

PR submitter checks the boxes after each reviewer finishes and gives 👍.

Subject matter expert: @nedbat
Subject matter expert: @e0d
Doc team review (sanity check/copy edit/dev edit): @catong

FYI: @scottrish

Testing

Ran ./run_tests.sh without warnings or errors

Post-review

Squash commits

robrap · 2016-02-25T15:22:27Z

en_us/developers/source/internationalization/i18n.rst

-string.::
+in an HTML page, they must be HTML escaped.  In the earlier example, this is
+handled through the ``<%page>`` directive with the ``h`` filter.  See the
+documentation on Safe Templates for more details.


@catong This isn't quite ready for review yet, but I'm not sure of the proper way to cross-link between documentation. This includes both the syntax and the english. I have done this a couple of times, so if you could let me know what I should do, that would be great. Thanks!

@robrap If the link is to an "anchor" within the same guide, which I see it is, the syntax is as follows. :ref:Safe Templates. If the link is to an anchor in another guide, it's the same with the addition of a prefix for the guide in which the anchor lives. For example, `:ref:`partnercoursestaff:Reporting Certificate Data.

Recommended wording is: "For more information, see :ref:Safe Templates``."
The ref gets replaced by a link whose text is the heading text of the cross-ref target.

robrap · 2016-02-25T15:56:16Z

en_us/developers/source/accessibility.rst

@@ -1,7 +1,7 @@
 .. _Accessibility Guidelines for Developers:

 #############################
-EdX Accessibility Guidelines
+edX Accessibility Guidelines


@catong This was just to make this consistent with all the other sections.

robrap · 2016-02-26T22:13:47Z

en_us/developers/source/safe_templates.rst

+
+    <%page expression_filter="h"/>
+    ...
+    ${get_course_date_summary(course, user) | n}


@nedbat I'm not sure if we want to use | n, | n, unicode, or HTML() for the case where we are expecting HTML. Using HTML() would be like using | n, unicode, but it might read better:

${HTML(get_course_date_summary(course, user))}

HTML() seems good to me.

nedbat · 2016-03-01T22:24:45Z

en_us/developers/source/safe_templates.rst

+    ${data | h}
+
+As we work toward this default, we'll add the following ``%page`` directive
+to each template once at the top, following any import statements.


Above the import statements would be better.

robrap · 2016-03-03T09:03:16Z

@nedbat @catong These files are ready for review again. Thanks.

nedbat · 2016-03-03T19:14:02Z

en_us/developers/source/internationalization/i18n.rst

-    <%! from openedx.core.djangolib.markup import HTML, ugettext as _ %>
+In order to mix plain text and HTML using ``format()``, you must use the
+``HTML()`` and ``Text()`` functions. The function ``HTML()`` can be used to
+wrap any replacement string that has an actual HTML tag that should not be


Active voice without options will be more direct: "Use the HTML() function when you have a replacement string that already contains HTML." or something.

nedbat · 2016-03-08T17:08:02Z

en_us/developers/source/safe_templates.rst

+   need to properly escape. In some cases, this may reduce the likelihood of
+   potential problems, but proper escaping is always necessary.
+
+#. Do not store escaped data. Again, since you do not know ahead of time all


No bold on this one?

I forgot we rewrote this to allow for bold.
@catong Can you please add bold here. Thank you.

nedbat · 2016-03-08T17:13:06Z

Resolve remaining comments as you see fit, then 👍

catong · 2016-03-08T20:13:18Z

en_us/developers/source/safe_templates.rst

+entire platform, and we will be able to remove these lines.
+
+Soon we will also have a linter available that will help with the transition
+to Mako templates that are safe by default. Additionally, the tool will help


Not clear whether "the tool" here refers to the linter or some other tool.

Same linter. Thanks.

catong · 2016-03-08T20:29:29Z

@robrap @nedbat I made a light editing pass of the safe templates file. I had some questions/comments that might result in some restructuring. I will do another pass after any structural/content changes. I also still need to take an overall look at the headings and cross-references in both the i18n and safe templates files, potentially add unique labels so that sphinx mapping works properly.

robrap · 2016-03-08T21:54:28Z

en_us/developers/source/safe_templates.rst

+    <%page expression_filter="h"/>
+    ...
+    ${data}
+


@catong It was easier for me to adjust and have you just grab my commit. I took out all discussion of the strategy here. That is handled elsewhere under strategy. Here we will just describe what we want people to do today.

robrap · 2016-03-08T23:10:46Z

en_us/developers/source/safe_templates.rst

+extremely careful when using ``| n, unicode``, and make sure the originating
+code is properly escaped. Note that the ``n`` filter turns off all default
+filters, including the default ``unicode`` filter, so it is added back
+explicitly. Here is an example.


@catong @nedbat This section was updated in a new commit as well.

robrap · 2016-03-08T23:59:24Z

@catong I'm done with my pass. As mentioned, I made some updates myself. I also added a couple of comments that I didn't update myself. I hope that wasn't confusing.

e0d · 2016-03-09T02:12:54Z

en_us/developers/source/safe_templates.rst

+
+For more details on translating strings and proper escaping, see :ref:`i18n`.
+
+There are some rare cases where we need to turn off default HTML-escaping using


For instance?

@e0d The following sentences and example are supposed to provide examples.
Here is an alternative intro sentence that scopes this to a "specific expression":
In some rare cases we need to turn off default HTML-escaping using | n, unicode for a specific expression.

catong · 2016-03-09T19:46:06Z

@nedbat @robrap Please see the latest state of the safe templates section (http://draft-dev-guide-i18n-update.readthedocs.org/en/latest/safe_templates.html). Rob and I talked and we agreed that the strategy section could become an intro to the detailed "how to make Mako templates safe" section. For now I have placed that combined section AFTER the "Editing Templates" section that has subsections for various file types, and added a cross-reference from the Mako template topic within "Editing Templates" to the new combined "Making Mako Templates Safe by Default" topic. Let me know if you think this latest incarnation works.

robrap · 2016-03-10T02:58:52Z

en_us/developers/source/safe_templates.rst

+ensure that all expressions use HTML-escaping by default. For details, see
+:ref:`Set HTML Escaping Filter as Default`.
+
+The following topics describe various methods of making your Mako templates


@catong Can we say: "The following sections describe a list of steps required for making.."

@robrap I have reworded as "The following topics describe the steps you need to take to make..." I don't want to use "sections" instead of "topics" because we generally refer to chapters as sections and the headings within a section as topics. OK?

catong · 2016-03-10T22:50:58Z

@robrap I've made a couple of final commits, and you now have my 👍 , so I leave the rest to you!
Please squash commits before merging. Thanks!

robrap · 2016-03-11T16:53:55Z

@catong As discussed, :+1 and I think this is good to go live.
@nedbat Do you agree? I'll still be working on additional updates as discussed before the marathon.

catong · 2016-03-11T18:26:44Z

@nedbat assures me that his earlier 👍 stands. @robrap my final changes are in. I'm going to squash commits in preparation for merging.

catong · 2016-03-11T18:37:29Z

Merging #892

Add safe templates doc; update i18n with best practices

robrap reviewed Feb 25, 2016
View reviewed changes

robrap force-pushed the robrap/safe-templates branch 2 times, most recently from a85012a to 46a4810 Compare February 25, 2016 15:48

robrap reviewed Feb 25, 2016
View reviewed changes

robrap force-pushed the robrap/safe-templates branch 2 times, most recently from 7bb82fa to 6cbf0f3 Compare February 26, 2016 22:01

robrap reviewed Feb 26, 2016
View reviewed changes

robrap mentioned this pull request Feb 26, 2016

Use markup HTML helper with Text openedx/edx-platform#11638

Merged

5 tasks

nedbat reviewed Mar 1, 2016
View reviewed changes

nedbat reviewed Mar 3, 2016
View reviewed changes

nedbat reviewed Mar 8, 2016
View reviewed changes

catong reviewed Mar 8, 2016
View reviewed changes

robrap reviewed Mar 8, 2016
View reviewed changes

catong force-pushed the robrap/safe-templates branch from 00a8c4c to a713017 Compare March 8, 2016 22:18

robrap reviewed Mar 8, 2016
View reviewed changes

e0d reviewed Mar 9, 2016
View reviewed changes

robrap reviewed Mar 10, 2016
View reviewed changes

Add safe templates doc, update i18n doc with best practices

b9de2eb

catong force-pushed the robrap/safe-templates branch from c2ede11 to b9de2eb Compare March 11, 2016 18:30

catong added a commit that referenced this pull request Mar 11, 2016

Merge pull request #892 from edx/robrap/safe-templates

9bba7a8

Add safe templates doc; update i18n with best practices

catong merged commit 9bba7a8 into master Mar 11, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add safe templates documentation #892

Add safe templates documentation #892

robrap commented Feb 25, 2016

robrap Feb 25, 2016

catong Feb 25, 2016

catong Feb 25, 2016

robrap Feb 25, 2016

robrap Feb 26, 2016

nedbat Feb 29, 2016

nedbat Mar 1, 2016

robrap commented Mar 3, 2016

nedbat Mar 3, 2016

nedbat Mar 8, 2016

robrap Mar 8, 2016

catong Mar 8, 2016

nedbat commented Mar 8, 2016

catong Mar 8, 2016

robrap Mar 8, 2016

catong commented Mar 8, 2016

robrap Mar 8, 2016

robrap Mar 8, 2016

robrap commented Mar 8, 2016

e0d Mar 9, 2016

robrap Mar 9, 2016

catong commented Mar 9, 2016

robrap Mar 10, 2016

catong Mar 10, 2016

catong commented Mar 10, 2016

robrap commented Mar 11, 2016

catong commented Mar 11, 2016

catong commented Mar 11, 2016


		For more details on translating strings and proper escaping, see :ref:`i18n`.

		There are some rare cases where we need to turn off default HTML-escaping using

Add safe templates documentation #892

Add safe templates documentation #892

Conversation

robrap commented Feb 25, 2016

Description

Date Needed

Reviewers

Testing

Post-review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robrap commented Mar 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedbat commented Mar 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catong commented Mar 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robrap commented Mar 8, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catong commented Mar 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catong commented Mar 10, 2016

robrap commented Mar 11, 2016

catong commented Mar 11, 2016

catong commented Mar 11, 2016