Implementing String, Number and Date toLocaleXXXString overrides as per ECMA 402, Chapter 13. Small…

Implementing String, Number and Date toLocaleString, localeCompare overrides as per ECMA 402, Chapter 13 - see http://wiki.ecmascript.org/doku.php?id=globalization:specification_drafts for details.

Small bug fix in date-format.js.

To see the changes that would happen for users, take a look at:
http://i18n.kaziprst.org/comparison.html

BUG=v8-i18n:29, v8:459

[cira1, 2012-10-03 20:41:46]

Simplify localeCompare.

[cira1, 2012-10-03 22:06:48]

Small changes

[cira1, 2012-10-03 22:08:20]

Please take a look.

[cira1, 2012-10-03 22:11:04]

Simplify Number.prototype.toLocaleString

[jungshik, 2012-10-03 22:55:09]

Will you add a pointer to v8 bug 459 to the CL description?

[jungshik, 2012-10-03 23:01:48]

With a reference to V8 bug 459 added to the description, LGTM

[jungshik, 2012-10-03 23:13:15]

On 2012/10/03 23:01:48, jungshik wrote:
> With a reference to V8 bug 459 added to the description, LGTM

BTW, you can make the comparison page at 
http://i18n.kaziprst.org/comparison.html
more 'dramatic' by showing what toLocaleXXX does in a non-English locale (before
and after).

[arv, 2012-10-03 23:57:34]

https://codereview.appspot.com/6591072/diff/1008/src/overrides.js
File src/overrides.js (right):

https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode24
src/overrides.js:24: String.prototype.localeCompare = function(that, locales,
options) {
This changes the enumerability.

Use Object.defineProperty instead

https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode24
src/overrides.js:24: String.prototype.localeCompare = function(that, locales,
options) {
This no longer reports the function as native.

assertEquals(String(String.prototype.toLocalString), 'function toLocaleString()
{ [native code] }')

https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode26
src/overrides.js:26: return new Intl.Collator(locales, options).compare(this,
that);
How do we ensure that we are using the original Intl and Intl.Collator?

[cira1, 2012-10-04 00:27:53]

On 2012/10/03 23:57:34, arv-chromium wrote:
> https://codereview.appspot.com/6591072/diff/1008/src/overrides.js
> File src/overrides.js (right):
> 
> https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode24
> src/overrides.js:24: String.prototype.localeCompare = function(that, locales,
> options) {
> This changes the enumerability.
> 
> Use Object.defineProperty instead

I can do that, but Object.defineProperty is slow in v8.

> 
> https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode24
> src/overrides.js:24: String.prototype.localeCompare = function(that, locales,
> options) {
> This no longer reports the function as native.
> 
> assertEquals(String(String.prototype.toLocalString), 'function
toLocaleString()
> { [native code] }')

Is that breaking the spec or just v8 test expectation? We can fix v8 test if
latter.
I could make it native, but it would complicate code needlessly.

> https://codereview.appspot.com/6591072/diff/1008/src/overrides.js#newcode26
> src/overrides.js:26: return new Intl.Collator(locales, options).compare(this,
> that);
> How do we ensure that we are using the original Intl and Intl.Collator?

I am not. I'll save references to methods I use on startup (hijacking that would
be hard) and then use those references in the future.

[cira1, 2012-10-04 23:47:31]

Save a reference to Intl.Constructor for security.

[cira1, 2012-10-04 23:52:08]

On 2012/10/04 23:47:31, cira1 wrote:
> Save a reference to Intl.Constructor for security.

Erik, please take another look.

1. Fixed enumerable issue - I hope we don't regress too much on startup speed
because of Object.defineProperty.

2. Saved reference to the constructors and added test for it.

3. I couldn't save reference to the XXX.prototype.(compare|format). Those are
getters and I couldn't find a way to get to it without executing it, which then
fails.

4. I don't think I can do much about functions not being native issue at this
point.

[cira1, 2012-10-05 23:52:00]

Calls internal methods instead of the prototype methods - for security.

[arv, 2012-10-08 14:18:56]

LGTM with nits

https://codereview.appspot.com/6591072/diff/22001/src/overrides.js
File src/overrides.js (right):

https://codereview.appspot.com/6591072/diff/22001/src/overrides.js#newcode32
src/overrides.js:32: {value: function(that, locales, options) {
Please fix indentation


Object.defineProperty(String.prototype, 'localeCompare', {
  value: function(that, locales, options) {
    // Call internal method.
    return compare(new collator(locales, options), this, that);
  },
  writable: true,
  configurable: true,
  enumerable: false
});

https://codereview.appspot.com/6591072/diff/22001/tests/intl/overrides/securi...
File tests/intl/overrides/security.js (right):

https://codereview.appspot.com/6591072/diff/22001/tests/intl/overrides/securi...
tests/intl/overrides/security.js:28: var throws = false;
Isn't there an assertThrows?

[cira1, 2012-10-08 16:37:03]

Using assertThrow, assertDoesNotThrow. Added both util methods to assert.js file
in tests/.

[cira1, 2012-10-08 16:40:19]

Fixed indentation in overrides.js.