Fix String#escapeHTML and String#unescapeHTML (to ensure no memory le…

…akage and more consistent output). [#47:resolved]
prototypejs · May 19, 2008 · 57a901f · 57a901f
1 parent 7a1ee73
commit 57a901f
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 14 deletions.
diff --git a/src/string.js b/src/string.js
@@ -83,14 +83,16 @@ Object.extend(String.prototype, {
   escapeHTML: function() {
     var self = arguments.callee;
     self.text.data = this;
-    return self.div.innerHTML;
+    return self.container.innerHTML;
   },
 
   unescapeHTML: function() {
     var div = new Element('div');
-    div.innerHTML = this.stripTags();
+    // Safari requires the text nested inside another element to render correctly
+    div.innerHTML = '<pre>' + this.stripTags() + '</pre>';
+    div = div.firstChild;
     return div.childNodes[0] ? (div.childNodes.length > 1 ? 
-      $A(div.childNodes).inject('', function(memo, node) { return memo+node.nodeValue }) : 
+      $A(div.childNodes).inject('', function(memo, node) { return memo + node.nodeValue }) : 
       div.childNodes[0].nodeValue) : '';
   },
 
@@ -211,15 +213,6 @@ Object.extend(String.prototype, {
   }
 });
 
-if (Prototype.Browser.WebKit || Prototype.Browser.IE) Object.extend(String.prototype, {
-  escapeHTML: function() {
-    return this.replace(/&/g,'&amp;').replace(/</g,'&lt;').replace(/>/g,'&gt;');
-  },
-  unescapeHTML: function() {
-    return this.stripTags().replace(/&amp;/g,'&').replace(/&lt;/g,'<').replace(/&gt;/g,'>');
-  }
-});
-
 String.prototype.gsub.prepareReplacement = function(replacement) {
   if (Object.isFunction(replacement)) return replacement;
   var template = new Template(replacement);
@@ -229,11 +222,39 @@ String.prototype.gsub.prepareReplacement = function(replacement) {
 String.prototype.parseQuery = String.prototype.toQueryParams;
 
 Object.extend(String.prototype.escapeHTML, {
-  div:  document.createElement('div'),
+  container: document.createElement('pre'),
   text: document.createTextNode('')
 });
 
-String.prototype.escapeHTML.div.appendChild(String.prototype.escapeHTML.text);
+String.prototype.escapeHTML.container.appendChild(String.prototype.escapeHTML.text);
+
+if (Prototype.Browser.IE)
+  // IE converts all newlines to carriage returns so we swap them back
+  String.prototype.unescapeHTML = String.prototype.unescapeHTML.wrap(function(proceed) {
+    return proceed().replace(/\r/g, '\n')
+  });
+
+if (Prototype.Browser.WebKit && Prototype.BrowserFeatures.SelectorsAPI)
+  // Safari 3.x has issues with escaping the ">" character
+  (function() {
+    var escapeHTML = String.prototype.escapeHTML;
+    Object.extend(
+      String.prototype.escapeHTML = escapeHTML.wrap(function(proceed) {
+        return proceed().replace(/>/g, "&gt;")
+      }), {
+      container: escapeHTML.container,
+      text: escapeHTML.text
+    })
+  })();
+
+if ('&'.escapeHTML() !== '&amp;') {
+  // Safari 2.x has issues with escaping html inside a "pre" element so we use the deprecated "xmp" element instead
+  Object.extend(String.prototype.escapeHTML, {
+    container: document.createElement('xmp'),
+    text: document.createTextNode('')
+  });
+  String.prototype.escapeHTML.container.appendChild(String.prototype.escapeHTML.text);
+}
 
 var Template = Class.create({
   initialize: function(template, pattern) {

diff --git a/test/unit/string_test.js b/test/unit/string_test.js
@@ -255,9 +255,13 @@ new Test.Unit.Runner({
 
     this.assertEqual(largeTextUnescaped, largeTextEscaped.unescapeHTML());
 
+    this.assertEqual('test \xfa', 'test &uacute;'.unescapeHTML());
     this.assertEqual('1\n2', '1\n2'.unescapeHTML());
     this.assertEqual('Pride & Prejudice', '<h1>Pride &amp; Prejudice</h1>'.unescapeHTML());
 
+    var escapedTest = '"&lt;" means "<" in HTML';
+    this.assertEqual(escapedTest, escapedTest.escapeHTML().unescapeHTML());
+
     this.benchmark(function() { largeTextEscaped.unescapeHTML() }, 1000);
 
   },