1175111 - Wrong property order in Object.getOwnPropertyNames(string) and others

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, defect)

Description

[ziyunfei]

js> Object.getOwnPropertyNames("123")
["length", "0", "1", "2"] // should be [ "0", "1", "2", "length" ], per spec 9.4.3.3

Comment 1

[Jason Orendorff [:jorendorff]]

Rules about own property ordering affect many builtins:
- Object.keys
- Object.getOwnPropertyNames
- Object.assign
- JSON.stringify
- Reflect.ownKeys
- Reflect.enumerate (not yet implemented, bug 1185493)

There may also be aspects of behavior exposed by the for-in loop alone that we get wrong.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

This fixes for-in, Object.getOwnPropertyNames etc to sort integer properties and put them before other properties.

The only problem I see is that the standard defines an integer property as any integer >= 0 and < 2**53, whereas our "indexed id" range is much smaller.

This seems to be a more general issue though and I think fixing this requires changes to IdIsIndex etc.

Comment 3

[Jason Orendorff [:jorendorff]]

Comment on attachment 8676399 [details] [diff] [review]
Patch

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

Thanks!

::: js/src/jsiter.cpp
@@ +144,5 @@
> +{
> +    uint32_t indexA, indexB;
> +    DebugOnly<bool> isIndexA = IdIsIndex(a, &indexA);
> +    DebugOnly<bool> isIndexB = IdIsIndex(b, &indexB);
> +    MOZ_ASSERT(isIndexA && isIndexB);

You can use MOZ_ALWAYS_TRUE for these.

@@ +165,2 @@
>          for (size_t i = 0; i < initlen; ++i, ++vp) {
>              if (!vp->isMagic(JS_ELEMENTS_HOLE)) {

Maybe swap the two arms of this if-else now, and remove the `!`.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/45dd451ce4ed

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/45dd451ce4ed

Comment 7

[Mike Shaver (:shaver emeritus)]

I'm sure the array part of this is my fault -- sorry!