Bug 996691 - Stop ignoring empty string property names when inspecting an object. r=fitzgen
authorOriol <oriol-bugzilla@hotmail.com>
Sun, 21 Aug 2016 18:47:00 -0400
changeset 336334 3e537d8eb88c440fd0b7aa88deec56aa688715b0
parent 336333 b2947c489cc7d1536ce1094609c4bd19d4aea1be
child 336335 ed793a550dd602631e25febe9fc39f3bab41e80a
push id10033
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:50:26 +0000
treeherdermozilla-aurora@5dddbefdf759 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs996691
milestone51.0a1
Bug 996691 - Stop ignoring empty string property names when inspecting an object. r=fitzgen
devtools/client/debugger/test/mochitest/browser_dbg_variables-view-08.js
devtools/client/debugger/test/mochitest/browser_dbg_variables-view-data.js
devtools/client/debugger/test/mochitest/doc_whitespace-property-names.html
devtools/client/shared/widgets/VariablesView.jsm
devtools/client/shared/widgets/VariablesViewController.jsm
devtools/client/webconsole/test/browser_jsterm_inspect.js
--- a/devtools/client/debugger/test/mochitest/browser_dbg_variables-view-08.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_variables-view-08.js
@@ -35,17 +35,17 @@ var test = Task.async(function* () {
   }
   ok(obj, "Should have found the 'obj' variable");
 
   info("Expanding variable 'obj'");
   let expanded = once(variables, "fetched");
   obj.expand();
   yield expanded;
 
-  let values = [" ", "\r", "\n", "\t", "\f", "\uFEFF", "\xA0"];
+  let values = ["", " ", "\r", "\n", "\t", "\f", "\uFEFF", "\xA0"];
   let count = values.length;
 
   for (let [property, value] of obj) {
     let index = values.indexOf(property);
     if (index >= 0) {
       --count;
       is(value._nameString, property,
          "The _nameString is different than the property name");
--- a/devtools/client/debugger/test/mochitest/browser_dbg_variables-view-data.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_variables-view-data.js
@@ -84,17 +84,17 @@ function performTest() {
   closeDebuggerAndFinish(gPanel);
 }
 
 function testHierarchy() {
   is(gVariablesView._currHierarchy.size, 13,
     "There should be 1 scope, 1 var, 1 proto, 8 props, 1 getter and 1 setter.");
 
   gScope = gVariablesView._currHierarchy.get("");
-  gVariable = gVariablesView._currHierarchy.get("[\"\"]");
+  gVariable = gVariablesView._currHierarchy.get("[]");
 
   is(gVariablesView._store.length, 1,
     "There should be only one scope in the view.");
   is(gScope._store.size, 1,
     "There should be only one variable in the scope.");
   is(gVariable._store.size, 9,
     "There should be 1 __proto__ and 8 properties in the variable.");
 }
--- a/devtools/client/debugger/test/mochitest/doc_whitespace-property-names.html
+++ b/devtools/client/debugger/test/mochitest/doc_whitespace-property-names.html
@@ -5,25 +5,25 @@
 <html>
   <head>
     <meta charset="utf-8"/>
     <title>Debugger + Whitespace property name test page</title>
   </head>
 
   <body>
     <script>
-    window.obj = {
-      " ": 0,
-      "\r": 1,
-      "\n": 2,
-      "\t": 3,
-      "\f": 4,
-      "\uFEFF": 5,
-      "\xA0": 6
-    };
     window.doPause = function () {
-      var obj = window.obj;
+      var obj = {
+        "": 0,
+        " ": 1,
+        "\r": 2,
+        "\n": 3,
+        "\t": 4,
+        "\f": 5,
+        "\uFEFF": 6,
+        "\xA0": 7
+      };
       debugger;
     };
     </script>
   </body>
 
 </html>
--- a/devtools/client/shared/widgets/VariablesView.jsm
+++ b/devtools/client/shared/widgets/VariablesView.jsm
@@ -112,17 +112,17 @@ VariablesView.prototype = {
    *
    * @param object aObject
    *        The raw object to display. You can only provide this object
    *        if you want the variables view to work in sync mode.
    */
   set rawObject(aObject) {
     this.empty();
     this.addScope()
-        .addItem("", { enumerable: true })
+        .addItem(undefined, { enumerable: true })
         .populate(aObject, { sorted: true });
   },
 
   /**
    * Adds a scope to contain any inspected variables.
    *
    * This new scope will be considered the parent of any other scope
    * added afterwards.
@@ -556,17 +556,17 @@ VariablesView.prototype = {
    * just unhidden.
    *
    * @param string aToken
    *        The variable or property to search for.
    */
   _doSearch: function (aToken) {
     if (this.controller && this.controller.supportsSearch()) {
       // Retrieve the main Scope in which we add attributes
-      let scope = this._store[0]._store.get("");
+      let scope = this._store[0]._store.get(undefined);
       if (!aToken) {
         // Prune the view from old previous content
         // so that we delete the intermediate search results
         // we created in previous searches
         for (let property of scope._store.values()) {
           property.remove();
         }
       }
@@ -1108,17 +1108,17 @@ VariablesView.simpleValueEvalMacro = fun
  * @param string aCurrentString
  *        The trimmed user inputted string.
  * @param string aPrefix [optional]
  *        Prefix for the symbolic name.
  * @return string
  *         The string to be evaluated.
  */
 VariablesView.overrideValueEvalMacro = function (aItem, aCurrentString, aPrefix = "") {
-  let property = "\"" + aItem._nameString + "\"";
+  let property = escapeString(aItem._nameString);
   let parent = aPrefix + aItem.ownerView.symbolicName || "this";
 
   return "Object.defineProperty(" + parent + "," + property + "," +
     "{ value: " + aCurrentString +
     ", enumerable: " + parent + ".propertyIsEnumerable(" + property + ")" +
     ", configurable: true" +
     ", writable: true" +
     "})";
@@ -1135,17 +1135,17 @@ VariablesView.overrideValueEvalMacro = f
  *        Prefix for the symbolic name.
  * @return string
  *         The string to be evaluated.
  */
 VariablesView.getterOrSetterEvalMacro = function (aItem, aCurrentString, aPrefix = "") {
   let type = aItem._nameString;
   let propertyObject = aItem.ownerView;
   let parentObject = propertyObject.ownerView;
-  let property = "\"" + propertyObject._nameString + "\"";
+  let property = escapeString(propertyObject._nameString);
   let parent = aPrefix + parentObject.symbolicName || "this";
 
   switch (aCurrentString) {
     case "":
     case "null":
     case "undefined":
       let mirrorType = type == "get" ? "set" : "get";
       let mirrorLookup = type == "get" ? "__lookupSetter__" : "__lookupGetter__";
@@ -1320,27 +1320,27 @@ Scope.prototype = {
    *        * boolean internalItem  true if the item is internally generated.
    *                           This is used for special variables
    *                           like <return> or <exception> and distinguishes
    *                           them from ordinary properties that happen
    *                           to have the same name
    * @return Variable
    *         The newly created Variable instance, null if it already exists.
    */
-  addItem: function (aName = "", aDescriptor = {}, aOptions = {}) {
+  addItem: function (aName, aDescriptor = {}, aOptions = {}) {
     let {relaxed} = aOptions;
     if (this._store.has(aName) && !relaxed) {
       return this._store.get(aName);
     }
 
     let child = this._createChild(aName, aDescriptor, aOptions);
     this._store.set(aName, child);
     this._variablesView._itemsByElement.set(child._target, child);
     this._variablesView._currHierarchy.set(child.absoluteName, child);
-    child.header = !!aName;
+    child.header = aName !== undefined;
 
     return child;
   },
 
   /**
    * Adds items for this variable.
    *
    * @param object aItems
@@ -1806,17 +1806,17 @@ Scope.prototype = {
    *
    * @param string aName
    *        The scope's name.
    * @param string aTargetClassName
    *        A custom class name for this scope's target element.
    * @param string aTitleClassName [optional]
    *        A custom class name for this scope's title element.
    */
-  _displayScope: function (aName, aTargetClassName, aTitleClassName = "") {
+  _displayScope: function (aName = "", aTargetClassName, aTitleClassName = "") {
     let document = this.document;
 
     let element = this._target = document.createElement("vbox");
     element.id = this._idString;
     element.className = aTargetClassName;
 
     let arrow = this._arrow = document.createElement("hbox");
     arrow.className = "arrow theme-twisty";
@@ -2355,29 +2355,29 @@ Variable.prototype = Heritage.extend(Sco
 
   /**
    * Gets this variable's path to the topmost scope in the form of a string
    * meant for use via eval() or a similar approach.
    * For example, a symbolic name may look like "arguments['0']['foo']['bar']".
    * @return string
    */
   get symbolicName() {
-    return this._nameString;
+    return this._nameString || "";
   },
 
   /**
    * Gets full path to this variable, including name of the scope.
    * @return string
    */
   get absoluteName() {
     if (this._absoluteName) {
       return this._absoluteName;
     }
 
-    this._absoluteName = this.ownerView._nameString + "[\"" + this._nameString + "\"]";
+    this._absoluteName = this.ownerView._nameString + "[" + escapeString(this._nameString) + "]";
     return this._absoluteName;
   },
 
   /**
    * Gets this variable's symbolic path to the topmost scope.
    * @return array
    * @see Variable._buildSymbolicPath
    */
@@ -2444,17 +2444,17 @@ Variable.prototype = Heritage.extend(Sco
    *             - "nasu"
    *             - { type: "undefined" }
    *             - { type: "null" }
    *             - { type: "object", class: "Object" }
    */
   setGrip: function (aGrip) {
     // Don't allow displaying grip information if there's no name available
     // or the grip is malformed.
-    if (!this._nameString || aGrip === undefined || aGrip === null) {
+    if (this._nameString === undefined || aGrip === undefined || aGrip === null) {
       return;
     }
     // Getters and setters should display grip information in sub-properties.
     if (this.getter || this.setter) {
       return;
     }
 
     let prevGrip = this._valueGrip;
@@ -3091,30 +3091,30 @@ Property.prototype = Heritage.extend(Var
    * @see Variable.symbolicName
    * @return string
    */
   get symbolicName() {
     if (this._symbolicName) {
       return this._symbolicName;
     }
 
-    this._symbolicName = this.ownerView.symbolicName + "[\"" + this._nameString + "\"]";
+    this._symbolicName = this.ownerView.symbolicName + "[" + escapeString(this._nameString) + "]";
     return this._symbolicName;
   },
 
   /**
    * @see Variable.absoluteName
    * @return string
    */
   get absoluteName() {
     if (this._absoluteName) {
       return this._absoluteName;
     }
 
-    this._absoluteName = this.ownerView.absoluteName + "[\"" + this._nameString + "\"]";
+    this._absoluteName = this.ownerView.absoluteName + "[" + escapeString(this._nameString) + "]";
     return this._absoluteName;
   }
 });
 
 /**
  * A generator-iterator over the VariablesView, Scopes, Variables and Properties.
  */
 VariablesView.prototype[Symbol.iterator] =
@@ -3905,16 +3905,27 @@ VariablesView.getClass = function (aGrip
 var generateId = (function () {
   let count = 0;
   return function (aName = "") {
     return aName.toLowerCase().trim().replace(/\s+/g, "-") + (++count);
   };
 })();
 
 /**
+ * Serialize a string to JSON. The result can be inserted in a string evaluated by `eval`.
+ *
+ * @param string aString
+ *       The string to be escaped. If undefined, the function returns the empty string.
+ * @return string
+ */
+function escapeString(aString) {
+  return JSON.stringify(aString) || "";
+}
+
+/**
  * Escape some HTML special characters. We do not need full HTML serialization
  * here, we just want to make strings safe to display in HTML attributes, for
  * the stringifiers.
  *
  * @param string aString
  * @return string
  */
 function escapeHTML(aString) {
--- a/devtools/client/shared/widgets/VariablesViewController.jsm
+++ b/devtools/client/shared/widgets/VariablesViewController.jsm
@@ -338,17 +338,17 @@ VariablesViewController.prototype = {
         aTarget.addItem("<handler>", { value: aGrip.proxyHandler }, { internalItem: true }),
         aGrip.proxyHandler);
 
       // Refuse to play the proxy's stupid game and return immediately
       let deferred = defer();
       deferred.resolve();
       return deferred.promise;
     }
-    
+
     if (aGrip.class === "Promise" && aGrip.promiseState) {
       const { state, value, reason } = aGrip.promiseState;
       aTarget.addItem("<state>", { value: state }, { internalItem: true });
       if (state === "fulfilled") {
         this.addExpander(
           aTarget.addItem("<value>", { value }, { internalItem: true }),
           value);
       } else if (state === "rejected") {
@@ -757,17 +757,17 @@ VariablesViewController.prototype = {
   setSingleVariable: function (options, configuration = {}) {
     this._setEvaluationMacros(configuration);
     this.view.empty();
 
     let scope = this.view.addScope(options.label);
     scope.expanded = true; // Expand the scope by default.
     scope.locked = true; // Prevent collapsing the scope.
 
-    let variable = scope.addItem("", { enumerable: true });
+    let variable = scope.addItem(undefined, { enumerable: true });
     let populated;
 
     if (options.objectActor) {
       // Save objectActor for properties filtering
       this.objectActor = options.objectActor;
       if (VariablesView.isPrimitive({ value: this.objectActor })) {
         populated = promise.resolve();
       } else {
--- a/devtools/client/webconsole/test/browser_jsterm_inspect.js
+++ b/devtools/client/webconsole/test/browser_jsterm_inspect.js
@@ -20,17 +20,17 @@ add_task(function* () {
 
   let updated = jsterm.once("variablesview-updated");
   jsterm.execute("inspect(window)");
   let view = yield updated;
   ok(view, "variables view object");
 
   // The single variable view contains a scope with the variable name
   // and unnamed subitem that contains the properties
-  let variable = view.getScopeAtIndex(0).get("");
+  let variable = view.getScopeAtIndex(0).get(undefined);
   ok(variable, "variable object");
 
   yield findVariableViewProperties(variable, [
     { name: "testProp", value: "testValue" },
     { name: "document", value: /HTMLDocument \u2192 data:/ },
   ], { webconsole: hud });
 
   /* Check that a primitive value can be inspected, too */