Open Bug 1036001 Opened 5 years ago Updated Last year

The ArrayLike previewer should just check for the length property

Categories

(DevTools :: Console, defect, P3)

x86
macOS
defect

Tracking

(Not tracked)

People

(Reporter: fitzgen, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [console-papercuts][polish-backlog])

... instead of hard coding certain classes.

This way, we could reuse this for all array like objects (which will all work with Array.prototype.X).

We should print them in the form <class> [ <array-like contents> ]

Then jQuery objects would look like

    Object [ <a/> , <a/> , ... ]

instead of

    { 0: <a/>, 1: <a/>, ... }

And arguments objects would look like

    Arguments [ 1, 2, 3 ]

instead of

    { 0: 1, 1: 2, 2: 3, ... }

etc...

This could also solve bug 1035545 by just treating new String("abc") like the array-like that it is.
This was intentionally done as-is. I did try only checking the presence of the length property, but I did end up with various objects which break the preview, that cannot be iterated or have unexpected behavior. I chose the 'safer' approach: preview what we know of. The reasoning is that previews are best effort and they shouldn't be misleading / cause problems.

That being said, I'm sure we can improve this code.
(In reply to Mihai Sucan [:msucan] from comment #1)
> This was intentionally done as-is. I did try only checking the presence of
> the length property, but I did end up with various objects which break the
> preview, that cannot be iterated or have unexpected behavior. I chose the
> 'safer' approach: preview what we know of. The reasoning is that previews
> are best effort and they shouldn't be misleading / cause problems.
> 
> That being said, I'm sure we can improve this code.

What type of objects were "breaking"?

We can always add the Arguments object to the special case, but I think this would be incredibly valuable for libraries like jQuery where we can't just special case via [[class]].
(In reply to Nick Fitzgerald [:fitzgen] from comment #2)
> (In reply to Mihai Sucan [:msucan] from comment #1)
> > This was intentionally done as-is. I did try only checking the presence of
> > the length property, but I did end up with various objects which break the
> > preview, that cannot be iterated or have unexpected behavior. I chose the
> > 'safer' approach: preview what we know of. The reasoning is that previews
> > are best effort and they shouldn't be misleading / cause problems.
> > 
> > That being said, I'm sure we can improve this code.
> 
> What type of objects were "breaking"?

I did not make a list. IIRC I just went through the DOM.

> We can always add the Arguments object to the special case, but I think this
> would be incredibly valuable for libraries like jQuery where we can't just
> special case via [[class]].

The problem is they'll throw exceptions in some cases. We can put try-catch around the whole code, but that might hide errors we should fix. We should still log these cases as warnings or something.

I see your point, so I agree with the change. We will see how well it works in practice.

(updating status to reflect that Gabriel is assigned)
Status: NEW → ASSIGNED
May revisit this in the future - working on too many things atm.
Assignee: gabriel.luong → nobody
Whiteboard: [console-papercuts]
Whiteboard: [console-papercuts] → [console-papercuts][devedition-40]
Setting devedition-40 bugs to p3, filter on FB20EAC4-3FC3-48D9-B15F-0587C3987C25
Priority: -- → P3
Status: ASSIGNED → NEW
Whiteboard: [console-papercuts][devedition-40] → [console-papercuts][polish-backlog]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.