Bug 1305981 - Fix grip truncation on object with non-enumerable properties. r=Honza;
The problem is that we are checking the computed props against `object.ownPropertyLength` to see if there's more to show.
But, if an object has a non-enumerable property (like `prototype`), `ownPropertyLength` counts it,
but it does not appear `object.preview`.
To fix that, we do a simple check : if there is more item in the preview object than `max`, then we show the "more..." label.
Adds a test to ensure the problem is fixed.
In the same time, I did a little refactor on how we deal with the last object delimiter when we get the props, so we don't have
to go back to the last object and remove the comma.
MozReview-Commit-ID: 1drpuUOsLNI
--- a/devtools/client/shared/components/reps/grip.js
+++ b/devtools/client/shared/components/reps/grip.js
@@ -54,59 +54,53 @@ define(function (require, exports, modul
let isInterestingProp = this.props.isInterestingProp || ((type, value) => {
return (
type == "boolean" ||
type == "number" ||
(type == "string" && value.length != 0)
);
});
- let ownProperties = object.preview ? object.preview.ownProperties : [];
+ let ownProperties = object.preview ? object.preview.ownProperties : {};
let indexes = this.getPropIndexes(ownProperties, max, isInterestingProp);
if (indexes.length < max && indexes.length < object.ownPropertyLength) {
// There are not enough props yet. Then add uninteresting props to display them.
indexes = indexes.concat(
this.getPropIndexes(ownProperties, max - indexes.length, (t, value, name) => {
return !isInterestingProp(t, value, name);
})
);
}
- let props = this.getProps(ownProperties, indexes);
- if (props.length < object.ownPropertyLength) {
+ const truncate = Object.keys(ownProperties).length > max;
+ let props = this.getProps(ownProperties, indexes, truncate);
+ if (truncate) {
// There are some undisplayed props. Then display "more...".
let objectLink = this.props.objectLink || span;
props.push(Caption({
key: "more",
object: objectLink({
object: object
- }, ((object ? object.ownPropertyLength : 0) - max) + " more…")
+ }, `${object.ownPropertyLength - max} more…`)
}));
- } else if (props.length > 0) {
- // Remove the last comma.
- // NOTE: do not change comp._store.props directly to update a property,
- // it should be re-rendered or cloned with changed props
- let last = props.length - 1;
- props[last] = React.cloneElement(props[last], {
- delim: ""
- });
}
return props;
},
/**
* Get props ordered by index.
*
* @param {Object} ownProperties Props object.
* @param {Array} indexes Indexes of props.
+ * @param {Boolean} truncate true if the grip will be truncated.
* @return {Array} Props.
*/
- getProps: function (ownProperties, indexes) {
+ getProps: function (ownProperties, indexes, truncate) {
let props = [];
// Make indexes ordered by ascending.
indexes.sort(function (a, b) {
return a - b;
});
indexes.forEach((i) => {
@@ -114,17 +108,17 @@ define(function (require, exports, modul
let prop = ownProperties[name];
let value = prop.value !== undefined ? prop.value : prop;
props.push(PropRep(Object.assign({}, this.props, {
key: name,
mode: "tiny",
name: name,
object: value,
equal: ": ",
- delim: ", ",
+ delim: i !== indexes.length - 1 || truncate ? ", " : "",
defaultRep: Grip
})));
});
return props;
},
/**
@@ -166,17 +160,17 @@ define(function (require, exports, modul
},
render: function () {
let object = this.props.object;
let props = this.safePropIterator(object,
(this.props.mode == "long") ? 100 : 3);
let objectLink = this.props.objectLink || span;
- if (this.props.mode == "tiny" || !props.length) {
+ if (this.props.mode == "tiny") {
return (
span({className: "objectBox objectBox-object"},
this.getTitle(object),
objectLink({
className: "objectLeftBrace",
object: object
}, "")
)
--- a/devtools/client/shared/components/test/mochitest/test_reps_grip.html
+++ b/devtools/client/shared/components/test/mochitest/test_reps_grip.html
@@ -22,16 +22,17 @@ window.onload = Task.async(function* ()
try {
yield testBasic();
// Test property iterator
yield testMaxProps();
yield testMoreThanMaxProps();
yield testUninterestingProps();
+ yield testNonEnumerableProps();
// Test that properties are rendered as expected by PropRep
yield testNestedObject();
yield testNestedArray();
} catch(e) {
ok(false, "Got an error: " + DevToolsUtils.safeErrorString(e));
} finally {
SimpleTest.finish();
@@ -42,26 +43,26 @@ window.onload = Task.async(function* ()
const testName = "testBasic";
// Test that correct rep is chosen
const gripStub = getGripStub("testBasic");
const renderedRep = shallowRenderComponent(Rep, { object: gripStub });
is(renderedRep.type, Grip.rep, `Rep correctly selects ${Grip.rep.displayName}`);
// Test rendering
- const defaultOutput = `Object`;
+ const defaultOutput = `Object { }`;
const modeTests = [
{
mode: undefined,
expectedOutput: defaultOutput,
},
{
mode: "tiny",
- expectedOutput: defaultOutput,
+ expectedOutput: `Object`,
},
{
mode: "short",
expectedOutput: defaultOutput,
},
{
mode: "long",
expectedOutput: defaultOutput,
@@ -138,16 +139,50 @@ window.onload = Task.async(function* ()
function testUninterestingProps() {
// Test object: `{a: undefined, b: undefined, c: "c", d: 1}`
// @TODO This is not how we actually want the preview to be output.
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1276376
const expectedOutput = `Object { a: undefined, b: undefined, c: "c", 1 more… }`;
}
+ function testNonEnumerableProps() {
+ // Test object: `Object.defineProperty({}, "foo", {enumerable : false});`
+ const testName = "testNonEnumerableProps";
+
+ // Test that correct rep is chosen
+ const gripStub = getGripStub("testNonEnumerableProps");
+ const renderedRep = shallowRenderComponent(Rep, { object: gripStub });
+ is(renderedRep.type, Grip.rep, `Rep correctly selects ${Grip.rep.displayName}`);
+
+ // Test rendering
+ const defaultOutput = `Object { }`;
+
+ const modeTests = [
+ {
+ mode: undefined,
+ expectedOutput: defaultOutput,
+ },
+ {
+ mode: "tiny",
+ expectedOutput: `Object`,
+ },
+ {
+ mode: "short",
+ expectedOutput: defaultOutput,
+ },
+ {
+ mode: "long",
+ expectedOutput: defaultOutput,
+ }
+ ];
+
+ testRepRenderModes(modeTests, testName, componentUnderTest, getGripStub(testName));
+ }
+
function testNestedObject() {
// Test object: `{objProp: {id: 1}, strProp: "test string"}`
const testName = "testNestedObject";
const defaultOutput = `Object { objProp: Object, strProp: "test string" }`;
const modeTests = [
{
@@ -325,17 +360,32 @@ window.onload = Task.async(function* ()
"writable": true,
"value": 1
}
},
"ownPropertiesLength": 4,
"safeGetterValues": {}
}
};
-
+ case "testNonEnumerableProps":
+ return {
+ "type": "object",
+ "actor": "server1.conn1.child1/obj30",
+ "class": "Object",
+ "extensible": true,
+ "frozen": false,
+ "sealed": false,
+ "ownPropertyLength": 1,
+ "preview": {
+ "kind": "Object",
+ "ownProperties": {},
+ "ownPropertiesLength": 1,
+ "safeGetterValues": {}
+ }
+ };
case "testNestedObject":
return {
"type": "object",
"class": "Object",
"actor": "server1.conn0.obj145",
"extensible": true,
"frozen": false,
"sealed": false,