This is one step towards a full implementation of the ECMAScript Internationalization API: Reimplement the three existing locale sensitive functions that the ECMAScript Internationalization spec redefines directly on top of ICU, using the default locale and default options per spec.

The current implementations of these functions use different mechanisms to let software outside of SpiderMonkey influence their behavior, and we have to decide whether any of this has to be maintained in the future.
- For String.prototype.localeCompare, SpiderMonkey by default uses the dumb Unicode code unit comparison specified in ES5 11.8.5 (which is neither locale sensitive nor conforms to the specification of localeCompare in 15.5.4.9). Client software can provide better implementations via JS_SetLocaleCallbacks, and XPConnect does so to install implementations from the intl/ directory in the Mozilla tree. The implementations I've tested on Mac and Windows do not conform to ES5 15.5.4.9 either (they don't implement canonical equivalence).
- For Number.prototype.toLocaleString, SpiderMonkey has its own implementation, but obtains localized decimal separator, thousands separator, and grouping information from either the operating system or from environment variables. The latter mechanism is used by the Android GeckoAppShell.java classes.
- For Date.prototype.toLocaleString, SpiderMonkey relies on the operating system's strftime function, and makes formatting based on strftime patterns available to applications through a non-standard method toLocaleFormat.
Proposal:
- Ignore localeCompare implementations provided by SpiderMonkey clients. No such implementation could conform to the ECMAScript Internationalization API specification because it doesn't have access to the new locales and options arguments. Such implementations will also no longer be necessary once SpiderMonkey relies on ICU to implement localeCompare. The field in the JSLocaleCallbacks struct remains in order to provide minimal binary compatibility with existing clients.
- Ignore decimal separator, thousands separator, and grouping information provided via environment variables. While such information could theoretically be merged into ICU output, it won't improve the results because the provider doesn't have access to the locales argument and ICU has all necessary information.
- Maintain Date.prototype.toLocaleFormat with its current functionality for compatibility with existing applications. However, this function will not benefit from the improvements provided by the ECMAScript Internationalization API (such as control over the language used). Date.prototype.toLocaleString will rely on ICU rather than strftime.
It may make sense to give SpiderMonkey containers a way to control the default locale used by locale sensitive functions; this is currently being discussed in bug 769872.

Created attachment 660330[details][diff][review]
Reimplement String.localeCompare, Number.toLocaleString, Date.toLocaleString using ICU (WIP)
First functional version. Doesn't cache any ICU objects yet, which it likely will have to do to achieve reasonable performance. Uses file names intl.h and intl.cpp to avoid collisions with the jsintl.h and jsintl.cpp being added via bug 769872 - in the end, code will be merged into jsintl. Tested only in js shell.

Comment on attachment 699997[details][diff][review]
Reimplement String.localeCompare, Number.toLocaleString, Date.toLocaleString using ICU (WIP)
The upcoming attachments use a different design from the previous ones: They implement the locale sensitive functions in String, Number, and Date as self-hosted JavaScript on top of Intl.Collator, Intl.NumberFormat, and Intl.DateTimeFormat.

Created attachment 727091[details][diff][review]
Reimplement Number.toLocaleString per ECMA-402.
Updated per comment 11. Carrying r+jwalden.
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #11)
> @@ +916,5 @@
> > JS_FN(js_valueOf_str, js_num_valueOf, 0, 0),
> > JS_FN("toFixed", num_toFixed, 1, 0),
> > JS_FN("toExponential", num_toExponential, 1, 0),
> > JS_FN("toPrecision", num_toPrecision, 1, 0),
> > +// ??? must be last - functions listed after it aren't available in self-hosted code
>
> Given the mixing we have in jsarray.cpp array_methods, I don't think this is
> necessary. What's the behavior you see that causes you to think it is?
For String, my self-hosted code couldn't find split anymore - it looked like the function didn't exist. For Number, I don't remember exactly, but it may have been valueOf where it'd find only the one in Object.prototype. I'll file a ticket when this code has landed and is more available for testing.
The only Array methods that show up after the self-hosted entries in the method table are filter and iterator, neither of which are used in self-hosted code.
> > + * Usage: callFunction(date_CheckThisNumber, this)
>
> This could just be date_CheckThisNumber(this), right? Don't see how
> callFunction is necessary for this case.
date_CheckThisNumber(this) would be equivalent to callFunction(date_CheckThisNumber, undefined, this). CallNonGenericMethod expects the this value as args.thisv().

(In reply to Norbert Lindenberg from comment #13)
> > > +// ??? must be last - functions listed after it aren't available in self-hosted code
> >
> > Given the mixing we have in jsarray.cpp array_methods, I don't think this is
> > necessary. What's the behavior you see that causes you to think it is?
>
> For String, my self-hosted code couldn't find split anymore - it looked like
> the function didn't exist. For Number, I don't remember exactly, but it may
> have been valueOf where it'd find only the one in Object.prototype. I'll
> file a ticket when this code has landed and is more available for testing.
>
> The only Array methods that show up after the self-hosted entries in the
> method table are filter and iterator, neither of which are used in
> self-hosted code.
My previous comment provided me with the idea for a simple test case, so there's now bug 853075.

(In reply to Norbert Lindenberg from comment #13)
> > > + * Usage: callFunction(date_CheckThisNumber, this)
> >
> > This could just be date_CheckThisNumber(this), right? Don't see how
> > callFunction is necessary for this case.
>
> date_CheckThisNumber(this) would be equivalent to
> callFunction(date_CheckThisNumber, undefined, this). CallNonGenericMethod
> expects the this value as args.thisv().
Oh, right, I'm dumb.
Looking at this closer, it happens we get this non-generic test performed when we call Date.prototype.valueOf, actually. So technically the helper, and the call to it, are unnecessary. It was very tempting to just remove it...but probably that's enough change to want an actual patch and review. I'll do that and post them here for you to test out, in a sec.
(In reply to Norbert Lindenberg from comment #15)
> My previous comment provided me with the idea for a simple test case, so
> there's now bug 853075.
Excellent! Thank you for this. I tweaked the comments on these to mention this bug, too.

Sorry for the confusion, but the internationalization API and thus the changes in this bug have remained disabled for all Firefox releases at least up to 26. They're now enabled in nightly, so if all goes well they may get into Firefox 27. I've removed the note from the Firefox 23 release notes, to be added to the notes for the release in which the API actually ships. The bug that really determines whether the API is in or not is bug 853301.

Going to mark this as complete as the reference docs for String.localeCompare, Number.toLocaleString, Date.toLocaleString are in place (see comment 27). I will use bug 853301 to update the dev doc release notes and compat tables as that bug is actually about enabling the Internationalization API and has less confusing version info.

Note

You need to
log in
before you can comment on or make changes to this bug.