From: Mike Samuel <mikesamuel@gmail.com>
Date: Thu, 22 Sep 2011 04:38:40 +0000 (-0700)
Subject: exp/template/html: elide comments in template source.
X-Git-Tag: weekly.2011-09-21~1
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=35819729b83b8521e9ae1bddca5c368672777de5;p=gostls13.git

exp/template/html: elide comments in template source.

When templates are stored in external files, developers often embed
comments to explain&|disable code.

  <!-- Oblique reference to project code name here -->
  {{if .C}}...{{else}}<!-- commented out default -->{{end}}

This unnecessarily increases the size of shipped HTML and can leak
information.

This change elides all comments of the following types:
1. <!-- ... --> comments found in source.
2. /*...*/ and // comments found in <script> elements.
3. /*...*/ and // comments found in <style> elements.

It does not elide /*...*/ or // comments found in HTML attributes:
4. <button onclick="/*...*/">
5. <div style="/*...*/">

I can find no examples of comments in attributes in Closure Templates
code and doing so would require keeping track of character positions
post decode in

  <button onclick="/&#42;...*/">

To prevent token joining, /*comments*/ are JS and CSS comments are
replaced with a whitespace char.
HTML comments are not, but to prevent token joining we could try to
detect cases like
   <<!---->b>
   </<!---->b>
which has a well defined meaning in HTML but will cause a validator
to barf.  This is difficult, and this is a very minor case.
I have punted for now, but if we need to address this case, the best
way would be to normalize '<' in stateText to '&lt;' consistently.

The whitespace to replace a JS /*comment*/ with depends on whether
there is an embedded line terminator since
    break/*
    */foo
    ...
is equivalent to
    break;
    foo
    ...
while
    break/**/foo
    ...
is equivalent to
    break foo;
    ...

Comment eliding can interfere with IE conditional comments.
http://en.wikipedia.org/wiki/Conditional_comment

<!--[if IE 6]>
<p>You are using Internet Explorer 6.</p>
<![endif]-->

/*@cc_on
  document.write("You are using IE4 or higher");
@*/

I have not encountered these in production template code, and
the typed content change in CL 4962067 provides an escape-hatch
if conditional comments are needed.

R=nigeltao
CC=golang-dev
https://golang.org/cl/4999042
---

diff --git a/src/pkg/exp/template/html/escape.go b/src/pkg/exp/template/html/escape.go
index 28019f2525..650a6acd28 100644
--- a/src/pkg/exp/template/html/escape.go
+++ b/src/pkg/exp/template/html/escape.go
@@ -568,12 +568,43 @@ func (e *escaper) escapeText(c context, n *parse.TextNode) context {
 					written = j + 1
 				}
 			}
+		} else if isComment(c.state) && c.delim == delimNone {
+			switch c.state {
+			case stateJSBlockCmt:
+				// http://es5.github.com/#x7.4:
+				// "Comments behave like white space and are
+				// discarded except that, if a MultiLineComment
+				// contains a line terminator character, then
+				// the entire comment is considered to be a
+				// LineTerminator for purposes of parsing by
+				// the syntactic grammar."
+				if bytes.IndexAny(s[written:i1], "\n\r\u2028\u2029") != -1 {
+					b.WriteByte('\n')
+				} else {
+					b.WriteByte(' ')
+				}
+			case stateCSSBlockCmt:
+				b.WriteByte(' ')
+			}
+			written = i1
+		}
+		if c.state != c1.state && isComment(c1.state) && c1.delim == delimNone {
+			// Preserve the portion between written and the comment start.
+			cs := i1 - 2
+			if c1.state == stateHTMLCmt {
+				// "<!--" instead of "/*" or "//"
+				cs -= 2
+			}
+			b.Write(s[written:cs])
+			written = i1
 		}
 		c, i = c1, i1
 	}
 
 	if written != 0 && c.state != stateError {
-		b.Write(n.Text[written:])
+		if !isComment(c.state) || c.delim != delimNone {
+			b.Write(n.Text[written:])
+		}
 		e.editTextNode(n, b.Bytes())
 	}
 	return c
