Bug 723431 - DOMTemplate should allow customisation of display of null/undefined values; r=dcamp
authorJoe Walker <jwalker@mozilla.com>
Wed, 25 Apr 2012 09:55:07 +0100
changeset 92594 be802d70aa5f9dcd05ac895feba7ee24c0c38c61
parent 92593 a974c45914eba0eed284cff94f20031a3cc1fd01
child 92595 86623a74f82acc9e026c4b0ade638efad00b35f6
push id22545
push usertim.taubert@gmx.de
push dateFri, 27 Apr 2012 12:45:03 +0000
treeherdermozilla-central@0f8ea3826bf7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdcamp
bugs723431, 736831
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 723431 - DOMTemplate should allow customisation of display of null/undefined values; r=dcamp * * * Bug 736831 - DOMTemplate should allow '_' to be part of a valid property name; r=dcamp
browser/devtools/shared/Templater.jsm
browser/devtools/shared/test/browser_templater_basic.js
--- a/browser/devtools/shared/Templater.jsm
+++ b/browser/devtools/shared/Templater.jsm
@@ -37,43 +37,64 @@
  * ***** END LICENSE BLOCK ***** */
 
 
 var EXPORTED_SYMBOLS = [ "Templater", "template" ];
 
 Components.utils.import("resource://gre/modules/Services.jsm");
 const Node = Components.interfaces.nsIDOMNode;
 
+/**
+ * For full documentation, see:
+ * https://github.com/mozilla/domtemplate/blob/master/README.md
+ */
+
 // WARNING: do not 'use_strict' without reading the notes in _envEval();
 
 /**
  * Begin a new templating process.
  * @param node A DOM element or string referring to an element's id
  * @param data Data to use in filling out the template
  * @param options Options to customize the template processing. One of:
  * - allowEval: boolean (default false) Basic template interpolations are
- * either property paths (e.g. ${a.b.c.d}), however if allowEval=true then we
- * allow arbitrary JavaScript
+ *   either property paths (e.g. ${a.b.c.d}), or if allowEval=true then we
+ *   allow arbitrary JavaScript
+ * - stack: string or array of strings (default empty array) The template
+ *   engine maintains a stack of tasks to help debug where it is. This allows
+ *   this stack to be prefixed with a template name
+ * - blankNullUndefined: By default DOMTemplate exports null and undefined
+ *   values using the strings 'null' and 'undefined', which can be helpful for
+ *   debugging, but can introduce unnecessary extra logic in a template to
+ *   convert null/undefined to ''. By setting blankNullUndefined:true, this
+ *   conversion is handled by DOMTemplate
  */
 function template(node, data, options) {
   var template = new Templater(options || {});
   template.processNode(node, data);
   return template;
 }
 
 /**
  * Construct a Templater object. Use template() in preference to this ctor.
  * @deprecated Use template(node, data, options);
  */
 function Templater(options) {
   if (options == null) {
     options = { allowEval: true };
   }
   this.options = options;
-  this.stack = [];
+  if (options.stack && Array.isArray(options.stack)) {
+    this.stack = options.stack;
+  }
+  else if (typeof options.stack === 'string') {
+    this.stack = [ options.stack ];
+  }
+  else {
+    this.stack = [];
+  }
 }
 
 /**
  * Cached regex used to find ${...} sections in some text.
  * Performance note: This regex uses ( and ) to capture the 'script' for
  * further processing. Not all of the uses of this regex use this feature so
  * if use of the capturing group is a performance drain then we should split
  * this regex in two.
@@ -85,17 +106,17 @@ Templater.prototype._templateRegion = /\
  * See Templater._processTextNode() for details.
  */
 Templater.prototype._splitSpecial = /\uF001|\uF002/;
 
 /**
  * Cached regex used to detect if a script is capable of being interpreted
  * using Template._property() or if we need to use Template._envEval()
  */
