Bug 1084430 - fix styling of <return> and <exception> in variable view; r=vporof
authorTom Tromey <tom@tromey.com>
Thu, 19 Nov 2015 07:52:00 +0100
changeset 273994 c6d9dafc91a1c76fd0a79c5650e84a58569b5af7
parent 273812 4dca10306d58d2e87cbed46a72472b62c17b0ec9
child 273995 e8da96844aed463be839032e44ba23636221dc86
push id29716
push userkwierso@gmail.com
push dateWed, 25 Nov 2015 00:33:30 +0000
treeherdermozilla-central@880133a30e49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvporof
bugs1084430
milestone45.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 1084430 - fix styling of <return> and <exception> in variable view; r=vporof
devtools/client/debugger/debugger-controller.js
devtools/client/debugger/test/mochitest/browser_dbg_step-out.js
devtools/client/netmonitor/netmonitor-view.js
devtools/client/shared/widgets/VariablesView.jsm
devtools/client/shared/widgets/VariablesViewController.jsm
devtools/client/storage/ui.js
devtools/client/webconsole/test/browser.ini
devtools/client/webconsole/test/browser_console_variables_view_special_names.js
--- a/devtools/client/debugger/debugger-controller.js
+++ b/devtools/client/debugger/debugger-controller.js
@@ -968,22 +968,25 @@ StackFrames.prototype = {
    * @param Scope aScope
    *        The scope where the references will be placed into.
    * @param object aFrame
    *        The frame to get some references from.
    */
   _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".
     if (aFrame.this) {
       let thisRef = aScope.addItem("this", { value: aFrame.this });
       DebuggerView.Variables.controller.addExpander(thisRef, aFrame.this);
     }
   },
--- a/devtools/client/debugger/test/mochitest/browser_dbg_step-out.js
+++ b/devtools/client/debugger/test/mochitest/browser_dbg_step-out.js
@@ -26,16 +26,19 @@ function testNormalReturn() {
     waitForCaretAndScopes(gPanel, 19).then(() => {
       let innerScope = gVars.getScopeAtIndex(0);
       let returnVar = innerScope.get("<return>");
 
       is(returnVar.name, "<return>",
         "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());
     });
 
     EventUtils.sendMouseEvent({ type: "mousedown" },
       gDebugger.document.getElementById("step-out"),
       gDebugger);
   });
@@ -48,16 +51,19 @@ function testReturnWithException() {
     waitForCaretAndScopes(gPanel, 26).then(() => {
       let innerScope = gVars.getScopeAtIndex(0);
       let exceptionVar = innerScope.get("<exception>");
 
       is(exceptionVar.name, "<exception>",
         "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));
     });
 
     EventUtils.sendMouseEvent({ type: "mousedown" },
       gDebugger.document.getElementById("step-out"),
       gDebugger);
   });
