#315 ordering of objects used in Selector - Prototype

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:id	tickets sorted by id (creation time)
Combine keywords for powerful searching.
Use advanced searching »

#315 √ resolved

ordering of objects used in Selector

Reported by ADO | August 29th, 2008 @ 12:11 PM | in 1.6.1

Selector.patterns (selector.js:310) is used as an ordered object - it is assumed that the members will be iterated over in the order they are added. The ECMAScript specification states that "an Object is an unordered collection of properties", so we cannot guarantee this.

This does not seem to cause a problem in any supported browser (they all iterate in the expected order), however it does cause problems with some other (proprietary) browsers. Tamarin uses hashtables to store object properties, so it is likely that similar issues will arise if it is included in Firefox 4.

Its also a bad idea because of the somewhat erratic ordering of members on IE - members of your [[Prototype]] in reverse order, members of the object itself in order, members of the objects in the prototype chain in order, going up the chain. If we ever have inheritance of these objects things will break in bizarre ways.

From my understanding of the code it looks like the best way of fixing this is to change the data structures around, and I don't feel confident doing that myself.

Comments and changes to this ticket

John-David Dalton August 29th, 2008 @ 04:38 PM
- → State changed from “new” to “enhancement”
- → Assigned user changed from “” to “Andrew Dupont”
Juriy Zaytsev August 29th, 2008 @ 06:41 PM
- no changes were found...
- 0001-Make-Selector.patterns-an-array.patch 6.3 KB
Juriy Zaytsev August 29th, 2008 @ 06:43 PM
- → Milestone changed from “” to “1.6.1”
@ADO Thanks for report!

I've attached a patch that makes Selector.patterns an ordered structure (i.e. an array).

Reviews are welcomed.
Juriy Zaytsev August 29th, 2008 @ 06:46 PM
On a side note, for (;;), as we know, is faster than for(in) since prototype chain is not "searched through" - that's another benefit of such change.
ADO August 29th, 2008 @ 09:42 PM
Thats pretty much the same patch as I made locally, it seems to work fine. The formatting seems slightly inconsistent - you do for (var i=0;... instead of for (var i = 0;..., but thats not really my call. Thanks for the fast response.
Juriy Zaytsev September 4th, 2008 @ 01:56 AM
- → Tag changed from “” to “patched”
I'm not sure how to make tests for such enhancement/bug.

Do we even need them?
ADO September 4th, 2008 @ 01:35 PM
I'm not sure that there are any sensible test cases for this, other than the fact that the browser I have works better with prototype now (and I can run the test suite).
Juriy Zaytsev September 4th, 2008 @ 01:43 PM
Which browser are you talking about?
ADO September 12th, 2008 @ 09:46 PM
Its ANT Galio Browser (http://www.antlimited.com/soluti.... There isn't a publicly available version for testing unfortunately.
GitHub Robot October 10th, 2008 @ 09:20 PM
- → State changed from “enhancement” to “resolved”
(from [b065eed3b8d7352b383dd007952ff065f8aca56d]) Selector.patterns should be represented as an ordered structure. [#315 state:resolved] http://github.com/sstephenson/pr...