Bug 1084430 - fix styling of <return> and <exception> in variable view; r=vporof

devtools/client/debugger/debugger-controller.js

@@ -973,12 +973,15 @@ StackFrames.prototype = {
   _insertScopeFrameReferences: function(aScope, aFrame) {
     // Add any thrown exception.
     if (this._currentException) {
-      let excRef = aScope.addItem("<exception>", { value: this._currentException });
+      let excRef = aScope.addItem("<exception>", { value: this._currentException },
+                                  { internalItem: true });
       DebuggerView.Variables.controller.addExpander(excRef, this._currentException);
     }
     // Add any returned value.
     if (this._currentReturnedValue) {
-      let retRef = aScope.addItem("<return>", { value: this._currentReturnedValue });
+      let retRef = aScope.addItem("<return>",
+                                  { value: this._currentReturnedValue },
+                                  { internalItem: true });
       DebuggerView.Variables.controller.addExpander(retRef, this._currentReturnedValue);
     }
     // Add "this".

devtools/client/debugger/test/mochitest/browser_dbg_step-out.js

@@ -31,6 +31,9 @@ function testNormalReturn() {
         "Should have the right property name for the returned value.");
       is(returnVar.value, 10,
         "Should have the right property value for the returned value.");
+      ok(returnVar._internalItem, "Should be an internal item");
+      ok(returnVar._target.hasAttribute("pseudo-item"),
+         "Element should be marked as a pseudo-item")
 
       resumeDebuggee().then(() => testReturnWithException());
     });
@@ -53,6 +56,9 @@ function testReturnWithException() {
         "Should have the right property name for the returned value.");
       is(exceptionVar.value, "boom",
         "Should have the right property value for the returned value.");
+      ok(exceptionVar._internalItem, "Should be an internal item");
+      ok(exceptionVar._target.hasAttribute("pseudo-item"),
+         "Element should be marked as a pseudo-item")
 
       resumeDebuggee().then(() => closeDebuggerAndFinish(gPanel));
     });

devtools/client/netmonitor/netmonitor-view.js