--- a/devtools/client/netmonitor/netmonitor-view.js
+++ b/devtools/client/netmonitor/netmonitor-view.js
@@ -2747,17 +2747,17 @@ NetworkDetailsView.prototype = {
     let kb = aResponse.headersSize / 1024;
     let size = L10N.numberWithDecimals(kb, HEADERS_SIZE_DECIMALS);
     let text = L10N.getFormatStr("networkMenu.sizeKB", size);
 
     let headersScope = this._headers.addScope(aName + " (" + text + ")");
     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);
     }
   }),
 
   /**
    * Sets the network request cookies shown in this view.
    *
@@ -2797,17 +2797,17 @@ NetworkDetailsView.prototype = {
    * @return object
    *        Returns a promise that resolves upon the adding of cookies.
    */
   _addCookies: Task.async(function*(aName, aResponse) {
     let cookiesScope = this._cookies.addScope(aName);
     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);
 
       // By default the cookie name and value are shown. If this is the only
       // information available, then nothing else is to be displayed.
       let cookieProps = Object.keys(cookie);
       if (cookieProps.length == 2) {
         continue;
@@ -2911,17 +2911,17 @@ NetworkDetailsView.prototype = {
     let paramsArray = NetworkHelper.parseQueryString(aQueryString);
     if (!paramsArray) {
       return;
     }
     let paramsScope = this._params.addScope(aName);
     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);
     }
   },
 
   /**
    * Sets the network response body shown in this view.
    *
    * @param string aUrl
--- a/devtools/client/shared/widgets/VariablesView.jsm
+++ b/devtools/client/shared/widgets/VariablesView.jsm
@@ -1273,21 +1273,23 @@ Scope.prototype = {
 
   /**
    * Create a new Variable that is a child of this Scope.
    *
    * @param string aName
    *        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) {
-    return new Variable(this, aName, aDescriptor);
+  _createChild: function(aName, aDescriptor, aOptions) {
+    return new Variable(this, aName, aDescriptor, aOptions);
   },
 
   /**
    * Adds a child to contain any inspected properties.
    *
    * @param string aName
    *        The child's name.
    * @param object aDescriptor
@@ -1298,28 +1300,37 @@ Scope.prototype = {
    *        e.g. - { value: 42 }
    *             - { value: true }
    *             - { value: "nasu" }
    *             - { value: { type: "undefined" } }
    *             - { value: { type: "null" } }
    *             - { value: { type: "object", class: "Object" } }
    *             - { get: { type: "object", class: "Function" },
    *                 set: { type: "undefined" } }
-   * @param boolean aRelaxed [optional]
-   *        Pass true if name duplicates should be allowed.
-   *        You probably shouldn't do it. Use this with caution.
+   * @param object aOptions
+   *        Specifies some options affecting the new variable.
+   *        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) {
-    if (this._store.has(aName) && !aRelaxed) {
+  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);
+    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;
 
     return child;
   },
 
@@ -2141,24 +2152,27 @@ XPCOMUtils.defineLazyGetter(Scope, "elli
  * Iterable via "for (let [name, property] of instance) { }".
  *
  * @param Scope aScope
  *        The scope to contain this variable.
  * @param string aName
  *        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) {
     aDescriptor.value = aDescriptor.getterValue;
     delete aDescriptor.get;
     delete aDescriptor.set;
   }
 
@@ -2188,21 +2202,23 @@ Variable.prototype = Heritage.extend(Sco
 
   /**
    * Create a new Property that is a child of Variable.
    *
    * @param string aName
    *        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) {
-    return new Property(this, aName, aDescriptor);
+  _createChild: function(aName, aDescriptor, aOptions) {
+    return new Property(this, aName, aDescriptor, aOptions);
   },
 
   /**
    * Remove this Variable from its parent and remove all children recursively.
    */
   remove: function() {
     if (this._linkedToInspector) {
       this.unhighlightDomNode();
@@ -2516,18 +2532,19 @@ Variable.prototype = Heritage.extend(Sco
     this._displayVariable();
     this._customizeVariable();
     this._prepareTooltips();
     this._setAttributes();
     this._addEventListeners();
 
     if (this._initialDescriptor.enumerable ||
         this._nameString == "this" ||
-        this._nameString == "<return>" ||
-        this._nameString == "<exception>") {
+        (this._internalItem &&
+         (this._nameString == "<return>" ||
+          this._nameString == "<exception>"))) {
       this.ownerView._enum.appendChild(this._target);
       this.ownerView._enumItems.push(this);
     } else {
       this.ownerView._nonenum.appendChild(this._target);
       this.ownerView._nonEnumItems.push(this);
     }
   },
 
@@ -2864,21 +2881,21 @@ Variable.prototype = Heritage.extend(Sco
 
     if (descriptor && "getterValue" in descriptor) {
       target.setAttribute("safe-getter", "");
     }
 
     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", "");
     }
     else if (name == "__proto__") {
       target.setAttribute("proto", "");
       target.setAttribute("pseudo-item", "");
     }
 
@@ -3003,17 +3020,17 @@ Variable.prototype = Heritage.extend(Sco
 
     this.expanded = true;
 
     let item = this.addItem(" ", {
       value: undefined,
       configurable: true,
       enumerable: true,
       writable: true
-    }, true);
+    }, {relaxed: true});
 
     // Force showing the separator.
     item._separatorLabel.hidden = false;
 
     EditableNameAndValue.create(item, {
       onSave: ([aKey, aValue]) => {
         if (!this._variablesView.preventDisableOnChange) {
           this._disable();
@@ -3046,19 +3063,21 @@ Variable.prototype = Heritage.extend(Sco
  * Iterable via "for (let [name, property] of instance) { }".
  *
  * @param Variable aVar
  *        The variable to contain this property.
  * @param string aName
  *        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) {
-  Variable.call(this, aVar, aName, aDescriptor);
+function Property(aVar, aName, aDescriptor, aOptions) {
+  Variable.call(this, aVar, aName, aDescriptor, aOptions);
 }
 
 Property.prototype = Heritage.extend(Variable.prototype, {
   /**
    * The class name applied to this property's target element.
    */
   targetClassName: "variables-view-property variable-or-property",
 
--- a/devtools/client/shared/widgets/VariablesViewController.jsm
+++ b/devtools/client/shared/widgets/VariablesViewController.jsm
@@ -426,17 +426,17 @@ VariablesViewController.prototype = {
     funcScope.target.setAttribute("scope", "");
     funcScope.showArrow();
 
     do {
       // Create a scope to contain all the inspected variables.
       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();
 
       // Add nodes for every argument and every other variable in scope.
       if (environment.bindings) {
         this._populateWithEnvironmentBindings(closure, environment.bindings);
       } else {
         let deferred = promise.defer();
--- a/devtools/client/storage/ui.js
+++ b/devtools/client/storage/ui.js
@@ -374,17 +374,17 @@ StorageUI.prototype = {
       return;
     }
     this.sidebar.hidden = false;
     this.view.empty();
     let mainScope = this.view.addScope(L10N.getStr("storage.data.label"));
     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
         itemVar.setGrip(value);
 
         // May be the item value is a json or a key value pair itself
         this.parseItemValue(item.name, value);
 
@@ -456,17 +456,17 @@ StorageUI.prototype = {
     }
 
     let jsonObject = Object.create(null);
     let view = this.view;
     jsonObject[name] = json;
     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});
   },
 
   /**
    * Tries to parse a string into an object on the basis of key-value pairs,
    * separated by various separators. If failed, tries to parse for single
--- a/devtools/client/webconsole/test/browser.ini
+++ b/devtools/client/webconsole/test/browser.ini
@@ -178,16 +178,17 @@ skip-if = e10s # Bug 1042253 - webconsol
 skip-if = buildapp == 'mulet' || e10s # Bug 1042253 - webconsole e10s tests
 [browser_console_server_logging.js]
 [browser_console_variables_view.js]
 skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_console_variables_view_filter.js]
 [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]
 skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_eval_in_debugger_stackframe.js]
 skip-if = e10s # Bug 1042253 - webconsole tests disabled with e10s
 [browser_eval_in_debugger_stackframe2.js]
 [browser_jsterm_inspect.js]
new file mode 100644
--- /dev/null
+++ b/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");
+  }
+});