Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(8)

Issue 6591072: Implementing String, Number and Date toLocaleXXXString overrides as per ECMA 402, Chapter 13. Small… (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by cira1
Modified:
2 years, 4 months ago
Reviewers:
jungshik, arv
CC:
v8-dev_googlegroups.com
Base URL:
http://v8-i18n.googlecode.com/svn/trunk/
Visibility:
Public.

Description

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

Patch Set 1 : Simplify Number.prototype.toLocaleString #

Total comments: 3

Patch Set 2 : Save a reference to Intl.Constructor for security. #

Patch Set 3 : Calls internal methods instead of the prototype methods - for security. #

Total comments: 2

Patch Set 4 : Using assertThrow, assertDoesNotThrow. Added both util methods to assert.js file in tests/. #

Patch Set 5 : Fixed indentation in overrides.js. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+347 lines, -3 lines) Patch
M build/all.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M src/date-format.js View 2 chunks +3 lines, -3 lines 0 comments Download
A src/overrides.js View 1 2 3 4 1 chunk +116 lines, -0 lines 0 comments Download
M tests/assert.js View 1 2 3 1 chunk +42 lines, -0 lines 0 comments Download
A tests/intl/overrides/date.js View 1 chunk +52 lines, -0 lines 0 comments Download
A tests/intl/overrides/number.js View 1 chunk +40 lines, -0 lines 0 comments Download
A tests/intl/overrides/security.js View 1 2 3 1 chunk +37 lines, -0 lines 0 comments Download
A tests/intl/overrides/string.js View 1 chunk +56 lines, -0 lines 0 comments Download

Messages

Total messages: 15
cira1
Simplify localeCompare.
2 years, 5 months ago (2012-10-03 20:41:46 UTC) #1
cira1
Small changes
2 years, 5 months ago (2012-10-03 22:06:48 UTC) #2
cira1
Please take a look.
2 years, 5 months ago (2012-10-03 22:08:20 UTC) #3
cira1
Simplify Number.prototype.toLocaleString
2 years, 5 months ago (2012-10-03 22:11:04 UTC) #4
jungshik
Will you add a pointer to v8 bug 459 to the CL description?
2 years, 5 months ago (2012-10-03 22:55:09 UTC) #5
jungshik
With a reference to V8 bug 459 added to the description, LGTM
2 years, 5 months ago (2012-10-03 23:01:48 UTC) #6
jungshik
On 2012/10/03 23:01:48, jungshik wrote: > With a reference to V8 bug 459 added to ...
2 years, 5 months ago (2012-10-03 23:13:15 UTC) #7
arv
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 ...
2 years, 5 months ago (2012-10-03 23:57:34 UTC) #8
cira1
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 > ...
2 years, 5 months ago (2012-10-04 00:27:53 UTC) #9
cira1
Save a reference to Intl.Constructor for security.
2 years, 5 months ago (2012-10-04 23:47:31 UTC) #10
cira1
On 2012/10/04 23:47:31, cira1 wrote: > Save a reference to Intl.Constructor for security. Erik, please ...
2 years, 5 months ago (2012-10-04 23:52:08 UTC) #11
cira1
Calls internal methods instead of the prototype methods - for security.
2 years, 5 months ago (2012-10-05 23:52:00 UTC) #12
arv
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 ...
2 years, 4 months ago (2012-10-08 14:18:56 UTC) #13
cira1
Using assertThrow, assertDoesNotThrow. Added both util methods to assert.js file in tests/.
2 years, 4 months ago (2012-10-08 16:37:03 UTC) #14
cira1
2 years, 4 months ago (2012-10-08 16:40:19 UTC) #15
Fixed indentation in overrides.js.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1437:571d95b2634e