diff --git a/src/pkg/exp/template/html/escape_test.go b/src/pkg/exp/template/html/escape_test.go
index 84bf6b7a4a..8a64515dec 100644
--- a/src/pkg/exp/template/html/escape_test.go
+++ b/src/pkg/exp/template/html/escape_test.go
@@ -363,13 +363,12 @@ func TestEscape(t *testing.T) {
 		{
 			"HTML comment",
 			"<b>Hello, <!-- name of world -->{{.C}}</b>",
-			// TODO: Elide comment.
-			"<b>Hello, <!-- name of world -->&lt;Cincinatti&gt;</b>",
+			"<b>Hello, &lt;Cincinatti&gt;</b>",
 		},
 		{
 			"HTML comment not first < in text node.",
 			"<<!-- -->!--",
-			"&lt;<!-- -->!--",
+			"&lt;!--",
 		},
 		{
 			"HTML normalization 1",
@@ -384,18 +383,18 @@ func TestEscape(t *testing.T) {
 		{
 			"HTML normalization 3",
 			"a<<!-- --><!-- -->b",
-			"a&lt;<!-- --><!-- -->b",
+			"a&lt;b",
 		},
 		{
 			"Split HTML comment",
 			"<b>Hello, <!-- name of {{if .T}}city -->{{.C}}{{else}}world -->{{.W}}{{end}}</b>",
-			"<b>Hello, <!-- name of city -->&lt;Cincinatti&gt;</b>",
+			"<b>Hello, &lt;Cincinatti&gt;</b>",
 		},
 		{
 			"JS line comment",
 			"<script>for (;;) { if (c()) break// foo not a label\n" +
 				"foo({{.T}});}</script>",
-			"<script>for (;;) { if (c()) break// foo not a label\n" +
+			"<script>for (;;) { if (c()) break\n" +
 				"foo( true );}</script>",
 		},
 		{
@@ -405,8 +404,8 @@ func TestEscape(t *testing.T) {
 			// Newline separates break from call. If newline
 			// removed, then break will consume label leaving
 			// code invalid.
-			"<script>for (;;) { if (c()) break/* foo not a label\n" +
-				" */foo( true );}</script>",
+			"<script>for (;;) { if (c()) break\n" +
+				"foo( true );}</script>",
 		},
 		{
 			"JS single-line block comment",
@@ -417,25 +416,25 @@ func TestEscape(t *testing.T) {
 			// removed, then break will consume label leaving
 			// code invalid.
 			"<script>for (;;) {\n" +
-				"if (c()) break/* foo a label */foo;" +
+				"if (c()) break foo;" +
 				"x( true );}</script>",
 		},
 		{
 			"JS block comment flush with mathematical division",
 			"<script>var a/*b*//c\nd</script>",
-			"<script>var a/*b*//c\nd</script>",
+			"<script>var a /c\nd</script>",
 		},
 		{
 			"JS mixed comments",
 			"<script>var a/*b*///c\nd</script>",
-			"<script>var a/*b*///c\nd</script>",
+			"<script>var a \nd</script>",
 		},
 		{
 			"CSS comments",
 			"<style>p// paragraph\n" +
 				`{border: 1px/* color */{{"#00f"}}}</style>`,
-			"<style>p// paragraph\n" +
-				"{border: 1px/* color */#00f}</style>",
+			"<style>p\n" +
+				"{border: 1px #00f}</style>",
 		},
 		{
 			"JS attr block comment",
@@ -462,12 +461,12 @@ func TestEscape(t *testing.T) {
 		{
 			"HTML substitution commented out",
 			"<p><!-- {{.H}} --></p>",
-			"<p><!--  --></p>",
+			"<p></p>",
 		},
 		{
 			"Comment ends flush with start",
 			"<!--{{.}}--><script>/*{{.}}*///{{.}}\n</script><style>/*{{.}}*///{{.}}\n</style><a onclick='/*{{.}}*///{{.}}' style='/*{{.}}*///{{.}}'>",
-			"<!----><script>/**///\n</script><style>/**///\n</style><a onclick='/**///' style='/**///'>",
+			"<script> \n</script><style> \n</style><a onclick='/**///' style='/**///'>",
 		},
 		{
 			"typed HTML in text",