From 4b5ecc26f424fa5cb22b931836daca7a09593363 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 19 Nov 2015 07:52:00 +0100 Subject: [PATCH] Bug 1084430 - fix styling of and in variable view; r=vporof --- .../client/debugger/debugger-controller.js | 7 ++- .../test/mochitest/browser_dbg_step-out.js | 6 ++ devtools/client/netmonitor/netmonitor-view.js | 6 +- .../client/shared/widgets/VariablesView.jsm | 55 +++++++++++++------ .../widgets/VariablesViewController.jsm | 2 +- devtools/client/storage/ui.js | 4 +- devtools/client/webconsole/test/browser.ini | 1 + ...er_console_variables_view_special_names.js | 38 +++++++++++++ 8 files changed, 93 insertions(+), 26 deletions(-) create mode 100644 devtools/client/webconsole/test/browser_console_variables_view_special_names.js diff --git a/devtools/client/debugger/debugger-controller.js b/devtools/client/debugger/debugger-controller.js index bcb6088c2cc5..76137abd8a49 100644 --- a/devtools/client/debugger/debugger-controller.js +++ b/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("", { value: this._currentException }); + let excRef = aScope.addItem("", { value: this._currentException }, + { internalItem: true }); DebuggerView.Variables.controller.addExpander(excRef, this._currentException); } // Add any returned value. if (this._currentReturnedValue) { - let retRef = aScope.addItem("", { value: this._currentReturnedValue }); + let retRef = aScope.addItem("", + { value: this._currentReturnedValue }, + { internalItem: true }); DebuggerView.Variables.controller.addExpander(retRef, this._currentReturnedValue); } // Add "this". diff --git a/devtools/client/debugger/test/mochitest/browser_dbg_step-out.js b/devtools/client/debugger/test/mochitest/browser_dbg_step-out.js index e6e38a7697fa..bf23a3562445 100644 --- a/devtools/client/debugger/test/mochitest/browser_dbg_step-out.js +++ b/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)); }); diff --git a/devtools/client/netmonitor/netmonitor-view.js b/devtools/client/netmonitor/netmonitor-view.js index b9781fc8992f..6a123a04a51a 100644 --- a/devtools/client/netmonitor/netmonitor-view.js +++ b/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); } }, diff --git a/devtools/client/shared/widgets/VariablesView.jsm b/devtools/client/shared/widgets/VariablesView.jsm index f8a27b69664a..f877ec53f7f5 100644 --- a/devtools/client/shared/widgets/VariablesView.jsm +++ b/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) { - return new Variable(this, aName, aDescriptor); + _createChild: function(aName, aDescriptor, aOptions) { + 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] - * 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 or 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); @@ -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) { - return new Property(this, aName, aDescriptor); + _createChild: function(aName, aDescriptor, aOptions) { + 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 == "" || - this._nameString == "") { + (this._internalItem && + (this._nameString == "" || + this._nameString == ""))) { 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 == "") { + else if (this._internalItem && name == "") { target.setAttribute("exception", ""); target.setAttribute("pseudo-item", ""); } - else if (name == "") { + else if (this._internalItem && name == "") { 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) { - 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, { diff --git a/devtools/client/shared/widgets/VariablesViewController.jsm b/devtools/client/shared/widgets/VariablesViewController.jsm index 8f32e06efe47..6b2c8b09c538 100644 --- a/devtools/client/shared/widgets/VariablesViewController.jsm +++ b/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(); diff --git a/devtools/client/storage/ui.js b/devtools/client/storage/ui.js index 446b1da0ddd6..fceeda7b4dd4 100644 --- a/devtools/client/storage/ui.js +++ b/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}); diff --git a/devtools/client/webconsole/test/browser.ini b/devtools/client/webconsole/test/browser.ini index d8602f955531..591a03ad494b 100644 --- a/devtools/client/webconsole/test/browser.ini +++ b/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] diff --git a/devtools/client/webconsole/test/browser_console_variables_view_special_names.js b/devtools/client/webconsole/test/browser_console_variables_view_special_names.js new file mode 100644 index 000000000000..a2e926388f49 --- /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 "" +// properly for ordinary displays. + +"use strict"; + +const TEST_URI = "data:text/html;charset=utf8,

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({ '': 47, '': 91 })"); + + let varView = yield hud.jsterm.once("variablesview-fetched"); + ok(varView, "variables view object"); + + let props = yield findVariableViewProperties(varView, [ + { name: "", value: 47 }, + { name: "", 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"); + } +});