-Templater.prototype._isPropertyScript = /^[a-zA-Z0-9.]*$/;
+Templater.prototype._isPropertyScript = /^[_a-zA-Z0-9.]*$/;
 
 /**
  * Recursive function to walk the tree processing the attributes as it goes.
  * @param node the node to process. If you pass a string in instead of a DOM
  * element, it is assumed to be an id for use with document.getElementById()
  * @param data the data to use for node processing.
  */
 Templater.prototype.processNode = function(node, data) {
@@ -148,17 +169,21 @@ Templater.prototype.processNode = functi
             var capture = node.hasAttribute('capture' + name.substring(2));
             node.addEventListener(name.substring(2), func, capture);
             if (capture) {
               node.removeAttribute('capture' + name.substring(2));
             }
           } else {
             // Replace references in all other attributes
             var newValue = value.replace(this._templateRegion, function(path) {
-              return this._envEval(path.slice(2, -1), data, value);
+              var insert = this._envEval(path.slice(2, -1), data, value);
+              if (this.options.blankNullUndefined && insert == null) {
+                insert = '';
+              }
+              return insert;
             }.bind(this));
             // Remove '_' prefix of attribute names so the DOM won't try
             // to use them before we've processed the template
             if (name.charAt(0) === '_') {
               node.removeAttribute(name);
               node.setAttribute(name.substring(1), newValue);
             } else if (value !== newValue) {
               attrs[i].value = newValue;
@@ -172,17 +197,17 @@ Templater.prototype.processNode = functi
 
     // Loop through our children calling processNode. First clone them, so the
     // set of nodes that we visit will be unaffected by additions or removals.
     var childNodes = Array.prototype.slice.call(node.childNodes);
     for (var j = 0; j < childNodes.length; j++) {
       this.processNode(childNodes[j], data);
     }
 
-    if (node.nodeType === Node.TEXT_NODE) {
+    if (node.nodeType === 3 /*Node.TEXT_NODE*/) {
       this._processTextNode(node, data);
     }
   } finally {
     delete data.__element;
     this.stack.pop();
   }
 };
 
@@ -342,40 +367,51 @@ Templater.prototype._processTextNode = f
     parts.forEach(function(part) {
       if (part === null || part === undefined || part === '') {
         return;
       }
       if (part.charAt(0) === '$') {
         part = this._envEval(part.slice(1), data, node.data);
       }
       this._handleAsync(part, node, function(reply, siblingNode) {
-        reply = this._toNode(reply, siblingNode.ownerDocument);
-        siblingNode.parentNode.insertBefore(reply, siblingNode);
+        var doc = siblingNode.ownerDocument;
+        if (reply == null) {
+          reply = this.options.blankNullUndefined ? '' : '' + reply;
+        }
+        if (typeof reply.cloneNode === 'function') {
+          // i.e. if (reply instanceof Element) { ...
+          reply = this._maybeImportNode(reply, doc);
+          siblingNode.parentNode.insertBefore(reply, siblingNode);
+        } else if (typeof reply.item === 'function' && reply.length) {
+          // if thing is a NodeList, then import the children
+          for (var i = 0; i < reply.length; i++) {
+            var child = this._maybeImportNode(reply.item(i), doc);
+            siblingNode.parentNode.insertBefore(child, siblingNode);
+          }
+        }
+        else {
+          // if thing isn't a DOM element then wrap its string value in one
+          reply = doc.createTextNode(reply.toString());
+          siblingNode.parentNode.insertBefore(reply, siblingNode);
+        }
+
       }.bind(this));
     }, this);
     node.parentNode.removeChild(node);
   }
 };
 
 /**
- * Helper to convert a 'thing' to a DOM Node.
- * This is (obviously) a no-op for DOM Elements (which are detected using
- * 'typeof thing.cloneNode !== "function"' (is there a better way that will
- * work in all environments, including a .jsm?)
- * Non DOM elements are converted to a string and wrapped in a TextNode.
+ * Return node or a import of node, if it's not in the given document
+ * @param node The node that we want to be properly owned
+ * @param doc The document that the given node should belong to
+ * @return A node that belongs to the given document
  */
