419090 - Object properties list in different (hash?) order than entered

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Mark]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.12) Gecko/20080201 Firefox/2.0.0.12
Build Identifier: 

Object properties seem to be returned in hash order in a for-each loop as opposed to entered order.  e.g.:

{a:1, b:1, c:1} appears as {b:1, c:1, a:1}.  I believe it's hash order because {c:1, b:1, a:1} or any other order you specify always returns the same order (b, c, a).

Reproducible: Always

Steps to Reproduce:
1. create an object with properties
2. echo the properties of that object
Actual Results:  
properties returned in hash-order

Expected Results:  
properties return in entered order

I fully realize that from a programatic point of view this does not matter.  It does, however, matter in terms of a usability point of view.  If there is a logical order to the properties and you view the object, that order is discarded.  e.g.:

myObj: {posX: 10, posY: 10, posZ: 10, width: 10, height: 10, depth: 10}

returns:

{"height": 10, "posY": 10, "posX": 10, "width": 10, "posZ": 10, "depth": 10}

Comment 1

[Daniel Gredler]

Hi,

It seems that all the browser interpreters implement this feature, so it's something that we at HtmlUnit (which emulates a browser and uses Rhino) would like to have.

I have two questions:

1. Would a patch fixing this "bug" be accepted, assuming it's up to snuff technically?

2. Would it be preferrable to: a) modify the contract of Scriptable#getIds() so that it is guaranteed to return property IDs in definition order, or b) add a new method (Scriptable#getOrderedIds(), for example) that is used only by ScriptRuntime#enumChangeObject(IdEnumeration)?

Regards,

Daniel

Comment 2

[Daniel Gredler]

Attachment: proposed patch

Attaching proposed patch which uses a doubly-linked list of slots to keep track of property definition order.

Comment 3

[Norris Boyd]

Strictly speaking, the current behavior is acceptable under ECMA. However, matching the behavior of JS engines in browsers is important. Bug 383592 is an open request with a proposed patch for making the architecture pluggable so that people who want a well-defined iteration order (and are willing to pay the performance penalty) can do so. This is probably the way to go, but in my work with it so far the performance cost of switching to the standard Java Map classes is too high. So some work needs to be done to improve performance before I can switch.

Comment 4

[Norris Boyd]

I've committed a modified version of the proposed patch (thanks!):

Checking in src/org/mozilla/javascript/ScriptableObject.java;
/cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ScriptableObject.java,v  <--  ScriptableObject.java
new revision: 1.131; previous revision: 1.130
done
RCS file: /cvsroot/mozilla/js/rhino/testsrc/doctests/serialize.doctest,v
done
Checking in testsrc/doctests/serialize.doctest;
/cvsroot/mozilla/js/rhino/testsrc/doctests/serialize.doctest,v  <--  serialize.doctest
initial revision: 1.1
done

My only changes were to go from a doubly-linked list to a singly-linked list to reduce the memory footprint. In my profiling the performance impact of adding this list was minimal.