LGTM, maybe we should use defineProperties() instead to make a single call with the two descriptors, but I can probably take care of that when merging this.

I would perhaps disagree. Since defineProperty() is the method we are considering to use here, I think all tests should be directed against that method - there is a risk that the defineProperties() implementation would work slightly differently in these border-case JS engines.

One alternative would perhaps be to refactor the code to always use defineProperties() instead - after all there seem to be many cases where defineProperty is called several times in a row against the same object.

@xkr47 you're absolutely right.

I was looking to merge this, and found some unit-test errors, when looking into the details, it seems that your detection is always failing. e.g. this is the output from chrome console:

> var f = {}
undefined
> f.prototype instanceof Object
false

then doing:

> Object.defineProperty(f, 'prototype', { writable: false });
Object {prototype: undefined}
> f.prototype instanceof Object
false

As a result, v8 will fail the detection, and will use the fallback mechanism, which is not the desired outcome here. In other words, we need a better detection for Rhino if it happens that it doesn't really support Object.defineProperty.

ok this definately needs more testing then.

What did you think about using defineProperties() in general?

@xkr47 let's stick to what it is there today, which is defineProperty.

Bah I've mixed JS concepts here.. been too long since.. Hope I remember this correctly now after some thought:

The standardized way to access the prototype that was used to create an object is available through Object.getPrototypeOf(obj). This does not exist in all JS implementations but there are of course polyfills to the rescue. The non-standard way which works in some different subset of JS impls is obj.__proto__.

obj.prototype simply doesn't exist for regular objects. It does for functions however, and those are the ones used when new objects are created. So when you do {} or new Object() it calls the Object constructor and the new object that got created will have it's Object.getPrototypeOf(newObj) return the value of Object.prototype as it was at the time of calling those statements at the beginning of this sentence.

soo the original issue in Rhino:

• Line 58 Set up Intl.NumberFormat to point at constructor
• Line 65 Reconfigure Intl.NumberFormat.prototype (which is the prototype property of the constructor) to not be writable
• Line 372 Add new format property to the constructor prototype. This crashes because the prototype is unexpectedly undefined.

So based on this if I'd update the test function to use a function (constructor) instead of an object then it should work.. Pushing change in a little while..

I still need to verify this with Rhino.

Alright, this makes more sense, and tests are passing. Let me know if this is enough for Rhino, and we can go from there.

Rhino says: LGTM :)

Good, I will merge this in the next few days.

@caridy any chance of a new release soon?

this Thursday!

https://github.com/andyearnshaw/Intl.js/releases/tag/v1.2.5