-Templater.prototype._toNode = function(thing, document) {
-  if (thing == null) {
-    thing = '' + thing;
-  }
-  // if thing isn't a DOM element then wrap its string value in one
-  if (typeof thing.cloneNode !== 'function') {
-    thing = document.createTextNode(thing.toString());
-  }
-  return thing;
+Templater.prototype._maybeImportNode = function(node, doc) {
+  return node.ownerDocument === doc ? node : doc.importNode(node, true);
 };
 
 /**
  * A function to handle the fact that some nodes can be promises, so we check
  * and resolve if needed using a marker node to keep our place before calling
  * an inserter function.
  * @param thing The object which could be real data or a promise of real data
  * we use it directly if it's not a promise, or resolve it if it is.
@@ -424,38 +460,38 @@ Templater.prototype._stripBraces = funct
  * a string to be cut into an array using <tt>split('.')</tt>
  * @param data the data to use for node processing
  * @param newValue (optional) If defined, this value will replace the
  * original value for the data at the path specified.
  * @return The value pointed to by <tt>path</tt> before any
  * <tt>newValue</tt> is applied.
  */
 Templater.prototype._property = function(path, data, newValue) {
-  this.stack.push(path);
   try {
     if (typeof path === 'string') {
       path = path.split('.');
     }
     var value = data[path[0]];
     if (path.length === 1) {
       if (newValue !== undefined) {
         data[path[0]] = newValue;
       }
       if (typeof value === 'function') {
         return value.bind(data);
       }
       return value;
     }
     if (!value) {
-      this._handleError('Can\'t find path=' + path);
+      this._handleError('"' + path[0] + '" is undefined');
       return null;
     }
     return this._property(path.slice(1), value, newValue);
-  } finally {
-    this.stack.pop();
+  } catch (ex) {
+    this._handleError('Path error with \'' + path + '\'', ex);
+    return '${' + path + '}';
   }
 };
 
 /**
  * Like eval, but that creates a context of the variables in <tt>env</tt> in
  * which the script is evaluated.
  * WARNING: This script uses 'with' which is generally regarded to be evil.
  * The alternative is to create a Function at runtime that takes X parameters
@@ -464,47 +500,45 @@ Templater.prototype._property = function
  * @param script The string to be evaluated.
  * @param data The environment in which to eval the script.
  * @param frame Optional debugging string in case of failure.
  * @return The return value of the script, or the error message if the script
  * execution failed.
  */
 Templater.prototype._envEval = function(script, data, frame) {
   try {
-    this.stack.push(frame);
+    this.stack.push(frame.replace(/\s+/g, ' '));
     if (this._isPropertyScript.test(script)) {
       return this._property(script, data);
     } else {
       if (!this.options.allowEval) {
         this._handleError('allowEval is not set, however \'' + script + '\'' +
             ' can not be resolved using a simple property path.');
         return '${' + script + '}';
       }
       with (data) {
         return eval(script);
       }
     }
   } catch (ex) {
-    this._handleError('Template error evaluating \'' + script + '\'' +
-        ' environment=' + Object.keys(data).join(', '), ex);
+    this._handleError('Template error evaluating \'' + script + '\'', ex);
     return '${' + script + '}';
   } finally {
     this.stack.pop();
   }
 };
 
 /**
  * A generic way of reporting errors, for easy overloading in different
  * environments.
  * @param message the error message to report.
  * @param ex optional associated exception.
  */
 Templater.prototype._handleError = function(message, ex) {
-  this._logError(message);
-  this._logError('In: ' + this.stack.join(' > '));
+  this._logError(message + ' (In: ' + this.stack.join(' > ') + ')');
   if (ex) {
     this._logError(ex);
   }
 };
 
 
 /**
  * A generic way of reporting errors, for easy overloading in different
--- a/browser/devtools/shared/test/browser_templater_basic.js
+++ b/browser/devtools/shared/test/browser_templater_basic.js
@@ -1,18 +1,22 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Tests that the DOM Template engine works properly
 
-let tempScope = {};
-Cu.import("resource:///modules/devtools/Templater.jsm", tempScope);
-Cu.import("resource:///modules/devtools/Promise.jsm", tempScope);
-let template = tempScope.template;
-let Promise = tempScope.Promise;
+/*
+ * These tests run both in Mozilla/Mochitest and plain browsers (as does
+ * domtemplate)
+ * We should endevour to keep the source in sync.
+ */
+
+var imports = {};
+Cu.import("resource:///modules/devtools/Templater.jsm", imports);
+Cu.import("resource:///modules/devtools/Promise.jsm", imports);
 
 function test() {
   addTab("http://example.com/browser/browser/devtools/shared/test/browser_templater_basic.html", function() {
     info("Starting DOM Templater Tests");
     runTest(0);
   });
 }
 
@@ -20,17 +24,17 @@ function runTest(index) {
   var options = tests[index] = tests[index]();
   var holder = content.document.createElement('div');
   holder.id = options.name;
   var body = content.document.body;
   body.appendChild(holder);
   holder.innerHTML = options.template;
 
   info('Running ' + options.name);
-  template(holder, options.data, options.options);
+  imports.template(holder, options.data, options.options);
 
   if (typeof options.result == 'string') {
     is(holder.innerHTML, options.result, options.name);
   }
   else {
     ok(holder.innerHTML.match(options.result), options.name);
   }
 
@@ -233,18 +237,50 @@ var tests = [
     options: { allowEval: true },
     result: '<p>2</p>'
   };},
 
   function() { return {
     name: 'propertyFail',
     template: '<p>${Math.max(1, 2)}</p>',
     result: '<p>${Math.max(1, 2)}</p>'
+  };},
+
+  // Bug 723431: DOMTemplate should allow customisation of display of
+  // null/undefined values
+  function() { return {
+    name: 'propertyUndefAttrFull',
+    template: '<p>${nullvar}|${undefinedvar1}|${undefinedvar2}</p>',
+    data: { nullvar: null, undefinedvar1: undefined },
+    result: '<p>null|undefined|undefined</p>'
+  };},
+
+  function() { return {
+    name: 'propertyUndefAttrBlank',
+    template: '<p>${nullvar}|${undefinedvar1}|${undefinedvar2}</p>',
+    data: { nullvar: null, undefinedvar1: undefined },
+    options: { blankNullUndefined: true },
+    result: '<p>||</p>'
+  };},
+
+  function() { return {
+    name: 'propertyUndefAttrFull',
+    template: '<div><p value="${nullvar}"></p><p value="${undefinedvar1}"></p><p value="${undefinedvar2}"></p></div>',
+    data: { nullvar: null, undefinedvar1: undefined },
+    result: '<div><p value="null"></p><p value="undefined"></p><p value="undefined"></p></div>'
+  };},
+
+  function() { return {
+    name: 'propertyUndefAttrBlank',
+    template: '<div><p value="${nullvar}"></p><p value="${undefinedvar1}"></p><p value="${undefinedvar2}"></p></div>',
+    data: { nullvar: null, undefinedvar1: undefined },
+    options: { blankNullUndefined: true },
+    result: '<div><p value=""></p><p value=""></p><p value=""></p></div>'
   };}
 ];
 
 function delayReply(data) {
-  var p = new Promise();
+  var p = new imports.Promise();
   executeSoon(function() {
     p.resolve(data);
   });
   return p;
 }