DOH failures under Rhino

[Adam Peller]

Also, tests._base.connect is failing at various points with the following error:

TypeError?: Cannot call method "apply" of undefined

this appears to be a regression from [14604]

[Adam Peller]

(In [14951]) Run browser-based tests conditionally. Refs #7523, #7178

[sjmiles]

connect uses an "array-as-hash" gambit.

Function listeners are stored as an array but iterated as a hash (for in). Array is used as it provides an auto-incrementing hash key (the array index).

Elements are deleted as if from a hash (i.e. with delete). This way existing entries retain their keys (array indices). Iterating as a hash is efficient because undefined keys are not iterated.

[14604] introduced code to iterate over a copy of the listeners array so the iterator is immune to changes to the original array. The copying is done via:

copy = [].concat(original)

In Rhino, using concat on a sparse Array results in a dense Array. IOW, if an array A has elements with indices (0, 2, 4), then under Rhino, "[].concat(A)" results in indices (0, 1, 2, 3, 4), where element 1 and 3 have value "undefined". "A.slice(0)" has the same behavior.

To patch the problem, I will modify connect to make an array copy using a loop if isRhino.

[sjmiles]

(In [14972]) Manual Array copy in connect.js under Rhino. Fixes #7523. !strict

[Adam Peller]

Ugh. I'm sorry I made you add more code to base. I wonder if this is something underspecified by ECMAScript or if it's a bug in Rhino? If the latter, we could just push this as a bug onto the Rhino team... I'll take a look at the ECMA spec.

[James Burke]

We can probably use the conditional build commands to strip out the rhino code when we do browser builds. If you are interested, I can explore what sort of >> statement we can use to cut out the rhino stuff for browser builds.

[James Burke]

(In [15043]) Refs #7523. Use conditional build options to leave out rhino-specific fix for browser builds, to limit impact on dojo base size. \!strict

[Adam Peller]

So Rhino 1.7 no longer seems to have this problem. We could write it up as feature detection, but we can probably just ditch this code path altogether?

[Adam Peller]

removes the isRhino workaround, tests pass with Rhino 1.7

[James Burke]

I'm OK with this code change, the bigger issue is deciding if we want to say Dojo only works with Rhino 1.7+. I do not have a good read on the % use of Rhino 1.6 + Dojo in the wild. peller, any insight you have? I figure your company might have some better stats on it.

[Adam Peller]

yeah, I'm finding people are still using Rhino 1.6, but I'm not sure why or how bad it would be to force an upgrade.

[Adam Peller]

need to reassess as we're now on a new loader and much of the core has changed. If there are still issues, it may make sense to start over with new ticket(s)

[haysmark]

There are still issues with Rhino.