Bug 90906 - Implement ECMAScript Internationalization API
Summary: Implement ECMAScript Internationalization API
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords: InRadar
: 137616 (view as bug list)
Depends on: 146926 147599 147601 147602 147603 147604 147605 147606 147607 147608 147609 147610 147611 147612 147613 147614 152448 153679 156993 159693
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-10 13:47 PDT by Jungshik Shin
Modified: 2016-07-12 15:37 PDT (History)
27 users (show)

See Also:


Attachments
Patch (63.05 KB, patch)
2015-05-21 18:07 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (446.48 KB, patch)
2015-05-27 14:08 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (357.78 KB, patch)
2015-05-29 20:32 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (358.12 KB, patch)
2015-06-01 07:04 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Add feature flag and base Intl namespace object (67.67 KB, patch)
2015-06-01 22:40 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch for landing (65.26 KB, patch)
2015-06-30 16:44 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Add namespace object and feature flag (67.29 KB, patch)
2015-06-30 18:03 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff
Patch (150.18 KB, patch)
2015-07-17 19:14 PDT, Andy VanWagoner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jungshik Shin 2012-07-10 13:47:57 PDT
Ecma TC39 is likely to approve I18N API [1] very soon. 

Chrome (v8) [2] and MSIE have been implementing the spec. Firefox has a plan for the implementation (most likely to be with ICU). 

