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

--HG--
extra : rebase_source : d55288c00a0ac54d7676e9b2e497df5fdefee5aa

devtools/client/shared/components/reps/grip.js

@@ -59,7 +59,7 @@ define(function (require, exports, module) {
         );
       });
 
-      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.
@@ -70,24 +70,17 @@ define(function (require, exports, module) {
         );
       }
 
-      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({
           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;
@@ -98,9 +91,10 @@ define(function (require, exports, module) {
      *
      * @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.
@@ -117,7 +111,7 @@ define(function (require, exports, module) {
           name: name,
           object: value,
           equal: ": ",
-          delim: ", ",
+          delim: i !== indexes.length - 1 || truncate ? ", " : "",
           defaultRep: Grip
         })));
       });
@@ -169,7 +163,7 @@ define(function (require, exports, module) {
         (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),

devtools/client/shared/components/test/mochitest/test_reps_grip.html

@@ -27,6 +27,7 @@ window.onload = Task.async(function* () {
     yield testMaxProps();
     yield testMoreThanMaxProps();
     yield testUninterestingProps();
+    yield testNonEnumerableProps();
 
     // Test that properties are rendered as expected by PropRep
     yield testNestedObject();
@@ -50,7 +51,7 @@ window.onload = Task.async(function* () {
     is(renderedRep.type, Grip.rep, `Rep correctly selects ${Grip.rep.displayName}`);
 
     // Test rendering
-    const defaultOutput = `Object`;
+    const defaultOutput = `Object {  }`;
 
     const modeTests = [
       {
@@ -59,7 +60,7 @@ window.onload = Task.async(function* () {
       },
       {
         mode: "tiny",
-        expectedOutput: defaultOutput,
+        expectedOutput: `Object`,
       },
       {
         mode: "short",
@@ -146,6 +147,40 @@ window.onload = Task.async(function* () {
     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";
@@ -362,7 +397,22 @@ window.onload = Task.async(function* () {
             "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",