Closed Bug 966559 Opened 6 years ago Closed 3 years ago

Internationalization API uses ICU C++ API for NumberingSystem


(Core :: JavaScript: Internationalization API, defect)

Not set



Tracking Status
firefox52 --- fixed


(Reporter: mozillabugs, Unassigned)




(2 files, 2 obsolete files)

The implementation of the ECMAScript Internationalization uses the ICU C++ API to obtain information about the default numbering system for a locale. Using the C++ API is unsafe when using an ICU library provided by the operating system:

The C++ API was used here because there was no equivalent C API at the time:
The missing C API has since been added in ICU 52.
Attached patch Use new C API instead of C++ API (obsolete) — Splinter Review
This patch builds with the Internationalization API enabled or disabled, and passes the intl402 tests if build with the Internationalization API enabled.

However, the new C API in ICU is still marked as draft in the API documentation, so it may be to early to use it in production:
Attached patch Use new C API instead of C++ API (obsolete) — Splinter Review
Now with updated Try run at
Attachment #8369022 - Attachment is obsolete: true
Duplicate of this bug: 919859
Component: JavaScript Engine → JavaScript: Internationalization API
This patch would be complete, but on b2g where we use the system ICU in some configurations, the system ICU is "50.1+" per build/autoconf/icu.m4.  According to this bug, unumsys.h was added in 52.1, so landing this patch as-is would break b2g with system ICU.

Where's that leave this patch?  Pocketed, I guess, until system b2g is newer.  Or maybe some sort of adjustment can be made based on $ANDROID_VERSION.  I dunno.  Not important enough to investigate and possibly solve now.
Attachment #8369122 - Attachment is obsolete: true
Depends on: 1298614
Can we now land it?
Dusted off the patch.
Comment on attachment 8806460 [details]
Bug 966559 - Use public C API for NumberingSystem.
Attachment #8806460 - Flags: review?(jwalden+bmo) → review+
Pushed by
Use public C API for NumberingSystem. r=Waldo
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.