If this can be added to JavaScriptCore or its 'extension' (at least for ports using ICU like Safari ), it'd benefit web app developers across the board. 
Moreover, some of HTML5 elements (e.g. <input type="date">) can be reimplemented with this API. Currently, some locale-specific data are embedded in the implementation (e.g. http://trac.webkit.org/browser/trunk/Source/WebCore/Resources/calendarPicker.js?rev=122212 ), which may not be scalable for tens of languages/locales. 


[1] 
http://wiki.ecmascript.org/doku.php?id=globalization:specification_drafts
http://norbertlindenberg.com/2012/06/ecmascript-internationalization-api/index.html

[2] http://code.google.com/p/v8-i18n
Comment 1 Kent Tamura 2012-07-10 17:35:38 PDT
(In reply to comment #0)
> Moreover, some of HTML5 elements (e.g. <input type="date">) can be reimplemented with this API. Currently, some locale-specific data are embedded in the implementation (e.g. http://trac.webkit.org/browser/trunk/Source/WebCore/Resources/calendarPicker.js?rev=122212 ), which may not be scalable for tens of languages/locales. 
> 
> [2] http://code.google.com/p/v8-i18n

I have never used v8-i18n, but does it apply OS's setting correctly?
A locale data in ICU doesn't match to the system's locale data (Bug 84388), and ICU doesn't reflect user's customization; e.g. One uses "fr" locale, but he can change the system locale setting so that '.' is used for decimal separators.
Comment 2 Nebojša Ćirić 2012-07-11 01:05:13 PDT
v8-i18n takes all of the settings from ICU, it doesn't use OS settings.

It would be possible to implement that functionality using native system calls, but as the spec goes there is nothing in there that requires this feature (it is left to implementation to decide).
Comment 3 Jungshik Shin 2012-08-09 12:37:23 PDT
re: comment !

The date picker etc has to follow the locale specified by 'lang/xml:lang'. 

If lang/xml:lang is not specified,  which one to follow, the current UI language of a browser or the OS settings, is debatable. Currently, v8-i18n uses the current UI language.
Comment 4 Jungshik Shin 2012-10-02 11:09:12 PDT
Ecma TC39 (in charge of EcmaScript standard) approved EcmaScript I18N API and referred it to Ecma GA (General Assembly) for the final approval.  Chrome has this implemented under v8Intl namespace and just switched to Intl.* in ToT (will show up next dev channel as Intl.*). Mozilla will have it soon (their implementation will also use ICU). Microsoft has an ActiveX-based add-on.
Comment 5 Steven R. Loomis 2014-11-04 16:14:36 PST
FYI: https://bugs.webkit.org/show_bug.cgi?id=137616 is a dup.
Comment 6 Alexey Proskuryakov 2014-11-04 16:29:31 PST
*** Bug 137616 has been marked as a duplicate of this bug. ***
Comment 7 Alexey Proskuryakov 2014-11-04 16:30:10 PST
rdar://problem/15881480
Comment 8 Dennis Becker 2015-03-06 07:34:26 PST
When will this be implemented in WebKit / Safari? It is now 2015, it is a standard since December 2012.

On caniuse.com you can see every other major browser vendor supports this API.
There is a huge (~130kb) polyfill Intl.js to fill this gap which is really ugly for an up-to-date browser!

Multi language websites really need such an implementation for localised dates, currencies or numbers.



ECMAScript Internationalization API Specification
http://www.ecma-international.org/ecma-402/1.0/

Can I Use? comparison
internationalization
Comment 9 Steven R. Loomis 2015-03-06 08:30:45 PST
(In reply to comment #8)
> When will this be implemented in WebKit / Safari? It is now 2015, it is a
> standard since December 2012.

On by default in node.js as of 0.12 http://blog.nodejs.org/2015/02/06/node-v0-12-0-stable/
Comment 10 Patrick A 2015-04-07 16:21:50 PDT
Dennis, (comment #8)

You said "There is a huge (~130kb) polyfill Intl.js".

Whose Intl.js is that? Would you have a pointer?

Do you know if it implements collation?

Anyone else,

What about Webkit support? What alternates do you suggest for collating international data on Safari if no support is planed?
Comment 11 Turadg Aleahmad 2015-04-07 16:28:23 PDT
Patrick,

The polyfill is available at https://github.com/andyearnshaw/Intl.js/

It's about to get bigger https://github.com/andyearnshaw/Intl.js/pull/85

This polyfill does not implement Intl.Collator, and I haven't found any that do.
Comment 12 Nebojša Ćirić 2015-04-08 09:58:48 PDT
Turadg,

 you probably won't find a native JS collation implementation. Collation data size (and complexity of the implementation) was the primary reason we started the ECMA 402 standardization process.

 Number and date formatting was just nice to have (other libraries already do that).
Comment 13 Andy VanWagoner 2015-05-21 18:07:17 PDT
Created attachment 253565 [details]
Patch
Comment 14 Andy VanWagoner 2015-05-21 18:13:22 PDT
Does this seem like the right place for the Intl namespace to live? Collator, DateTimeFormat, and NumberFormat will be sibling files as they are created.
Comment 15 Benjamin Poulain 2015-05-22 13:40:15 PDT
Comment on attachment 253565 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253565&action=review

r- for the CMakeLists.txt. No real complain otherwise.

> Source/JavaScriptCore/ChangeLog:9
> +        Started work on implementing the Internationaization APIs by adding the feature flag
> +        (disabled until ready), and adding a global Intl object to serve as the namespace.

It would be good to provide a link to the spec here for reference.

> Source/JavaScriptCore/ChangeLog:16
> +        * runtime/JSGlobalObject.cpp:

There is one more build file that needs to know about JSGlobalObject.cpp: Source/JavaScriptCore/CMakeLists.txt

CMake is used for GTK, EFL, and some Windows variant.

> Source/WTF/ChangeLog:9
> +        Started work on implementing the Internationaization APIs by adding the feature flag
> +        (disabled until ready), and adding a global Intl object to serve as the namespace.

You don't need to repeat yourself in each changelog.

Each file has the description for the changes in their particular subpart of the project.
Comment 16 Andy VanWagoner 2015-05-27 14:08:01 PDT
Created attachment 253799 [details]
Patch
Comment 17 Andy VanWagoner 2015-05-27 14:09:25 PDT
I've tried adding the missing headers so I can use NumberFormat from ICU, but I get a build error in trying to find UnicodeString. You can see the error in the new patchset.
Comment 18 Darin Adler 2015-05-27 18:39:00 PDT
Comment on attachment 253799 [details]
Patch

Generally we need to use the C API to ICU, not the C++ API. I think you’ll have better luck if you take that approach. The code here seems to be trying to use the C++.
Comment 19 Andy VanWagoner 2015-05-27 18:43:32 PDT
ok. I will try that.

(In reply to comment #18)
> Comment on attachment 253799 [details]
> Patch
> 
> Generally we need to use the C API to ICU, not the C++ API. I think you’ll
> have better luck if you take that approach. The code here seems to be trying
> to use the C++.
Comment 20 Darin Adler 2015-05-29 09:23:56 PDT
Comment on attachment 253799 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=253799&action=review

review- because EWS shows this is not complying on iOS or Mac yet; didn’t review in detail

> Source/JavaScriptCore/icu/unicode/bytestream.h:48
> +class U_COMMON_API ByteSink : public UMemory {

Please don’t add ICU headers for ICU’s C++ classes to the project. We cannot create a dependency on ICU’s C++ API; unlike the ICU C API it’s not guaranteed compatible across versions of the ICU framework.

> Source/JavaScriptCore/icu/unicode/uobject.h:101
> +class U_COMMON_API UMemory {

Pl
Comment 21 Andy VanWagoner 2015-05-29 20:32:49 PDT
Created attachment 253950 [details]
Patch
Comment 22 Andy VanWagoner 2015-05-29 20:35:25 PDT
I'm afraid that I'm too frustrated to keep working on this in my limited free time. I hope my patch is at least a helpful start for someone who knows what they are doing in the webkit source.
Comment 23 Darin Adler 2015-05-30 16:11:51 PDT
Sure, I agree that this is not a good “first WebKit patch” project. Usually better to start with something smaller.
Comment 24 Andy VanWagoner 2015-06-01 07:04:38 PDT
Created attachment 253997 [details]
Patch
Comment 25 Andy VanWagoner 2015-06-01 07:08:04 PDT
If I can get help on the basic structure, building out the `intl` namespace and the objects in it, then I can work on getting it to have the right behavior.

Right now, the patch compiles, but crashes immediately. It looks like I'm not allocating the memory for the NumberFormat properly. Can someone please take a look and offer any pointers?
Comment 26 Andy VanWagoner 2015-06-01 22:40:29 PDT
Created attachment 254051 [details]
Add feature flag and base Intl namespace object
Comment 27 Andy VanWagoner 2015-06-03 13:27:38 PDT
Comment on attachment 254051 [details]
Add feature flag and base Intl namespace object

This latest patch compiles, and when the flag is enabled, adds the `intl` object. There are tests (currently skipped, but pass when flag is enabled) to ensure the object matches the spec.
Comment 28 Benjamin Poulain 2015-06-30 14:51:09 PDT
Comment on attachment 254051 [details]
Add feature flag and base Intl namespace object

View in context: https://bugs.webkit.org/attachment.cgi?id=254051&action=review

This is a good patch. Let's land it.

I'll help Andy move forward with Intl, and I'll take on myself to remove the code if needed.

> LayoutTests/js/script-tests/intl.js:17
> +// is deletable, inferred from use of "Initial" in spec, consistent with other implementations

Something useful for testing is testing all the properties. You can use Object.getOwnPropertyDescritpor() to get them for any property.
Comment 29 WebKit Commit Bot 2015-06-30 15:21:51 PDT
Comment on attachment 254051 [details]
Add feature flag and base Intl namespace object

Rejecting attachment 254051 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 254051, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
/intl.html
patching file LayoutTests/js/script-tests/intl.js
patching file ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKitLibraries/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file WebKitLibraries/win/tools/vsprops/FeatureDefines.props
patching file WebKitLibraries/win/tools/vsprops/FeatureDefinesCairo.props

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Benjamin Poulain']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.appspot.com/results/4582240751190016
Comment 30 Benjamin Poulain 2015-06-30 16:44:25 PDT
Created attachment 255874 [details]
Patch for landing
Comment 31 Andy VanWagoner 2015-06-30 18:03:48 PDT
Created attachment 255886 [details]
Add namespace object and feature flag
Comment 32 WebKit Commit Bot 2015-06-30 19:02:56 PDT
Comment on attachment 255886 [details]
Add namespace object and feature flag

Clearing flags on attachment: 255886

Committed r186161: <http://trac.webkit.org/changeset/186161>
Comment 33 WebKit Commit Bot 2015-06-30 19:03:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 34 Andy VanWagoner 2015-06-30 19:05:14 PDT
The landed patch did not implement the whole API, just got the process started.
Comment 35 Simon Fraser (smfr) 2015-06-30 19:51:05 PDT
This broke tests on the 32-bit JSC bot:
https://build.webkit.org/builders/Apple%20Mavericks%2032-bit%20JSC%20%28BuildAndTest%29/builds/11124/steps/webkit-32bit-jsc-test/logs/stdio

Please fix or it will be rolled out.
Comment 36 Andy VanWagoner 2015-06-30 20:18:58 PDT
(In reply to comment #35)
> This broke tests on the 32-bit JSC bot:
> https://build.webkit.org/builders/Apple%20Mavericks%2032-
> bit%20JSC%20%28BuildAndTest%29/builds/11124/steps/webkit-32bit-jsc-test/logs/
> stdio
> 
> Please fix or it will be rolled out.

The failed tests look like the new tests I added. I put in the TestExpectations to Skip these tests until the flag is enabled by default.

Is there another way I can indicate these tests shouldn't be run?
Comment 37 Simon Fraser (smfr) 2015-06-30 22:47:20 PDT
The failing tests are under LayoutTests/js/script-tests/, but I don't know how to disable these.
Comment 38 Simon Fraser (smfr) 2015-06-30 23:01:47 PDT
Filip fixed it in http://trac.webkit.org/changeset/186166
Comment 39 Andy VanWagoner 2015-07-17 19:14:41 PDT
Created attachment 257013 [details]
Patch
Comment 40 WebKit Commit Bot 2015-07-17 19:17:10 PDT
Attachment 257013 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlDateTimeFormatPrototype.cpp:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlNumberFormatConstructor.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlNumberFormatConstructor.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlCollatorPrototype.cpp:48:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlDateTimeFormatConstructor.cpp:36:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlDateTimeFormatConstructor.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/IntlNumberFormatPrototype.cpp:48:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 8 in 41 files


If any of these errors are false positives, please file a bug against check-webkit-style.