1257784 - Reps: make sure there is no recursion when generating object preview

Status: RESOLVED WORKSFORME

(DevTools :: Shared Components, defect, P1)

Description

[Jan Honza Odvarko [:Honza] (always need-info? me)]

We need to avoid cause recursion when generating object preview using reps/object.js template in all cases. This problem can't happen in devtools now - the JSON Viewer is safe (JSON doesn't have back references) as well as DOM view is safe (it's using grips, also no back references), but we still want to make the logic solid.

This can happen when an object has a property that refers to self.

For example:

var object = {}
object.self = object;

var tree = TreeView({
  object: object,
  renderValue: props => {
    return Rep(props);
  }
});

ReactDOM.render(tree, parentNode);

The object.js rep iterates all its properties and generates a preview for each. Every prop with type == 'object' is doing the same ...

The proper way might be using mode 'tiny' for child properties and skip 'object' type properties preview (in isInterestingProp callback).


There is a little fix in patch for bug 1201475, but not 100% solid. 

We want to have a test for this case (using the example above).

Honza

Comment 1

[Lin Clark [:linclark]]

This seems like a P1 to me, but I'm open to downgrading to P2.

Comment 2

[Steve Chung [:steveck]]

Hi Lin,

Based on Honza's comment, should we treat all the display of object properties by tiny mode? For example: "Object{objProp: Object}" instead of "Object{objProp: Object{...}}".

Comment 3

[Lin Clark [:linclark]]

I'm not sure what Honza's thoughts with this issue were or whether it's still an issue. I'll mark it for him to look at when he gets back.

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

(In reply to Steve Chung [:steveck] from comment #2)
> Based on Honza's comment, should we treat all the display of object
> properties by tiny mode? For example: "Object{objProp: Object}" instead of
> "Object{objProp: Object{...}}".
Yes, and this is how it works now.

I don't think this is an issue anymore.

You can also see this line:
https://dxr.mozilla.org/mozilla-central/source/devtools/client/shared/components/reps/object.js#101

The 'tiny' mode is hardcoded to avoid recursion so, I think we are safe here.

I am closing this report.

Honza