1036001 - The ArrayLike previewer should just check for the length property

Status: RESOLVED INACTIVE

(DevTools :: Console, defect, P3)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

... 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.

Comment 1

[Mihai Sucan [:msucan]]

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.

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(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]].

Comment 3

[Mihai Sucan [:msucan]]

(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)

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

May revisit this in the future - working on too many things atm.

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Setting devedition-40 bugs to p3, filter on FB20EAC4-3FC3-48D9-B15F-0587C3987C25

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

inactive for 5 years, and not sure that's something we want