Document and fix HTML-escape.

sdk/lib/convert/html_escape.dart

Index: sdk/lib/convert/html_escape.dart
diff --git a/sdk/lib/convert/html_escape.dart b/sdk/lib/convert/html_escape.dart
index 512d9c31f2bf4da229779f9a6dba2d21d6869946..eb1b8d9e0214a9be3712ce587a23527fc9cbadc5 100644
--- a/sdk/lib/convert/html_escape.dart
+++ b/sdk/lib/convert/html_escape.dart
@@ -7,39 +7,121 @@ part of dart.convert;
 // TODO(floitsch) - Document - Issue 13097
 const HtmlEscape HTML_ESCAPE = const HtmlEscape();
 
+/**
+ * HTML escape modes.
+ *
+ * Allows specifying a mode for HTML escaping that depend on the context
+ * where the escaped result is going to be used.

[Søren Gjesse, 2015/03/19 14:09:33]

Maybe mention what the different contexts are.

[Lasse Reichstein Nielsen, 2015/03/20 08:47:04]

Done.

+ *
+ * All modes require escaping of `&` (ampersand) characters, and may
+ * enable escaping of more characters.
+ */
 class HtmlEscapeMode {
   final String _name;
+  /** Whether to escape '<' and '>'. */
   final bool escapeLtGt;
+  /** Whether to escape '"' (quote). */
   final bool escapeQuot;
+  /** Whether to escape "'" (apostrophe). */
   final bool escapeApos;
-  final bool escapeSlash;
 
-  // TODO(floitsch) - Document - Issue 13097
+  /**
+   * Default escaping mode which escape all characters.
+   *
+   * The result of such an escaping is usable both in element content and
+   * in any attribute value.
+   *
+   * The escaping only works for elements with normal HTML content,
+   * and not for, for example, script or style element content,
+   * which require escapes matching their particular content syntax.
+   */
   static const HtmlEscapeMode UNKNOWN =
-    const HtmlEscapeMode._('unknown', true, true, true, true);
-
-  // TODO(floitsch) - Document - Issue 13097
+      const HtmlEscapeMode._('unknown', true, true, true);
+
+  /**
+   * Escaping mode for text going into double-quoted HTML attribute values.
+   *
+   * The result should not be used as the content of an unquoted
+   * or single-quoted attribute value.
+   *
+   * Escapes only double quotes (`"`) but not single quotes (`'`).
+   */
   static const HtmlEscapeMode ATTRIBUTE =
-    const HtmlEscapeMode._('attribute', false, true, false, false);
-
-  // TODO(floitsch) - Document - Issue 13097
+      const HtmlEscapeMode._('attribute', false, true, false);
+
+  /**
+   * Escaping mode for text going into single-quoted HTML attribute values.
+   *
+   * The result should not be used as the content of an unquoted
+   * or double-quoted attribute value.
+   *
+   * Escapes only single quotes (`'`) but not double quotes (`"`).
+   */
+  static const HtmlEscapeMode SQ_ATTRIBUTE =
+      const HtmlEscapeMode._('attribute', false, false, true);
+
+  /**
+   * Escaping mode for text going into HTML element content.
+   *
+   * The escaping only works for elements with normal HTML content,
+   * and not for, for example, script or style element content,
+   * which require escapes matching their particular content syntax.
+   *
+   * Escapes `<` and `>` characters.
+   */
   static const HtmlEscapeMode ELEMENT =
-    const HtmlEscapeMode._('element', true, false, false, true);
-
-  // TODO(floitsch) - Document - Issue 13097
-  const HtmlEscapeMode._(this._name, this.escapeLtGt, this.escapeQuot,
-    this.escapeApos, this.escapeSlash);
+      const HtmlEscapeMode._('element', true, false, false);
+
+  const HtmlEscapeMode._(
+      this._name, this.escapeLtGt, this.escapeQuot, this.escapeApos);
+
+  /**
+   * Create a custom escaping mode.
+   *
+   * All modes escape `&`.
+   * The mode can further be set to escape `<` and `>` ([escapeLtGt]),
+   * `"` ([escapeQuot]) and/or `'` ([escapeApos]).
+   */
+  const HtmlEscapeMode({String name: "custom",
+                        this.escapeLtGt: false,
+                        this.escapeQuot: false,
+                        this.escapeApos: false}) : _name = name;
 
   String toString() => _name;
 }
 
