1281047 - Reps: array indexes aren't sorted properly

Status: RESOLVED FIXED

(DevTools :: Shared Components, defect, P1)

Description

[Lin Clark [:linclark]]

Attachment: array-vals.png

The array indexes seem to be sorted as if they were strings, not numbers.

Comment 1

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Test string:
[ "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z" ]

Comment 2

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Another test:
let a = [];for(let i = 100; i<200; i++) {a.push(i)}

Comment 3

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Lin: Which repo should I be using here?

If I use https://github.com/devtools-html/gecko-dev then arrays are displayed as empty square brackets e.g. [ ]. Clicking on them does nothing.

If I use the patch from bug 1283870 on fx-team I can see e.g. [ "a", "b" ] but clicking on it does not expand it.

I am setting devtools.webconsole.new-frontend-enabled to true.

Which repo should I be using and which patches do I need to have web console working with reps so that I can reproduce the problem in this bug?

Comment 4

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

I can reproduce if I enable the DOM panel so I will work on it there.

Comment 5

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

In summary I needed the patch from bug 1283123 and bug 1283870.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

In devtools/server/actors/object.js::enumArrayProperties() we use the following to get the property names:

```
Object.getOwnPropertyNames([ "a", "b", "c" ])
```

The problem is that this returns [ "1", "2", "3" ] instead of [ 1, 2, 3 ].

Of course, an array can contain object properties e.g. myArr.someProp so we need to support both in this instance.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

So it seems that in this instance getOwnPropertyNames() is correct... array indexes are strings internally. It is not obvious because e.g. arr[1] is coerced into arr["1"].

We should still sort arrays as a special case because this is what most users would expect.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 1281047-fix-array-key-order.diff

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1b09eca51db0

Comment 10

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

Comment on attachment 8769702 [details] [diff] [review]
1281047-fix-array-key-order.diff

Review of attachment 8769702 [details] [diff] [review]:
-----------------------------------------------------------------

Works for me. We should make sure that this is covered by a test (in bug 1264676)

Nit: the commit message should be:
Bug 1281047 - Properly sort array indexes; r=Honza

Thanks Mike!
Honza

Comment 11

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: 1281047-fix-array-key-order.diff

The test will be added in bug 1286186 so this can be landed.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/4773ee41e97d
Properly sort array indexes; r=honza

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4773ee41e97d