@@ -2752,7 +2752,7 @@ NetworkDetailsView.prototype = {
     headersScope.expanded = true;
 
     for (let header of aResponse.headers) {
-      let headerVar = headersScope.addItem(header.name, {}, true);
+      let headerVar = headersScope.addItem(header.name, {}, {relaxed: true});
       let headerValue = yield gNetwork.getString(header.value);
       headerVar.setGrip(headerValue);
     }
@@ -2802,7 +2802,7 @@ NetworkDetailsView.prototype = {
     cookiesScope.expanded = true;
 
     for (let cookie of aResponse.cookies) {
-      let cookieVar = cookiesScope.addItem(cookie.name, {}, true);
+      let cookieVar = cookiesScope.addItem(cookie.name, {}, {relaxed: true});
       let cookieValue = yield gNetwork.getString(cookie.value);
       cookieVar.setGrip(cookieValue);
 
@@ -2916,7 +2916,7 @@ NetworkDetailsView.prototype = {
     paramsScope.expanded = true;
 
     for (let param of paramsArray) {
-      let paramVar = paramsScope.addItem(param.name, {}, true);
+      let paramVar = paramsScope.addItem(param.name, {}, {relaxed: true});
       paramVar.setGrip(param.value);
     }
   },

devtools/client/shared/widgets/VariablesView.jsm

@@ -1278,11 +1278,13 @@ Scope.prototype = {
    *        The name of the new Property.
    * @param object aDescriptor
    *        The variable's descriptor.
+   * @param object aOptions
+   *        Options of the form accepted by addItem.
    * @return Variable
    *         The newly created child Variable.
    */
-  _createChild: function(aName, aDescriptor) {
+  _createChild: function(aName, aDescriptor, aOptions) {
-    return new Variable(this, aName, aDescriptor);
+    return new Variable(this, aName, aDescriptor, aOptions);
   },
 
   /**
@@ -1303,18 +1305,27 @@ Scope.prototype = {
    *             - { value: { type: "object", class: "Object" } }
    *             - { get: { type: "object", class: "Function" },
    *                 set: { type: "undefined" } }
-   * @param boolean aRelaxed [optional]
+   * @param object aOptions
-   *        Pass true if name duplicates should be allowed.
+   *        Specifies some options affecting the new variable.
-   *        You probably shouldn't do it. Use this with caution.
+   *        Recognized properties are
+   *        * boolean relaxed  true if name duplicates should be allowed.
+   *                           You probably shouldn't do it. Use this
+   *                           with caution.
+   *        * 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 = {}, aRelaxed = false) {
+  addItem: function(aName = "", aDescriptor = {}, aOptions = {}) {
-    if (this._store.has(aName) && !aRelaxed) {
+    let {relaxed} = aOptions;
+    if (this._store.has(aName) && !relaxed) {
       return this._store.get(aName);
     }
 
-    let child = this._createChild(aName, aDescriptor);
+    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);
@@ -2146,14 +2157,17 @@ XPCOMUtils.defineLazyGetter(Scope, "ellipsis", () =>
  *        The variable's name.
  * @param object aDescriptor
  *        The variable's descriptor.
+ * @param object aOptions
+ *        Options of the form accepted by Scope.addItem
  */
-function Variable(aScope, aName, aDescriptor) {
+function Variable(aScope, aName, aDescriptor, aOptions) {
   this._setTooltips = this._setTooltips.bind(this);
   this._activateNameInput = this._activateNameInput.bind(this);
   this._activateValueInput = this._activateValueInput.bind(this);
   this.openNodeInInspector = this.openNodeInInspector.bind(this);
   this.highlightDomNode = this.highlightDomNode.bind(this);
   this.unhighlightDomNode = this.unhighlightDomNode.bind(this);
+  this._internalItem = aOptions.internalItem;
 
   // Treat safe getter descriptors as descriptors with a value.
   if ("getterValue" in aDescriptor) {
@@ -2193,11 +2207,13 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
    *        The name of the new Property.
    * @param object aDescriptor
    *        The property's descriptor.
+   * @param object aOptions
+   *        Options of the form accepted by Scope.addItem
    * @return Property
    *         The newly created child Property.
    */
-  _createChild: function(aName, aDescriptor) {
+  _createChild: function(aName, aDescriptor, aOptions) {
-    return new Property(this, aName, aDescriptor);
+    return new Property(this, aName, aDescriptor, aOptions);
   },
 
   /**
@@ -2521,8 +2537,9 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
 
     if (this._initialDescriptor.enumerable ||
         this._nameString == "this" ||
-        this._nameString == "<return>" ||
+        (this._internalItem &&
-        this._nameString == "<exception>") {
+         (this._nameString == "<return>" ||
+          this._nameString == "<exception>"))) {
       this.ownerView._enum.appendChild(this._target);
       this.ownerView._enumItems.push(this);
     } else {
@@ -2869,11 +2886,11 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
     if (name == "this") {
       target.setAttribute("self", "");
     }
-    else if (name == "<exception>") {
+    else if (this._internalItem && name == "<exception>") {
       target.setAttribute("exception", "");
       target.setAttribute("pseudo-item", "");
     }
-    else if (name == "<return>") {
+    else if (this._internalItem && name == "<return>") {
       target.setAttribute("return", "");
       target.setAttribute("pseudo-item", "");
     }
@@ -3008,7 +3025,7 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
       configurable: true,
       enumerable: true,
       writable: true
-    }, true);
+    }, {relaxed: true});
 
     // Force showing the separator.
     item._separatorLabel.hidden = false;
@@ -3051,9 +3068,11 @@ Variable.prototype = Heritage.extend(Scope.prototype, {
  *        The property's name.
  * @param object aDescriptor
  *        The property's descriptor.
+ * @param object aOptions
+ *        Options of the form accepted by Scope.addItem
  */
-function Property(aVar, aName, aDescriptor) {
+function Property(aVar, aName, aDescriptor, aOptions) {
-  Variable.call(this, aVar, aName, aDescriptor);
+  Variable.call(this, aVar, aName, aDescriptor, aOptions);
 }
 
 Property.prototype = Heritage.extend(Variable.prototype, {

devtools/client/shared/widgets/VariablesViewController.jsm

@@ -431,7 +431,7 @@ VariablesViewController.prototype = {
       let label = StackFrameUtils.getScopeLabel(environment);
 
       // Block scopes may have the same label, so make addItem allow duplicates.
-      let closure = funcScope.addItem(label, undefined, true);
+      let closure = funcScope.addItem(label, undefined, {relaxed: true});
       closure.target.setAttribute("scope", "");
       closure.showArrow();
 

devtools/client/storage/ui.js

@@ -379,7 +379,7 @@ StorageUI.prototype = {
     mainScope.expanded = true;
 
     if (item.name && item.valueActor) {
-      let itemVar = mainScope.addItem(item.name + "", {}, true);
+      let itemVar = mainScope.addItem(item.name + "", {}, {relaxed: true});
 
       item.valueActor.string().then(value => {
         // The main area where the value will be displayed
@@ -461,7 +461,7 @@ StorageUI.prototype = {
     let valueScope = view.getScopeAtIndex(1) ||
                      view.addScope(L10N.getStr("storage.parsedValue.label"));
     valueScope.expanded = true;
-    let jsonVar = valueScope.addItem("", Object.create(null), true);
+    let jsonVar = valueScope.addItem("", Object.create(null), {relaxed: true});
     jsonVar.expanded = true;
     jsonVar.twisty = true;
     jsonVar.populate(jsonObject, {expanded: true});

devtools/client/webconsole/test/browser.ini

@@ -183,6 +183,7 @@ skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_console_variables_view_dom_nodes.js]
 [browser_console_variables_view_dont_sort_non_sortable_classes_properties.js]
 skip-if = buildapp == 'mulet'
+[browser_console_variables_view_special_names.js]
 [browser_console_variables_view_while_debugging.js]
 skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_console_variables_view_while_debugging_and_inspecting.js]

devtools/client/webconsole/test/browser_console_variables_view_special_names.js

@@ -0,0 +1,38 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// Check that variables view handles special names like "<return>"
+// properly for ordinary displays.
+
+"use strict";
+
+const TEST_URI = "data:text/html;charset=utf8,<p>test for bug 1084430";
+
+var test = asyncTest(function* () {
+  yield loadTab(TEST_URI);
+  let hud = yield openConsole();
+
+  ok(hud, "web console opened");
+
+  hud.setFilterState("log", false);
+  registerCleanupFunction(() => hud.setFilterState("log", true));
+
+  hud.jsterm.execute("inspect({ '<return>': 47, '<exception>': 91 })");
+
+  let varView = yield hud.jsterm.once("variablesview-fetched");
+  ok(varView, "variables view object");
+
+  let props = yield findVariableViewProperties(varView, [
+    { name: "<return>", value: 47 },
+    { name: "<exception>", value: 91 },
+  ], { webconsole: hud });
+
+  for (let prop of props) {
+    ok(!prop.matchedProp._internalItem, prop.name + " is not marked internal");
+    let target = prop.matchedProp._target;
+    ok(!target.hasAttribute("pseudo-item"),
+       prop.name + " is not a pseudo-item");
+  }
+});