-  // TODO(floitsch) - Document - Issue 13097
+/**
+ * Converter which escapes characters that means something when parsed as HTML.

[floitsch, 2015/03/19 14:10:39]

that have a meaning in HTML?

otherwise: "mean" (not "means")

[Lasse Reichstein Nielsen, 2015/03/20 08:47:04]

Rewritten.

+ *
+ * The converter finds characters that have meaning in HTML and replaces
+ * them with HTML corresponding entities.
+ *
+ * The characters that need escaping in HTML are:
+ *
+ * * `&` (ampersand) always need to be escaped.
+ * * `<` (less than) and '>' (greater than) when inside an element.
+ * * `"` (quote) when inside a double-quoted attribute value.
+ * * `'` (apostrophe) when inside a single-quoted attribute value.
+ *       Apostrophe is escaped as `&#39;` instead of `&apos;` since
+ *       not all browsers understand `&apos;`.
+ *
+ * Escaping `>` (greater than) isn't necessary, but the result is often
+ * found to be easier to read if greater-than is also escaped whenever
+ * less-than is.
+ */
 class HtmlEscape extends Converter<String, String> {
 
-  // TODO(floitsch) - Document - Issue 13097
+  /** The [HtmlEscapeMode] used by the converter. */
   final HtmlEscapeMode mode;
 
-  // TODO(floitsch) - Document - Issue 13097
+  /**
+   * Create converter that escapes HTML characters.
+   *
+   * If [mode] is provided as either [HtmlEscapeMode.ATTRIBUTE] or
+   * [HtmlEscapeMode.ELEMENT], only the corresponding subset of HTML
+   * characters are escaped.
+   * The default is to escape all HTML characters.
+   */
   const HtmlEscape([this.mode = HtmlEscapeMode.UNKNOWN]);
 
   String convert(String text) {
@@ -47,29 +129,34 @@ class HtmlEscape extends Converter<String, String> {
     return val == null ? text : val;
   }
 
+  /**
+   * Converts the substring of text from start to end.
+   *
+   * Returns `null` if no changes were necessary, otherwise returns
+   * the converted string.
+   */
   String _convert(String text, int start, int end) {
     StringBuffer result = null;
     for (int i = start; i < end; i++) {
       var ch = text[i];
-      String replace = null;
+      String replacement = null;
       switch (ch) {
-        case '&': replace = '&amp;'; break;
-        case '\u00A0'/*NO-BREAK SPACE*/: replace = '&nbsp;'; break;
-        case '"': if (mode.escapeQuot) replace = '&quot;'; break;
-        case "'": if (mode.escapeApos) replace = '&#x27;'; break;
-        case '<': if (mode.escapeLtGt) replace = '&lt;'; break;
-        case '>': if (mode.escapeLtGt) replace = '&gt;'; break;
-        case '/': if (mode.escapeSlash) replace = '&#x2F;'; break;
+        case '&': replacement = '&amp;'; break;
+        case '"': if (mode.escapeQuot) replacement = '&quot;'; break;
+        case "'": if (mode.escapeApos) replacement = '&#39;'; break;

[Søren Gjesse, 2015/03/19 14:09:33]

Maybe add a comment why ' is not used.

[Lasse Reichstein Nielsen, 2015/03/20 08:47:05]

It's in the documentation for the class.

+        case '<': if (mode.escapeLtGt) replacement = '&lt;'; break;
+        case '>': if (mode.escapeLtGt) replacement = '&gt;'; break;
       }
-      if (replace != null) {
-        if (result == null) result = new StringBuffer(text.substring(start, i));
-        result.write(replace);
-      } else if (result != null) {
-        result.write(ch);
+      if (replacement != null) {
+        if (result == null) result = new StringBuffer();
+        if (i > start) result.write(text.substring(start, i));
+        result.write(replacement);
+        start = i + 1;
       }
     }
-
-    return result != null ? result.toString() : null;
+    if (result == null) return null;
+    if (end > start) result.write(text.substring(start, end));
+    return result.toString();
   }
 
   StringConversionSink startChunkedConversion(Sink<String> sink) {